[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333583: [lldb-test] Add a testing harness for the JITs 
IRMemoryMap (authored by vedantk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47508?vs=149173=149183#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47508

Files:
  lldb/trunk/lit/Expr/TestIRMemoryMap.test
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -15,25 +15,31 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Expression/IRMemoryMap.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "llvm/ADT/IntervalMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+#include 
 #include 
 
 using namespace lldb;
@@ -46,6 +52,15 @@
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
 cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
+cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(IRMemoryMapSubcommand));
+
+/// Create a target using the file pointed to by \p Filename, or abort.
+TargetSP createTarget(Debugger , const std::string );
+
+/// Read \p Filename into a null-terminated buffer, or abort.
+std::unique_ptr openFile(const std::string );
 
 namespace breakpoint {
 static cl::opt Target(cl::Positional, cl::desc(""),
@@ -135,8 +150,49 @@
 
 static int dumpSymbols(Debugger );
 }
+
+namespace irmemorymap {
+static cl::opt Target(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::sub(IRMemoryMapSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(IRMemoryMapSubcommand));
+using AllocationT = std::pair;
+bool areAllocationsOverlapping(const AllocationT , const AllocationT );
+using AddrIntervalMap =
+  IntervalMap>;
+bool evalMalloc(IRMemoryMap , StringRef Line,
+AddrIntervalMap );
+int evaluateMemoryMapCommands(Debugger );
+} // namespace irmemorymap
+
 } // namespace opts
 
+TargetSP opts::createTarget(Debugger , const std::string ) {
+  TargetSP Target;
+  Status ST =
+  Dbg.GetTargetList().CreateTarget(Dbg, Filename, /*triple*/ "",
+   /*get_dependent_modules*/ false,
+   /*platform_options*/ nullptr, Target);
+  if (ST.Fail()) {
+errs() << formatv("Failed to create target '{0}: {1}\n", Filename, ST);
+exit(1);
+  }
+  return Target;
+}
+
+std::unique_ptr opts::openFile(const std::string ) {
+  auto MB = MemoryBuffer::getFileOrSTDIN(Filename);
+  if (!MB) {
+errs() << formatv("Could not open file '{0}: {1}\n", Filename,
+  MB.getError().message());
+exit(1);
+  }
+  return std::move(*MB);
+}
+
 void opts::breakpoint::dumpState(const BreakpointList , LinePrinter ) {
   P.formatLine("{0} breakpoint{1}", List.GetSize(), plural(List.GetSize()));
   if (List.GetSize() > 0)
@@ -177,7 +233,7 @@
 switch (Cmd[0]) {
 case '%':
   if (Cmd.consume_front("%p") && (Cmd.empty() || !isalnum(Cmd[0]))) {
-OS << sys::path::parent_path(CommandFile);
+OS << sys::path::parent_path(breakpoint::CommandFile);
 break;
   }
   // fall through
@@ -192,26 +248,11 @@
 }
 
 int opts::breakpoint::evaluateBreakpoints(Debugger ) {
-  TargetSP Target;
-  Status ST =
-  Dbg.GetTargetList().CreateTarget(Dbg, breakpoint::Target, /*triple*/ "",
-   /*get_dependent_modules*/ false,
-   /*platform_options*/ nullptr, Target);
-  if (ST.Fail()) {
-errs() << formatv("Failed to create target 

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thank you for making the changes. This looks fine to me. The more testing, the 
better.




Comment at: tools/lldb-test/lldb-test.cpp:532
+  // Print the result of the allocation before checking its validity.
+  outs() << format("Malloc: address = 0x%x\n", Addr);
+

`%x` is not right here. Maybe use `formatv`?


https://reviews.llvm.org/D47508



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 149173.
vsk edited the summary of this revision.
vsk added a comment.

- Really fix the allocation overlap test. The previous version of this patch 
would not detect overlaps in which the end of the new allocation is contained 
within an existing allocation.

> The idea that came to me while looking at this is testing this gdb-client 
> style. This would allow you to mock the server responses to allocation and 
> e.g. test handling of allocation failures. However, the problem is these 
> tests sit on top of SBAPI and there seems to be no way to issue "raw" 
> allocation requests through that (although maybe there is a case to be made 
> for SBProcess.AllocateMemory as a generally useful API).
> 
> However, if this does the job you want, then I'm fine with that too.

Testing at this level looks to be sufficient to uncover the bugs I'm concerned 
about, so I'd prefer not to extend the SB API if possible.


https://reviews.llvm.org/D47508

Files:
  lit/Expr/TestIRMemoryMap.test
  source/Target/Process.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -15,37 +15,53 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Expression/IRMemoryMap.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "llvm/ADT/IntervalMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+#include 
 #include 
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm;
 
 namespace opts {
+
 static cl::SubCommand BreakpointSubcommand("breakpoints",
"Test breakpoint resolution");
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
 cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
+cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(IRMemoryMapSubcommand));
+
+/// Create a target using the file pointed to by \p Filename, or abort.
+TargetSP createTarget(Debugger , const std::string );
+
+/// Read \p Filename into a null-terminated buffer, or abort.
+std::unique_ptr openFile(const std::string );
 
 namespace breakpoint {
 static cl::opt Target(cl::Positional, cl::desc(""),
@@ -135,8 +151,49 @@
 
 static int dumpSymbols(Debugger );
 }
+
+namespace irmemorymap {
+static cl::opt Target(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::sub(IRMemoryMapSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(IRMemoryMapSubcommand));
+using AllocationT = std::pair;
+bool areAllocationsOverlapping(const AllocationT , const AllocationT );
+using AddrIntervalMap =
+  IntervalMap>;
+bool evalMalloc(IRMemoryMap , StringRef Line,
+AddrIntervalMap );
+int evaluateMemoryMapCommands(Debugger );
+} // namespace irmemorymap
+
 } // namespace opts
 
+TargetSP opts::createTarget(Debugger , const std::string ) {
+  TargetSP Target;
+  Status ST =
+  Dbg.GetTargetList().CreateTarget(Dbg, Filename, /*triple*/ "",
+   /*get_dependent_modules*/ false,
+   /*platform_options*/ nullptr, Target);
+  if (ST.Fail()) {
+errs() << formatv("Failed to create target '{0}: {1}\n", Filename, ST);
+exit(1);
+  }
+  return Target;
+}
+
+std::unique_ptr opts::openFile(const std::string ) {
+  auto MB = MemoryBuffer::getFileOrSTDIN(Filename);
+  if (!MB) {
+errs() << formatv("Could not open file '{0}: {1}\n", Filename,
+  MB.getError().message());
+exit(1);
+  }
+  return std::move(*MB);
+}
+
 void opts::breakpoint::dumpState(const BreakpointList , LinePrinter ) {
   P.formatLine("{0} breakpoint{1}", List.GetSize(), plural(List.GetSize()));
   if 

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 149159.
vsk edited the summary of this revision.
vsk added a comment.

- Use %zu, and improve detection of overlapping allocations.


https://reviews.llvm.org/D47508

Files:
  lit/Expr/TestIRMemoryMap.test
  source/Target/Process.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -15,37 +15,53 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Expression/IRMemoryMap.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "llvm/ADT/IntervalMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+#include 
 #include 
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm;
 
 namespace opts {
+
 static cl::SubCommand BreakpointSubcommand("breakpoints",
"Test breakpoint resolution");
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
 cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
+cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(IRMemoryMapSubcommand));
+
+/// Create a target using the file pointed to by \p Filename, or abort.
+TargetSP createTarget(Debugger , const std::string );
+
+/// Read \p Filename into a null-terminated buffer, or abort.
+std::unique_ptr openFile(const std::string );
 
 namespace breakpoint {
 static cl::opt Target(cl::Positional, cl::desc(""),
@@ -135,8 +151,47 @@
 
 static int dumpSymbols(Debugger );
 }
+
+namespace irmemorymap {
+static cl::opt Target(cl::Positional, cl::desc(""),
+   cl::Required,
+   cl::sub(IRMemoryMapSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(IRMemoryMapSubcommand));
+using AddrIntervalMap =
+  IntervalMap>;
+bool evalMalloc(IRMemoryMap , StringRef Line,
+AddrIntervalMap );
+int evaluateMemoryMapCommands(Debugger );
+} // namespace irmemorymap
+
 } // namespace opts
 
+TargetSP opts::createTarget(Debugger , const std::string ) {
+  TargetSP Target;
+  Status ST =
+  Dbg.GetTargetList().CreateTarget(Dbg, Filename, /*triple*/ "",
+   /*get_dependent_modules*/ false,
+   /*platform_options*/ nullptr, Target);
+  if (ST.Fail()) {
+errs() << formatv("Failed to create target '{0}: {1}\n", Filename, ST);
+exit(1);
+  }
+  return Target;
+}
+
+std::unique_ptr opts::openFile(const std::string ) {
+  auto MB = MemoryBuffer::getFileOrSTDIN(Filename);
+  if (!MB) {
+errs() << formatv("Could not open file '{0}: {1}\n", Filename,
+  MB.getError().message());
+exit(1);
+  }
+  return std::move(*MB);
+}
+
 void opts::breakpoint::dumpState(const BreakpointList , LinePrinter ) {
   P.formatLine("{0} breakpoint{1}", List.GetSize(), plural(List.GetSize()));
   if (List.GetSize() > 0)
@@ -177,7 +232,7 @@
 switch (Cmd[0]) {
 case '%':
   if (Cmd.consume_front("%p") && (Cmd.empty() || !isalnum(Cmd[0]))) {
-OS << sys::path::parent_path(CommandFile);
+OS << sys::path::parent_path(breakpoint::CommandFile);
 break;
   }
   // fall through
@@ -192,26 +247,11 @@
 }
 
 int opts::breakpoint::evaluateBreakpoints(Debugger ) {
-  TargetSP Target;
-  Status ST =
-  Dbg.GetTargetList().CreateTarget(Dbg, breakpoint::Target, /*triple*/ "",
-   /*get_dependent_modules*/ false,
-   /*platform_options*/ nullptr, Target);
-  if (ST.Fail()) {
-errs() << formatv("Failed to create target '{0}: {1}\n", breakpoint::Target,
-  ST);
-exit(1);
-  }
-
-  auto MB = MemoryBuffer::getFileOrSTDIN(CommandFile);
-  if (!MB) {
-

[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

2018-05-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

The idea that came to me while looking at this is testing this gdb-client 
style. This would allow you to mock the server responses to allocation and e.g. 
test handling of allocation failures. However, the problem is these tests sit 
on top of SBAPI and  there seems to be no way to  issue "raw" allocation 
requests through that (although maybe there is a case to be made for 
SBProcess.AllocateMemory as a generally useful API).

However, if this does the job you want, then I'm fine with that too.




Comment at: tools/lldb-test/lldb-test.cpp:503
+  uint8_t Alignment;
+  int Matches = sscanf(Line.data(), "malloc %lu %hhu", , );
+  if (Matches != 2)

is `Line` null-terminated here? Also a size_t arg should have a `%zu` modifier, 
but I am not sure if all msvc versions support that. It might be best to make 
the type uint64_t and then use SCNu64.



Comment at: tools/lldb-test/lldb-test.cpp:536-542
+  bool Overlaps = AllocatedIntervals.lookup(Addr, false);
+  if (Size && !Overlaps)
+Overlaps = AllocatedIntervals.lookup(Addr + Size - 1, false);
+  if (Overlaps) {
+outs() << "Malloc error: overlapping allocation detected\n";
+exit(1);
+  }

It looks like this won't detect the case when a larger interval is placed on 
top of a smaller one (e.g. `0x1000-0x4000` and `0x2000-0x3000`).


https://reviews.llvm.org/D47508



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits