[Lldb-commits] [lldb] r357451 - Simplify TestGdbRemoteRegisterState

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr  2 00:47:38 2019
New Revision: 357451

URL: http://llvm.org/viewvc/llvm-project?rev=357451&view=rev
Log:
Simplify TestGdbRemoteRegisterState

While reviewing D56233 it became clear to me that this test can be
simplified. There's no need for a start-stop cycle in the inferior -- we
can start fiddling with its registers as soon as it is launched.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteRegisterState.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteRegisterState.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteRegisterState.py?rev=357451&r1=357450&r2=357451&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteRegisterState.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteRegisterState.py
 Tue Apr  2 00:47:38 2019
@@ -24,15 +24,6 @@ class TestGdbRemoteRegisterState(gdbremo
 if with_suffix:
 self.add_thread_suffix_request_packets()
 self.add_threadinfo_collection_packets()
-self.test_sequence.add_log_lines([
-# Start the inferior...
-"read packet: $c#63",
-# ... match output
-{"type": "output_match", "regex": self.maybe_strict_output_regex(
-r"message:main entered\r\n")},
-], True)
-# ... then interrupt.
-self.add_interrupt_packets()
 
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)


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

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for lldb-server on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The patch is pretty big, so I haven't done an in-depth review yet. The fact 
that you've ran clang-format over entire files (and not just the diffs) does 
not help.

For a start, I've highlighted the parts that I believe can/should go into a 
separate patch. Some of them are obvious and can be committed immediately. 
Others should go through a separate review. We can get back to the core of the 
patch once these have been dealt with (FWIW, I haven't seen anything obviously 
wrong from a quick glance).




Comment at: include/lldb/Core/Debugger.h:338
   }
+  bool StartEventHandlerThread();
 

This looks like experimental code accidentally left in (?)



Comment at: include/lldb/Target/Platform.h:599
 error.SetErrorStringWithFormat(
-"Platform::ReadFile() is not supported in the %s platform",
+"Platform::WriteFile() is not supported in the %s platform",
 GetName().GetCString());

To keep the scope of this patch (which is already pretty big) as small as 
possible, could you submit this as a separate patch? No review needed.



Comment at: include/lldb/lldb-types.h:42-45
+typedef void *process_t;  // Process type is HANDLE
+typedef void *thread_t;   // Host thread type
+typedef void *file_t; // Host file type
+typedef unsigned int __w64 socket_t;  // Host socket type

Please revert or submit as a separate patch.



Comment at: lldb.xcodeproj/project.pbxproj:10522
"-lxml2",
+   "-lLLVMSupport",
"-framework",

I am surpised that you needed to change the xcode project in any way, as this 
code should not even build there. Please explain.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteRegisterState.py:28
+
+# Skip O* packet test for Windows. Will add it back when pty is 
supported.
+triple = self.dbg.GetSelectedPlatform().GetTriple()

The fact that you were able to do this told me that this part of the test is 
actually irrelevant. So, I just went ahead and deleted this part altogether 
(r357451).



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:208-211
+# In current implementation of llgs on Windows, as a response to '\x03' 
packet, the debugger
+# of the native process will trigger a call to DebugBreakProcess that will 
create a new thread
+# to handle the exception debug event. So one more stop thread will be 
notified to the
+# delegate, e.g. llgs.  So tests below to assert the stop threads number 
will all fail.

That sounds fun. Is that how things are supposed to work, or is that something 
you're planning to change/fix? (mostly just curious).



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py:674-675
 
+#@expectedFailureAll(oslist=["windows"])
+#skipIfWindows
 @llgs_test

delete ?



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py:1494
 threads = self.wait_for_thread_count(3, timeout_seconds=5)
-self.assertEqual(len(threads), 3)
+self.assertEqual(len(threads) - 1, 3)
 

The -1 looks like it needs to be windows-only.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py:434
 attempts = 0
-MAX_ATTEMPTS = 20
+MAX_ATTEMPTS = 1
 

I don't think this can go in like this. However, I agree that this double-retry 
loop is pretty ugly. When I get some time, I'll try to see if I can rewrite 
this part to use reverse connects. That way we won't get any races at all. For 
the time being, I'd just revert this.



Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:1
 //===-- main.cpp *- C++ 
-*-===//
 //

Could you (as a separate patch) convert this file to use c++ threads? That 
should allow us to cut down on the ifdefs by using standard std::thread and 
std::mutex.



Comment at: source/Host/common/Socket.cpp:89-104
+socket_up = llvm::make_unique(true, child_processes_inherit);
 break;
   case ProtocolUdp:
-socket_up =
-llvm::make_unique(true, child_processes_inherit);
+socket_up = llvm::make_unique(true, child_processes_inherit);
 break;
   case ProtocolUnixDomain:
 #ifndef LLDB_DISABLE_POSIX

Please avoid spurious reformats of entire files. This makes it patches hard to 
review, particularly when they are as large as this one.

For the changes in this spec

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

lgtm. Thanks for the patience.




Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:401
+  basename_module_spec.GetUUID().Clear();
+  basename_module_spec.GetFileSpec().GetDirectory().Clear();
+  module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);

clayborg wrote:
> labath wrote:
> > What will happen if you just delete this line? Based on my experiments, I 
> > believe we should still end up looking through the exec-search-paths list. 
> > However, the test does not seem to be modifying this setting, so it's not 
> > clear to me how it is even finding the binary in the first place. What's 
> > the mechanism you're relying on for the lookup in the test?
> It doesn't work without clearing the fullpath here and I didn't want to 
> modify the current behavior of the exec search paths in here just in case it 
> would affect others that were depending on a certain behavior.
Ok, I don't think we have to block on that, but I'd still like to understand 
how the files are found in the first place. Right now, I don't understand it 
because:
a) they are not in exec-search-path
b) (I think) they are not in CWD
So the fact that they are found may be accidental and prone to breakage.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60001/new/

https://reviews.llvm.org/D60001



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

[Lldb-commits] [PATCH] D60068: PDBFPO: Refactor register reference resolution

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added a comment.

Thanks.

BTW, do you have any thoughts where this code could live (it can't stay here 
because I need to access it from SymbolFileBreakpad too). I was thinking of the 
Symbol library, as both users are `SymbolFile`s.

I don't think this will have any external dependencies (that I can't get rid 
of), so another option might be Utility, but that seems a bit too low for this 
functionality (it will include code for generating dwarf expressions).




Comment at: 
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp:287
+
+  // lookup register reference as lvalue in preceding assignments
+  auto it = m_dependent_programs.find(symbol->GetName());

amccarth wrote:
> nit:  As long as you're correcting this comment, can you fix "look up" to be 
> two words?
will do.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60068/new/

https://reviews.llvm.org/D60068



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

[Lldb-commits] [lldb] r357455 - PDBFPO: Refactor register reference resolution

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr  2 01:44:24 2019
New Revision: 357455

URL: http://llvm.org/viewvc/llvm-project?rev=357455&view=rev
Log:
PDBFPO: Refactor register reference resolution

Summary:
This refactors moves the register name->number resolution out of the
FPOProgramNodeRegisterRef class. Instead I create a special
FPOProgramNodeSymbol class, which holds unresolved symbols, and move the
resolution into the ResolveRegisterRefs visitor.

The background here is that I'd like to use this code for Breakpad
unwind info, which uses similar syntax to describe unwind info. For
example, a simple breakpad unwind program might look like:
.cfa: $esp 8 + $ebp: .cfa 8 - ^

To be able to do this, I need to be able to customize register
resolving, as that is presently hardcoded to use codeview register
names, but breakpad supports a lot more architectures with different
register names. Moving the resolution into a separate class will allow
each user to use a different resolution logic.

Reviewers: aleksandr.urakov, zturner, amccarth

Subscribers: jdoerfert, lldb-commits

Differential Revision: https://reviews.llvm.org/D60068

Modified:

lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

Modified: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp?rev=357455&r1=357454&r2=357455&view=diff
==
--- 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
 Tue Apr  2 01:44:24 2019
@@ -27,9 +27,21 @@ namespace {
 class FPOProgramNode;
 class FPOProgramASTVisitor;
 
+class NodeAllocator {
+public:
+  template  T *makeNode(Args &&... args) {
+void *new_node_mem = m_alloc.Allocate(sizeof(T), alignof(T));
+return new (new_node_mem) T(std::forward(args)...);
+  }
+
+private:
+  llvm::BumpPtrAllocator m_alloc;
+};
+
 class FPOProgramNode {
 public:
   enum Kind {
+Symbol,
 Register,
 IntegerLiteral,
 BinaryOp,
@@ -49,46 +61,31 @@ private:
   Kind m_token_kind;
 };
 
-class FPOProgramNodeRegisterRef : public FPOProgramNode {
+class FPOProgramNodeSymbol: public FPOProgramNode {
 public:
-  FPOProgramNodeRegisterRef(llvm::StringRef name)
-  : FPOProgramNode(Register), m_name(name) {}
+  FPOProgramNodeSymbol(llvm::StringRef name)
+  : FPOProgramNode(Symbol), m_name(name) {}
 
   void Accept(FPOProgramASTVisitor *visitor) override;
 
   llvm::StringRef GetName() const { return m_name; }
-  uint32_t GetLLDBRegNum() const { return m_lldb_reg_num; }
-
-  bool ResolveLLDBRegisterNum(llvm::Triple::ArchType arch_type);
 
 private:
   llvm::StringRef m_name;
-  uint32_t m_lldb_reg_num = LLDB_INVALID_REGNUM;
 };
 
-bool FPOProgramNodeRegisterRef::ResolveLLDBRegisterNum(
-llvm::Triple::ArchType arch_type) {
-
-  llvm::StringRef reg_name = m_name.slice(1, m_name.size());
-
-  // lookup register name to get lldb register number
-  llvm::ArrayRef> register_names =
-  llvm::codeview::getRegisterNames();
-  auto it = llvm::find_if(
-  register_names,
-  [®_name](const llvm::EnumEntry ®ister_entry) {
-return reg_name.compare_lower(register_entry.Name) == 0;
-  });
+class FPOProgramNodeRegisterRef : public FPOProgramNode {
+public:
+  FPOProgramNodeRegisterRef(uint32_t lldb_reg_num)
+  : FPOProgramNode(Register), m_lldb_reg_num(lldb_reg_num) {}
 
-  if (it == register_names.end()) {
-return false;
-  }
+  void Accept(FPOProgramASTVisitor *visitor) override;
 
-  auto reg_id = static_cast(it->Value);
-  m_lldb_reg_num = npdb::GetLLDBRegisterNumber(arch_type, reg_id);
+  uint32_t GetLLDBRegNum() const { return m_lldb_reg_num; }
 
-  return m_lldb_reg_num != LLDB_INVALID_REGNUM;
-}
+private:
+  uint32_t m_lldb_reg_num;
+};
 
 class FPOProgramNodeIntegerLiteral : public FPOProgramNode {
 public:
@@ -157,12 +154,17 @@ class FPOProgramASTVisitor {
 public:
   virtual ~FPOProgramASTVisitor() = default;
 
+  virtual void Visit(FPOProgramNodeSymbol *node) {}
   virtual void Visit(FPOProgramNodeRegisterRef *node) {}
   virtual void Visit(FPOProgramNodeIntegerLiteral *node) {}
   virtual void Visit(FPOProgramNodeBinaryOp *node) {}
   virtual void Visit(FPOProgramNodeUnaryOp *node) {}
 };
 
+void FPOProgramNodeSymbol::Accept(FPOProgramASTVisitor *visitor) {
+  visitor->Visit(this);
+}
+
 void FPOProgramNodeRegisterRef::Accept(FPOProgramASTVisitor *visitor) {
   visitor->Visit(this);
 }
@@ -216,11 +218,10 @@ void FPOProgramASTVisitorMergeDependent:
 void FPOProgramASTVisitorMergeDependent::TryReplace(
 FPOProgramNode *&node_ref) const {
 
-  while (node_ref->GetKind() == FPOProgramNode::Register) {
-auto *node_register_ref =
-static_cast(node_ref);
+  while (node_ref->GetKind() == FPOProgramNod

[Lldb-commits] [PATCH] D60068: PDBFPO: Refactor register reference resolution

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB357455: PDBFPO: Refactor register reference resolution 
(authored by labath, committed by ).
Herald added a subscriber: abidh.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D60068?vs=193091&id=193242#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60068/new/

https://reviews.llvm.org/D60068

Files:
  source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

Index: source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===
--- source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -27,9 +27,21 @@
 class FPOProgramNode;
 class FPOProgramASTVisitor;
 
+class NodeAllocator {
+public:
+  template  T *makeNode(Args &&... args) {
+void *new_node_mem = m_alloc.Allocate(sizeof(T), alignof(T));
+return new (new_node_mem) T(std::forward(args)...);
+  }
+
+private:
+  llvm::BumpPtrAllocator m_alloc;
+};
+
 class FPOProgramNode {
 public:
   enum Kind {
+Symbol,
 Register,
 IntegerLiteral,
 BinaryOp,
@@ -49,46 +61,31 @@
   Kind m_token_kind;
 };
 
-class FPOProgramNodeRegisterRef : public FPOProgramNode {
+class FPOProgramNodeSymbol: public FPOProgramNode {
 public:
-  FPOProgramNodeRegisterRef(llvm::StringRef name)
-  : FPOProgramNode(Register), m_name(name) {}
+  FPOProgramNodeSymbol(llvm::StringRef name)
+  : FPOProgramNode(Symbol), m_name(name) {}
 
   void Accept(FPOProgramASTVisitor *visitor) override;
 
   llvm::StringRef GetName() const { return m_name; }
-  uint32_t GetLLDBRegNum() const { return m_lldb_reg_num; }
-
-  bool ResolveLLDBRegisterNum(llvm::Triple::ArchType arch_type);
 
 private:
   llvm::StringRef m_name;
-  uint32_t m_lldb_reg_num = LLDB_INVALID_REGNUM;
 };
 
-bool FPOProgramNodeRegisterRef::ResolveLLDBRegisterNum(
-llvm::Triple::ArchType arch_type) {
-
-  llvm::StringRef reg_name = m_name.slice(1, m_name.size());
-
-  // lookup register name to get lldb register number
-  llvm::ArrayRef> register_names =
-  llvm::codeview::getRegisterNames();
-  auto it = llvm::find_if(
-  register_names,
-  [®_name](const llvm::EnumEntry ®ister_entry) {
-return reg_name.compare_lower(register_entry.Name) == 0;
-  });
+class FPOProgramNodeRegisterRef : public FPOProgramNode {
+public:
+  FPOProgramNodeRegisterRef(uint32_t lldb_reg_num)
+  : FPOProgramNode(Register), m_lldb_reg_num(lldb_reg_num) {}
 
-  if (it == register_names.end()) {
-return false;
-  }
+  void Accept(FPOProgramASTVisitor *visitor) override;
 
-  auto reg_id = static_cast(it->Value);
-  m_lldb_reg_num = npdb::GetLLDBRegisterNumber(arch_type, reg_id);
+  uint32_t GetLLDBRegNum() const { return m_lldb_reg_num; }
 
-  return m_lldb_reg_num != LLDB_INVALID_REGNUM;
-}
+private:
+  uint32_t m_lldb_reg_num;
+};
 
 class FPOProgramNodeIntegerLiteral : public FPOProgramNode {
 public:
@@ -157,12 +154,17 @@
 public:
   virtual ~FPOProgramASTVisitor() = default;
 
+  virtual void Visit(FPOProgramNodeSymbol *node) {}
   virtual void Visit(FPOProgramNodeRegisterRef *node) {}
   virtual void Visit(FPOProgramNodeIntegerLiteral *node) {}
   virtual void Visit(FPOProgramNodeBinaryOp *node) {}
   virtual void Visit(FPOProgramNodeUnaryOp *node) {}
 };
 
+void FPOProgramNodeSymbol::Accept(FPOProgramASTVisitor *visitor) {
+  visitor->Visit(this);
+}
+
 void FPOProgramNodeRegisterRef::Accept(FPOProgramASTVisitor *visitor) {
   visitor->Visit(this);
 }
@@ -216,11 +218,10 @@
 void FPOProgramASTVisitorMergeDependent::TryReplace(
 FPOProgramNode *&node_ref) const {
 
-  while (node_ref->GetKind() == FPOProgramNode::Register) {
-auto *node_register_ref =
-static_cast(node_ref);
+  while (node_ref->GetKind() == FPOProgramNode::Symbol) {
+auto *node_symbol_ref = static_cast(node_ref);
 
-auto it = m_dependent_programs.find(node_register_ref->GetName());
+auto it = m_dependent_programs.find(node_symbol_ref->GetName());
 if (it == m_dependent_programs.end()) {
   break;
 }
@@ -234,39 +235,70 @@
   FPOProgramASTVisitorResolveRegisterRefs(
   const llvm::DenseMap
   &dependent_programs,
-  llvm::Triple::ArchType arch_type)
-  : m_dependent_programs(dependent_programs), m_arch_type(arch_type) {}
+  llvm::Triple::ArchType arch_type, NodeAllocator &alloc)
+  : m_dependent_programs(dependent_programs), m_arch_type(arch_type),
+m_alloc(alloc) {}
 
-  bool Resolve(FPOProgramNode *program);
+  bool Resolve(FPOProgramNode *&program);
 
 private:
-  void Visit(FPOProgramNodeRegisterRef *node) override;
-  void Visit(FPOProgramNodeIntegerLiteral *node) override {}
   void Visit(FPOProgramNodeBinaryOp *node) override;
   void Visit(FPOProgramNodeUnaryOp *node) override;
 
-private:
+  bool Try

[Lldb-commits] [lldb] r357456 - Fix llvm_unreachable in TestWriteMemory

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr  2 01:56:22 2019
New Revision: 357456

URL: http://llvm.org/viewvc/llvm-project?rev=357456&view=rev
Log:
Fix llvm_unreachable in TestWriteMemory

The test was hitting llvm_unreachable in
Platform::GetSoftwareBreakpointTrapOpcode because it could not figure
out the architecture of the process. Since that is not the purpose of
the test, I change the test to use an explicit
CreateTargetWithFileAndTargetTriple command to specify it.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py?rev=357456&r1=357455&r2=357456&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
 Tue Apr  2 01:56:22 2019
@@ -14,7 +14,7 @@ class TestWriteMemory(GDBRemoteTestBase)
 return "OK"
 
 self.server.responder = MyResponder()
-target = self.dbg.CreateTarget('')
+target = self.dbg.CreateTargetWithFileAndTargetTriple('', 
'x86_64-pc-linux')
 process = self.connect(target)
 
 bp = target.BreakpointCreateByAddress(0x1000)


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

[Lldb-commits] [PATCH] D60022: [Process] Fix WriteMemory return value

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Just an FYI: I was hitting llvm_unreachable in 
Platform::GetSoftwareBreakpointTrapOpcode in this test. I fixed it in r357456 
by specifying an explicit triple in the test. (I'm not sure how this wasn't hit 
on the darwin bots.)




Comment at: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py:19-20
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets")
+process = self.connect(target)

jasonmolenda wrote:
> davide wrote:
> > I'm not sure you really need this, we can commit it if we need to debug. 
> > Please note that the bots always run with `TraceOn() == True`.
> no big deal, but fwiw I put this sequence in every time I write a gdb remote 
> client test - it's essential for debugging any problems with them.  The 
> amount of output is very small, it's not like a real debug session.
I think it would make sense to teach the general architecture of these tests (I 
guess that would be the MockGDBServer class) to dump some additional info in 
the `TraceOn()` case.

BTW, when running a test locally, you can pass `--channel "gdb-remote packets"` 
to dotest to have it output the relevant logs to a file (the file will go to 
the test traces directory).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60022/new/

https://reviews.llvm.org/D60022



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

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

Ping


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59826/new/

https://reviews.llvm.org/D59826



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

[Lldb-commits] [lldb] r357459 - Fix flakyness in TestCommandScriptImmediateOutput

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr  2 02:45:40 2019
New Revision: 357459

URL: http://llvm.org/viewvc/llvm-project?rev=357459&view=rev
Log:
Fix flakyness in TestCommandScriptImmediateOutput

I'm not sure why this surfaced at this particular point, but
TestCommandScriptImmediateOutput (a pexpect test) had a synchronization
issue, where the (lldb) promts it was expecting were getting out of
sync. This happened for two reasons:
- it did not expect the initial (lldb) prompt we print at startup
- launchArgs() returned None, which resulted in an extra "target create
  None" command being issued to lldb (and an extra unhandled prompt
  being printed).

Resolving these two issues seems to fix (or at least, improve) the test.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
lldb/trunk/packages/Python/lldbsuite/test/lldbpexpect.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py?rev=357459&r1=357458&r2=357459&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py
 Tue Apr  2 02:45:40 2019
@@ -31,7 +31,9 @@ class CommandScriptImmediateOutputTestCa
 @skipIfDarwin
 def test_command_script_immediate_output_console(self):
 """Test that LLDB correctly allows scripted commands to set immediate 
output to the console."""
+prompt = "\(lldb\) "
 self.launch(timeout=10)
+self.expect(prompt)
 
 script = os.path.join(self.getSourceDir(), 'custom_command.py')
 prompt = "\(lldb\) "
@@ -44,7 +46,7 @@ class CommandScriptImmediateOutputTestCa
 'mycommand',
 patterns='this is a test string, just a test string')
 self.sendline('command script delete mycommand', patterns=[prompt])
-self.quit(gracefully=False)
+self.quit()
 
 @skipIfRemote  # test not remote-ready llvm.org/pr24813
 @expectedFailureAll(
@@ -54,7 +56,9 @@ class CommandScriptImmediateOutputTestCa
 @skipIfDarwin
 def test_command_script_immediate_output_file(self):
 """Test that LLDB correctly allows scripted commands to set immediate 
output to a file."""
+prompt = "\(lldb\) "
 self.launch(timeout=10)
+self.expect(prompt)
 
 test_files = {self.getBuildArtifact('read.txt'): 'r',
   self.getBuildArtifact('write.txt'): 'w',
@@ -71,7 +75,6 @@ class CommandScriptImmediateOutputTestCa
 init.write(starter_string)
 
 script = os.path.join(self.getSourceDir(), 'custom_command.py')
-prompt = "\(lldb\) "
 
 self.sendline('command script import %s' % script, patterns=[prompt])
 
@@ -85,7 +88,7 @@ class CommandScriptImmediateOutputTestCa
 
 self.sendline('command script delete mywrite', patterns=[prompt])
 
-self.quit(gracefully=False)
+self.quit()
 
 for path, mode in test_files.items():
 with open(path, 'r') as result:
@@ -96,4 +99,3 @@ class CommandScriptImmediateOutputTestCa
 result.readline(), write_string + mode + '\n')
 
 self.assertTrue(os.path.isfile(path))
-os.remove(path)

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbpexpect.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbpexpect.py?rev=357459&r1=357458&r2=357459&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbpexpect.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbpexpect.py Tue Apr  2 
02:45:40 2019
@@ -27,7 +27,7 @@ else:
 TestBase.setUp(self)
 
 def launchArgs(self):
-pass
+return ""
 
 def launch(self, timeout=None):
 if timeout is None:


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

[Lldb-commits] [PATCH] D60068: PDBFPO: Refactor register reference resolution

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

I think both approaches are acceptable, but I would prefer to include it into 
the Symbols library because this functionality is more symbols-related than 
general.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60068/new/

https://reviews.llvm.org/D60068



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

[Lldb-commits] [lldb] r357463 - Make operator==s consistent between c++ and python APIs

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Apr  2 03:18:46 2019
New Revision: 357463

URL: http://llvm.org/viewvc/llvm-project?rev=357463&view=rev
Log:
Make operator==s consistent between c++ and python APIs

Summary:
modify-python-lldb.py had code to insert python equality operators to
some classes. Some of those classes already had c++ equality operators,
and some didn't.

This makes the situation more consistent, by removing all equality
handilng from modify-python-lldb. Instead, I add c++ operators to
classes where they were missing, and expose them in the swig interface
files so that they are available to python too.

The only tricky case was the SBAddress class, which had an operator==
defined as a free function, which is not handled by swig. This function
cannot be removed without breaking ABI, and we cannot add an extra
operator== member, as that would make equality comparisons ambiguous.
For this class, I define a python __eq__ function by hand and have it
delegate to the operator!=, which I have defined as a member function.

This isn't fully NFC, as the semantics of some equality functions in
python changes slightly, but I believe it changes for the better (e.g.,
previously SBBreakpoint.__eq__ would consider two breakpoints with the
same ID as equal, even if they belonged to different targets; now they
are only equal if they belong to the same target).

Reviewers: jingham, clayborg, zturner

Subscribers: jdoerfert, JDevlieghere, lldb-commits

Differential Revision: https://reviews.llvm.org/D59819

Modified:
lldb/trunk/include/lldb/API/SBAddress.h
lldb/trunk/include/lldb/API/SBFileSpec.h
lldb/trunk/include/lldb/API/SBWatchpoint.h
lldb/trunk/scripts/Python/modify-python-lldb.py
lldb/trunk/scripts/interface/SBAddress.i
lldb/trunk/scripts/interface/SBBreakpoint.i
lldb/trunk/scripts/interface/SBFileSpec.i
lldb/trunk/scripts/interface/SBModule.i
lldb/trunk/scripts/interface/SBWatchpoint.i
lldb/trunk/scripts/lldb.swig
lldb/trunk/source/API/SBAddress.cpp
lldb/trunk/source/API/SBFileSpec.cpp
lldb/trunk/source/API/SBWatchpoint.cpp

Modified: lldb/trunk/include/lldb/API/SBAddress.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBAddress.h?rev=357463&r1=357462&r2=357463&view=diff
==
--- lldb/trunk/include/lldb/API/SBAddress.h (original)
+++ lldb/trunk/include/lldb/API/SBAddress.h Tue Apr  2 03:18:46 2019
@@ -31,6 +31,10 @@ public:
 
   explicit operator bool() const;
 
+  // operator== is a free function
+
+  bool operator!=(const SBAddress &rhs) const;
+
   bool IsValid() const;
 
   void Clear();

Modified: lldb/trunk/include/lldb/API/SBFileSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBFileSpec.h?rev=357463&r1=357462&r2=357463&view=diff
==
--- lldb/trunk/include/lldb/API/SBFileSpec.h (original)
+++ lldb/trunk/include/lldb/API/SBFileSpec.h Tue Apr  2 03:18:46 2019
@@ -30,6 +30,10 @@ public:
 
   explicit operator bool() const;
 
+  bool operator==(const SBFileSpec &rhs) const;
+
+  bool operator!=(const SBFileSpec &rhs) const;
+
   bool IsValid() const;
 
   bool Exists() const;

Modified: lldb/trunk/include/lldb/API/SBWatchpoint.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBWatchpoint.h?rev=357463&r1=357462&r2=357463&view=diff
==
--- lldb/trunk/include/lldb/API/SBWatchpoint.h (original)
+++ lldb/trunk/include/lldb/API/SBWatchpoint.h Tue Apr  2 03:18:46 2019
@@ -27,6 +27,10 @@ public:
 
   explicit operator bool() const;
 
+  bool operator==(const SBWatchpoint &rhs) const;
+
+  bool operator!=(const SBWatchpoint &rhs) const;
+
   bool IsValid() const;
 
   SBError GetError();

Modified: lldb/trunk/scripts/Python/modify-python-lldb.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/modify-python-lldb.py?rev=357463&r1=357462&r2=357463&view=diff
==
--- lldb/trunk/scripts/Python/modify-python-lldb.py (original)
+++ lldb/trunk/scripts/Python/modify-python-lldb.py Tue Apr  2 03:18:46 2019
@@ -87,10 +87,6 @@ compile_unit_iter = "def compile_uni
 # Eligible objects are those containers with unambiguous iteration support.
 len_def = "def __len__(self): return self.%s()"
 
-# This supports the rich comparison methods of __eq__ and __ne__.
-eq_def = "def __eq__(self, other): return isinstance(other, %s) and %s"
-ne_def = "def __ne__(self, other): return not self.__eq__(other)"
-
 # A convenience iterator for SBSymbol!
 symbol_in_section_iter_def = '''
 def symbol_in_section_iter(self, section):
@@ -135,17 +131,6 @@ d = {'SBBreakpoint': ('GetNumLocations',
  'SBModule-symbol-in-section': symbol_in_section_iter_def
  }
 
-#
-# This dictionary defines a mapp

[Lldb-commits] [PATCH] D59819: Make operator==s consistent between c++ and python APIs

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357463: Make operator==s consistent between c++ and python 
APIs (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59819?vs=192271&id=193249#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59819/new/

https://reviews.llvm.org/D59819

Files:
  lldb/trunk/include/lldb/API/SBAddress.h
  lldb/trunk/include/lldb/API/SBFileSpec.h
  lldb/trunk/include/lldb/API/SBWatchpoint.h
  lldb/trunk/scripts/Python/modify-python-lldb.py
  lldb/trunk/scripts/interface/SBAddress.i
  lldb/trunk/scripts/interface/SBBreakpoint.i
  lldb/trunk/scripts/interface/SBFileSpec.i
  lldb/trunk/scripts/interface/SBModule.i
  lldb/trunk/scripts/interface/SBWatchpoint.i
  lldb/trunk/scripts/lldb.swig
  lldb/trunk/source/API/SBAddress.cpp
  lldb/trunk/source/API/SBFileSpec.cpp
  lldb/trunk/source/API/SBWatchpoint.cpp

Index: lldb/trunk/source/API/SBAddress.cpp
===
--- lldb/trunk/source/API/SBAddress.cpp
+++ lldb/trunk/source/API/SBAddress.cpp
@@ -69,6 +69,13 @@
   return false;
 }
 
+bool SBAddress::operator!=(const SBAddress &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBAddress, operator!=,(const SBAddress &),
+   &rhs);
+
+  return !(*this == rhs);
+}
+
 bool SBAddress::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBAddress, IsValid);
   return this->operator bool();
@@ -294,6 +301,8 @@
   LLDB_REGISTER_CONSTRUCTOR(SBAddress, (lldb::addr_t, lldb::SBTarget &));
   LLDB_REGISTER_METHOD(const lldb::SBAddress &,
SBAddress, operator=,(const lldb::SBAddress &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+ SBAddress, operator!=,(const lldb::SBAddress &));
   LLDB_REGISTER_METHOD_CONST(bool, SBAddress, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBAddress, operator bool, ());
   LLDB_REGISTER_METHOD(void, SBAddress, Clear, ());
Index: lldb/trunk/source/API/SBWatchpoint.cpp
===
--- lldb/trunk/source/API/SBWatchpoint.cpp
+++ lldb/trunk/source/API/SBWatchpoint.cpp
@@ -70,6 +70,20 @@
   return bool(m_opaque_wp.lock());
 }
 
+bool SBWatchpoint::operator==(const SBWatchpoint &rhs) const {
+  LLDB_RECORD_METHOD_CONST(
+  bool, SBWatchpoint, operator==,(const SBWatchpoint &), rhs);
+
+  return GetSP() == rhs.GetSP();
+}
+
+bool SBWatchpoint::operator!=(const SBWatchpoint &rhs) const {
+  LLDB_RECORD_METHOD_CONST(
+  bool, SBWatchpoint, operator!=,(const SBWatchpoint &), rhs);
+
+  return !(*this == rhs);
+}
+
 SBError SBWatchpoint::GetError() {
   LLDB_RECORD_METHOD_NO_ARGS(lldb::SBError, SBWatchpoint, GetError);
 
@@ -303,6 +317,10 @@
   LLDB_REGISTER_METHOD(lldb::watch_id_t, SBWatchpoint, GetID, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBWatchpoint, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBWatchpoint, operator bool, ());
+  LLDB_REGISTER_METHOD_CONST(
+  bool, SBWatchpoint, operator==,(const lldb::SBWatchpoint &));
+  LLDB_REGISTER_METHOD_CONST(
+  bool, SBWatchpoint, operator!=,(const lldb::SBWatchpoint &));
   LLDB_REGISTER_METHOD(lldb::SBError, SBWatchpoint, GetError, ());
   LLDB_REGISTER_METHOD(int32_t, SBWatchpoint, GetHardwareIndex, ());
   LLDB_REGISTER_METHOD(lldb::addr_t, SBWatchpoint, GetWatchAddress, ());
Index: lldb/trunk/source/API/SBFileSpec.cpp
===
--- lldb/trunk/source/API/SBFileSpec.cpp
+++ lldb/trunk/source/API/SBFileSpec.cpp
@@ -62,6 +62,20 @@
   return *this;
 }
 
+bool SBFileSpec::operator==(const SBFileSpec &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBFileSpec, operator==,(const SBFileSpec &rhs),
+   rhs);
+
+  return ref() == rhs.ref();
+}
+
+bool SBFileSpec::operator!=(const SBFileSpec &rhs) const {
+  LLDB_RECORD_METHOD_CONST(bool, SBFileSpec, operator!=,(const SBFileSpec &rhs),
+   rhs);
+
+  return !(*this == rhs);
+}
+
 bool SBFileSpec::IsValid() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBFileSpec, IsValid);
   return this->operator bool();
@@ -185,6 +199,10 @@
   LLDB_REGISTER_CONSTRUCTOR(SBFileSpec, (const char *, bool));
   LLDB_REGISTER_METHOD(const lldb::SBFileSpec &,
SBFileSpec, operator=,(const lldb::SBFileSpec &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+ SBFileSpec, operator==,(const lldb::SBFileSpec &));
+  LLDB_REGISTER_METHOD_CONST(bool,
+ SBFileSpec, operator!=,(const lldb::SBFileSpec &));
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, operator bool, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFileSpec, Exists, ());
Index: lldb/trunk/scripts/lldb.swig
===

[Lldb-commits] [PATCH] D59634: Add minidump support to obj2yaml

[James Henderson via Phabricator via lldb-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

In D59634#1449621 , @labath wrote:

> James, do you have any thoughts on this? I think the minidump-specific 
> changes are all pretty straight-forward here, so the thing I'm most 
> interested in is whether this is ok from a obj2yaml perspective.


Sorry, I've been a bit snowed under with other stuff recently. I don't claim to 
be an expert on obj2yaml, but this LGTM.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59634/new/

https://reviews.llvm.org/D59634



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

[Lldb-commits] [PATCH] D60119: modify-python-lldb.py: clean up __iter__ and __len__ support

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: zturner, jingham, amccarth.
Herald added a subscriber: jdoerfert.

Instead of modifying the swig-generated code, just add the appropriate
methods to the interface files in order to get the swig to do the
generation for us.

This is a straight-forward move from the python script to the interface
files. The single class which has nontrivial handling in the script
(SBModule) has been left for a separate patch.

For the cases where I did not find any tests exercising the
iteration/length methods (i.e., no tests failed after I stopped emitting
them), I tried to add basic tests for that functionality.


https://reviews.llvm.org/D60119

Files:
  packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py
  
packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py
  packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
  packages/Python/lldbsuite/test/python_api/default-constructor/sb_section.py
  packages/Python/lldbsuite/test/python_api/default-constructor/sb_thread.py
  packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
  scripts/Python/modify-python-lldb.py
  scripts/interface/SBBreakpoint.i
  scripts/interface/SBCompileUnit.i
  scripts/interface/SBDebugger.i
  scripts/interface/SBInstructionList.i
  scripts/interface/SBProcess.i
  scripts/interface/SBSection.i
  scripts/interface/SBStringList.i
  scripts/interface/SBSymbolContextList.i
  scripts/interface/SBTarget.i
  scripts/interface/SBThread.i
  scripts/interface/SBType.i
  scripts/interface/SBValue.i
  scripts/interface/SBValueList.i

Index: scripts/interface/SBValueList.i
===
--- scripts/interface/SBValueList.i
+++ scripts/interface/SBValueList.i
@@ -102,6 +102,10 @@
 GetFirstValueByName (const char* name) const;
 
 %pythoncode %{
+def __iter__(self):
+'''Iterate over all values in a lldb.SBValueList object.'''
+return lldb_iter(self, 'GetSize', 'GetValueAtIndex')
+
 def __len__(self):
 return int(self.GetSize())
 
Index: scripts/interface/SBValue.i
===
--- scripts/interface/SBValue.i
+++ scripts/interface/SBValue.i
@@ -495,6 +495,14 @@
 for idx in range(len(accessor)):
 children.append(accessor[idx])
 return children
+
+def __iter__(self):
+'''Iterate over all child values of a lldb.SBValue object.'''
+return lldb_iter(self, 'GetNumChildren', 'GetChildAtIndex')
+
+def __len__(self):
+'''Return the number of child values of a lldb.SBValue object.'''
+return self.GetNumChildren()
 
 __swig_getmethods__["children"] = get_value_child_list
 if _newclass: children = property(get_value_child_list, None, doc='''A read only property that returns a list() of lldb.SBValue objects for the children of the value.''')
Index: scripts/interface/SBType.i
===
--- scripts/interface/SBType.i
+++ scripts/interface/SBType.i
@@ -509,6 +509,16 @@
 GetSize();
 
 ~SBTypeList();
+
+%pythoncode%{
+def __iter__(self):
+'''Iterate over all types in a lldb.SBTypeList object.'''
+return lldb_iter(self, 'GetSize', 'GetTypeAtIndex')
+
+def __len__(self):
+'''Return the number of types in a lldb.SBTypeList object.'''
+return self.GetSize()
+%}
 };
 
 } // namespace lldb
Index: scripts/interface/SBThread.i
===
--- scripts/interface/SBThread.i
+++ scripts/interface/SBThread.i
@@ -428,6 +428,14 @@
 SafeToCallFunctions ();
 
 %pythoncode %{
+def __iter__(self):
+'''Iterate over all frames in a lldb.SBThread object.'''
+return lldb_iter(self, 'GetNumFrames', 'GetFrameAtIndex')
+
+def __len__(self):
+'''Return the number of frames in a lldb.SBThread object.'''
+return self.GetNumFrames()
+
 class frames_access(object):
 '''A helper object that will lazily hand out frames for a thread when supplied an index.'''
 def __init__(self, sbthread):
Index: scripts/interface/SBTarget.i
===
--- scripts/interface/SBTarget.i
+++ scripts/interface/SBTarget.i
@@ -1103,6 +1103,21 @@
 modules.append(self.GetModuleAtIndex(idx))
 return modules
 
+def module_iter(self):
+'''Returns an iterator over all modules in a lldb.SBTarget
+object.'''
+return lldb_iter(self, 'GetNumModules', 'GetModuleAtIndex')
+
+def breakpoint_iter(self):
+'''Returns an iterator over all breakpoints in a lldb.SBTarget
+object.'''
+r

[Lldb-commits] [PATCH] D59634: Add minidump support to obj2yaml

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D59634#1451182 , @jhenderson wrote:

> Sorry, I've been a bit snowed under with other stuff recently. I don't claim 
> to be an expert on obj2yaml, but this LGTM.


No worries. You still have the fastest average response time for my code 
reviews/emails. :)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59634/new/

https://reviews.llvm.org/D59634



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

[Lldb-commits] [PATCH] D59634: Add minidump support to obj2yaml

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3cee663e71f6: Add minidump support to obj2yaml (authored by 
labath).
Herald added a subscriber: hiraditya.

Changed prior to commit:
  https://reviews.llvm.org/D59634?vs=191656&id=193254#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59634/new/

https://reviews.llvm.org/D59634

Files:
  llvm/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/tools/obj2yaml/basic-minidump.yaml
  llvm/tools/obj2yaml/CMakeLists.txt
  llvm/tools/obj2yaml/minidump2yaml.cpp
  llvm/tools/obj2yaml/obj2yaml.cpp
  llvm/tools/obj2yaml/obj2yaml.h

Index: llvm/tools/obj2yaml/obj2yaml.h
===
--- llvm/tools/obj2yaml/obj2yaml.h
+++ llvm/tools/obj2yaml/obj2yaml.h
@@ -13,6 +13,7 @@
 #define LLVM_TOOLS_OBJ2YAML_OBJ2YAML_H
 
 #include "llvm/Object/COFF.h"
+#include "llvm/Object/Minidump.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -23,6 +24,8 @@
  const llvm::object::ObjectFile &Obj);
 std::error_code macho2yaml(llvm::raw_ostream &Out,
const llvm::object::Binary &Obj);
+llvm::Error minidump2yaml(llvm::raw_ostream &Out,
+  const llvm::object::MinidumpFile &Obj);
 std::error_code wasm2yaml(llvm::raw_ostream &Out,
   const llvm::object::WasmObjectFile &Obj);
 
Index: llvm/tools/obj2yaml/obj2yaml.cpp
===
--- llvm/tools/obj2yaml/obj2yaml.cpp
+++ llvm/tools/obj2yaml/obj2yaml.cpp
@@ -10,6 +10,7 @@
 #include "Error.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"
+#include "llvm/Object/Minidump.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
 
@@ -40,6 +41,8 @@
   // TODO: If this is an archive, then burst it and dump each entry
   if (ObjectFile *Obj = dyn_cast(&Binary))
 return errorCodeToError(dumpObject(*Obj));
+  if (MinidumpFile *Minidump = dyn_cast(&Binary))
+return minidump2yaml(outs(), *Minidump);
 
   return Error::success();
 }
Index: llvm/tools/obj2yaml/minidump2yaml.cpp
===
--- /dev/null
+++ llvm/tools/obj2yaml/minidump2yaml.cpp
@@ -0,0 +1,24 @@
+//===- minidump2yaml.cpp - Minidump to yaml conversion tool -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Error.h"
+#include "obj2yaml.h"
+#include "llvm/Object/Minidump.h"
+#include "llvm/ObjectYAML/MinidumpYAML.h"
+#include "llvm/Support/YAMLTraits.h"
+
+using namespace llvm;
+
+Error minidump2yaml(raw_ostream &Out, const object::MinidumpFile &Obj) {
+  auto ExpectedObject = MinidumpYAML::Object::create(Obj);
+  if (!ExpectedObject)
+return ExpectedObject.takeError();
+  yaml::Output Output(Out);
+  Output << *ExpectedObject;
+  return llvm::Error::success();
+}
Index: llvm/tools/obj2yaml/CMakeLists.txt
===
--- llvm/tools/obj2yaml/CMakeLists.txt
+++ llvm/tools/obj2yaml/CMakeLists.txt
@@ -13,6 +13,7 @@
   dwarf2yaml.cpp
   elf2yaml.cpp
   macho2yaml.cpp
+  minidump2yaml.cpp
   wasm2yaml.cpp
   Error.cpp
   )
Index: llvm/test/tools/obj2yaml/basic-minidump.yaml
===
--- /dev/null
+++ llvm/test/tools/obj2yaml/basic-minidump.yaml
@@ -0,0 +1,35 @@
+# RUN: yaml2obj %s | obj2yaml - | FileCheck %s
+
+--- !minidump
+Streams: 
+  - Type:SystemInfo
+Processor Arch:  ARM64
+Platform ID: Linux
+CSD Version RVA: 0x01020304
+CPU: 
+  CPUID:   0x05060708
+  - Type:LinuxAuxv
+Content: DEADBEEFBAADF00D
+  - Type:LinuxMaps
+Text: |
+  400d9000-400db000 r-xp  b3:04 227/system/bin/app_process
+  400db000-400dc000 r--p 1000 b3:04 227/system/bin/app_process
+
+...
+
+# CHECK:  --- !minidump
+# CHECK-NEXT: Streams: 
+# CHECK-NEXT:   - Type:SystemInfo
+# CHECK-NEXT: Processor Arch:  ARM64
+# CHECK-NEXT: Platform ID: Linux
+# CHECK-NEXT: CSD Version RVA: 0x01020304
+# CHECK-NEXT: CPU: 
+# CHECK-NEXT:   CPUID:   0x05060708
+# CHECK-NEXT:   - Type:LinuxAuxv
+# CHECK-NEXT: Content: DEADBEEFBAADF00D
+# CHECK-NEXT:   - Type:LinuxMaps
+# CHECK-NEXT: Text: |
+# CHECK-NEXT:   400d9000-400db000 r-xp  b3:04 227/system/bin/app_process
+# CHECK-NEXT:   400db000-400dc000 r--

[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: zturner, amccarth, jhenderson, clayborg.
Herald added a subscriber: jdoerfert.
Herald added a project: LLVM.

The ModuleList stream consists of an integer giving the number of
entries in the list, followed by the list itself. Each entry in the list
describes a module (dynamically loaded objects which were loaded in the
process when it crashed (or when the minidump was generated).

The code for reading the list is relatively straight-forward, with a
single gotcha. Some minidump writers are emitting padding after the
"count" field in order to align the subsequent list on 8 byte boundary
(this depends on how their ModuleList type was defined and the native
alignment of various types on their platform). Fortunately, the minidump
format contains enough redundancy (in the form of the stream length
field in the stream directory), which allows us to detect this situation
and correct it.

This patch just adds the ability to parse the stream. Code for
conversion to/from yaml will come in a follow-up patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D60121

Files:
  include/llvm/BinaryFormat/Minidump.h
  include/llvm/Object/Minidump.h
  lib/Object/Minidump.cpp
  unittests/Object/MinidumpTest.cpp

Index: unittests/Object/MinidumpTest.cpp
===
--- unittests/Object/MinidumpTest.cpp
+++ unittests/Object/MinidumpTest.cpp
@@ -280,3 +280,115 @@
   EXPECT_THAT_EXPECTED(File.getString(68), HasValue("a"));
   EXPECT_THAT_EXPECTED(File.getString(75), HasValue("a"));
 }
+
+TEST(MinidumpFile, getModuleList) {
+  std::vector OneModule{
+  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  0x20, 0, 0, 0,// StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  0, 0, 0, 0, 0, 0, 0, 0,   // Flags
+// Stream Directory
+  4, 0, 0, 0, 112, 0, 0, 0, // Type, DataSize,
+  0x2c, 0, 0, 0,// RVA
+  // ModuleList
+  1, 0, 0, 0, // NumberOfModules
+  1, 2, 3, 4, 5, 6, 7, 8, // BaseOfImage
+  9, 0, 1, 2, 3, 4, 5, 6, // SizeOfImage, Checksum
+  7, 8, 9, 0, 1, 2, 3, 4, // TimeDateStamp, ModuleNameRVA
+  0, 0, 0, 0, 0, 0, 0, 0, // Signature, StructVersion
+  0, 0, 0, 0, 0, 0, 0, 0, // FileVersion
+  0, 0, 0, 0, 0, 0, 0, 0, // ProductVersion
+  0, 0, 0, 0, 0, 0, 0, 0, // FileFlagsMask, FileFlags
+  0, 0, 0, 0, // FileOS
+  0, 0, 0, 0, 0, 0, 0, 0, // FileType, FileSubType
+  0, 0, 0, 0, 0, 0, 0, 0, // FileDate
+  1, 2, 3, 4, 5, 6, 7, 8, // CvRecord
+  9, 0, 1, 2, 3, 4, 5, 6, // MiscRecord
+  7, 8, 9, 0, 1, 2, 3, 4, // Reserved0
+  5, 6, 7, 8, 9, 0, 1, 2, // Reserved1
+  };
+  // Same as before, but with a padded module list.
+  std::vector PaddedModule{
+  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  0x20, 0, 0, 0,// StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  0, 0, 0, 0, 0, 0, 0, 0,   // Flags
+// Stream Directory
+  4, 0, 0, 0, 116, 0, 0, 0, // Type, DataSize,
+  0x2c, 0, 0, 0,// RVA
+  // ModuleList
+  1, 0, 0, 0, // NumberOfModules
+  0, 0, 0, 0, // Padding
+  1, 2, 3, 4, 5, 6, 7, 8, // BaseOfImage
+  9, 0, 1, 2, 3, 4, 5, 6, // SizeOfImage, Checksum
+  7, 8, 9, 0, 1, 2, 3, 4, // TimeDateStamp, ModuleNameRVA
+  0, 0, 0, 0, 0, 0, 0, 0, // Signature, StructVersion
+  0, 0, 0, 0, 0, 0, 0, 0, // FileVersion
+  0, 0, 0, 0, 0, 0, 0, 0, // ProductVersion
+  0, 0, 0, 0, 0, 0, 0, 0, // FileFlagsMask, FileFlags
+  0, 0, 0, 0, // FileOS
+  0, 0, 0, 0, 0, 0, 0, 0, // FileType, FileSubType
+  0, 0, 0, 0, 0, 0, 0, 0, // FileDate
+  1, 2, 3, 4, 5, 6, 7, 8, // CvRecord
+  9, 0, 1, 2, 3, 4, 5, 6, // MiscRecord
+  7, 8, 9, 0, 1, 2, 3, 4, // Reserved0
+  5, 6, 7, 8, 9, 0, 1, 2, // Reserved1
+  };
+
+  for (const std::vector &Data : {OneModule, PaddedModule}) {
+auto ExpectedFile = create(Data);
+ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+const MinidumpFile &File = **ExpectedFile;
+Expected> ExpectedModule = File.getModuleList();
+ASSERT_THAT_EXPECTED(ExpectedModule, Succeeded());
+ASSERT_EQ(1u, ExpectedModule->size());
+const Module &M = ExpectedModule.get()[0];
+EXPECT_EQ(0x0807060504030201u, M.BaseOfImage);
+EXPECT_EQ(0x02010009u, M.SizeOfImage);
+EXPECT_EQ(0x06050403u, M.Checksum);
+EXPECT_EQ(0x00090807u, M.TimeDateStamp);
+EXPECT_EQ(0x04030201u, M.Modul

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Any way to dump the AST in a test to verify we don't create multiple?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59826/new/

https://reviews.llvm.org/D59826



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

[Lldb-commits] [lldb] r357482 - Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Tue Apr  2 08:40:54 2019
New Revision: 357482

URL: http://llvm.org/viewvc/llvm-project?rev=357482&view=rev
Log:
Allow partial UUID matching in Minidump core file plug-in

Breakpad had bugs in earlier versions where it would take a 20 byte ELF build 
ID and put it into the minidump file as a 16 byte PDB70 UUID with an age of 
zero. This would make it impossible to do postmortem debugging with one of 
these older minidump files.

This fix allows partial matching of UUIDs. To do this we first try and match 
with the full UUID value, and then fall back to removing the original directory 
path from the module specification and we remove the UUID requirement, and then 
manually do the matching ourselves. This allows scripts to find symbols files 
using a symbol server, place them all in a directory, use the "setting set 
target.exec-search-paths" setting to specify the directory, and then load the 
core file. The Target::GetSharedModule() can then find the correct file without 
doing any other matching and load it.

Tests were added to cover a partial UUID match where the breakpad file has a 16 
byte UUID and the actual file on disk has a 20 byte UUID, both where the first 
16 bytes match, and don't match.

Differential Revision: https://reviews.llvm.org/D60001


Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
   (with props)
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357482&r1=357481&r2=357482&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Tue Apr  2 08:40:54 2019
@@ -8,6 +8,7 @@ from six import iteritems
 import shutil
 
 import lldb
+import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -132,3 +133,54 @@ class MiniDumpUUIDTestCase(TestBase):
 self.assertEqual(2, len(modules))
 self.verify_module(modules[0], "/not/exist/a", None)
 self.verify_module(modules[1], "/not/exist/b", None)
+
+def test_partial_uuid_match(self):
+"""
+Breakpad has been known to create minidump files using CvRecord in 
each
+module whose signature is set to PDB70 where the UUID only 
contains the
+first 16 bytes of a 20 byte ELF build ID. Code was added to 
+ProcessMinidump.cpp to deal with this and allows partial UUID 
matching. 
+
+This test verifies that if we have a minidump with a 16 byte UUID, 
that
+we are able to associate a symbol file with a 20 byte UUID only if 
the
+first 16 bytes match. In this case we will see the path from the 
file
+we found in the test directory and the 20 byte UUID from the actual
+file, not the 16 byte shortened UUID from the minidump.
+"""
+so_path = self.getBuildArtifact("libuuidmatch.so")
+self.yaml2obj("libuuidmatch.yaml", so_path)
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+cmd = 'settings set target.exec-search-paths "%s"' % 
(os.path.dirname(so_path))
+self.dbg.HandleCommand(cmd)
+self.process = 
self.target.LoadCore("linux-arm-partial-uuids-match.dmp")
+modules = self.target.modules
+self.assertEqual(1, len(modules))
+self.verify_module(modules[0],
+   "libuuidmatch.so", 
+   "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")
+
+def test_partial_uuid_mismatch(self):
+"""
+Breakpad has been known to create minidump files using CvRecord in 
each
+module whose signature is set to PDB70 where the UUID only 
contains the
+first 16 bytes of a 20 byte ELF build ID. Code was added to 
+ProcessMinidump.cpp to deal with this and allows partial UUID 
matching. 
+
+This test verifies that if we have a minidump with a 16 byte UUID, 
that
+ 

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357482: Allow partial UUID matching in Minidump core file 
plug-in (authored by gclayton, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60001?vs=193198&id=193299#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60001/new/

https://reviews.llvm.org/D60001

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
  lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
@@ -0,0 +1,14 @@
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_DYN
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.note.gnu.build-id
+Type:SHT_NOTE
+Flags:   [ SHF_ALLOC ]
+Address: 0x0114
+AddressAlign:0x0004
+Content: 040014000300474E55008295E17C66689E05CBB5DEE5003865D55267C116
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml
@@ -0,0 +1,14 @@
+--- !ELF
+FileHeader:  
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_DYN
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.note.gnu.build-id
+Type:SHT_NOTE
+Flags:   [ SHF_ALLOC ]
+Address: 0x0114
+AddressAlign:0x0004
+Content: 040014000300474E55007295E17C66689E05CBB5DEE5003865D55267C116
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -8,6 +8,7 @@
 import shutil
 
 import lldb
+import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -132,3 +133,54 @@
 self.assertEqual(2, len(modules))
 self.verify_module(modules[0], "/not/exist/a", None)
 self.verify_module(modules[1], "/not/exist/b", None)
+
+def test_partial_uuid_match(self):
+"""
+Breakpad has been known to create minidump files using CvRecord in each
+module whose signature is set to PDB70 where the UUID only contains the
+first 16 bytes of a 20 byte ELF build ID. Code was added to 
+ProcessMinidump.cpp to deal with this and allows partial UUID matching. 
+
+This test verifies that if we have a minidump with a 16 byte UUID, that
+we are able to associate a symbol file with a 20 byte UUID only if the
+first 16 bytes match. In this case we will see the path from the file
+we found in the test directory and the 20 byte UUID from the actual
+file, not the 16 byte shortened UUID from the minidump.
+"""
+so_path = self.getBuildArtifact("libuuidmatch.so")
+self.yaml2obj("libuuidmatch.yaml", so_path)
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+cmd = 'settings set target.exec-search-paths "%s"' % (os.path.dirname(so_path))
+self.dbg.HandleCommand(cmd)
+self.process = self.target.LoadCore("linux-arm-partial-uuids-match.dmp")
+modules = self.target.modules
+self.assertEqual(1, len(modules))
+self.verify

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked an inline comment as done.
clayborg added inline comments.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:401
+  basename_module_spec.GetUUID().Clear();
+  basename_module_spec.GetFileSpec().GetDirectory().Clear();
+  module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);

labath wrote:
> clayborg wrote:
> > labath wrote:
> > > What will happen if you just delete this line? Based on my experiments, I 
> > > believe we should still end up looking through the exec-search-paths 
> > > list. However, the test does not seem to be modifying this setting, so 
> > > it's not clear to me how it is even finding the binary in the first 
> > > place. What's the mechanism you're relying on for the lookup in the test?
> > It doesn't work without clearing the fullpath here and I didn't want to 
> > modify the current behavior of the exec search paths in here just in case 
> > it would affect others that were depending on a certain behavior.
> Ok, I don't think we have to block on that, but I'd still like to understand 
> how the files are found in the first place. Right now, I don't understand it 
> because:
> a) they are not in exec-search-path
> b) (I think) they are not in CWD
> So the fact that they are found may be accidental and prone to breakage.
I added the build directory to the "settings set target.exec-search-paths" in 
the tests in case this changes in the future.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60001/new/

https://reviews.llvm.org/D60001



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

Re: [Lldb-commits] [lldb] r357482 - Allow partial UUID matching in Minidump core file plug-in

[Davide Italiano via lldb-commits]

This broke the green dragon cmake bot

==
FAIL: test_partial_uuid_match (TestMiniDumpUUID.MiniDumpUUIDTestCase)
--
Traceback (most recent call last): File
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py",
line 161, in test_partial_uuid_match
"7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116") File
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py",
line 33, in verify_module self.assertEqual(verify_path,
module.GetFileSpec().fullpath) AssertionError: 'libuuidmatch.so' !=
'/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/functionalities/postmortem/minidump-new/TestMiniDumpUUID.test_partial_uuid_match/libuuidmatch.so'
Config=x86_64-/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang-9

Seems relatively straightforward to fix, can you take a look?
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/22921/console

(Also, change your reply to, it's still using your @apple address, you
can just mail Chris Lattner).

--
Davide

On Tue, Apr 2, 2019 at 8:39 AM Greg Clayton via lldb-commits
 wrote:
>
> Author: gclayton
> Date: Tue Apr  2 08:40:54 2019
> New Revision: 357482
>
> URL: http://llvm.org/viewvc/llvm-project?rev=357482&view=rev
> Log:
> Allow partial UUID matching in Minidump core file plug-in
>
> Breakpad had bugs in earlier versions where it would take a 20 byte ELF build 
> ID and put it into the minidump file as a 16 byte PDB70 UUID with an age of 
> zero. This would make it impossible to do postmortem debugging with one of 
> these older minidump files.
>
> This fix allows partial matching of UUIDs. To do this we first try and match 
> with the full UUID value, and then fall back to removing the original 
> directory path from the module specification and we remove the UUID 
> requirement, and then manually do the matching ourselves. This allows scripts 
> to find symbols files using a symbol server, place them all in a directory, 
> use the "setting set target.exec-search-paths" setting to specify the 
> directory, and then load the core file. The Target::GetSharedModule() can 
> then find the correct file without doing any other matching and load it.
>
> Tests were added to cover a partial UUID match where the breakpad file has a 
> 16 byte UUID and the actual file on disk has a 20 byte UUID, both where the 
> first 16 bytes match, and don't match.
>
> Differential Revision: https://reviews.llvm.org/D60001
>
>
> Added:
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp
>(with props)
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
>(with props)
> Modified:
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
> lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357482&r1=357481&r2=357482&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
>  Tue Apr  2 08:40:54 2019
> @@ -8,6 +8,7 @@ from six import iteritems
>  import shutil
>
>  import lldb
> +import os
>  from lldbsuite.test.decorators import *
>  from lldbsuite.test.lldbtest import *
>  from lldbsuite.test import lldbutil
> @@ -132,3 +133,54 @@ class MiniDumpUUIDTestCase(TestBase):
>  self.assertEqual(2, len(modules))
>  self.verify_module(modules[0], "/not/exist/a", None)
>  self.verify_module(modules[1], "/not/exist/b", None)
> +
> +def test_partial_uuid_match(self):
> +"""
> +Breakpad has been known to create minidump files using CvRecord 
> in each
> +module whose signature is set to PDB70 where the UUID only 
> contains the
> +first 16 bytes of a 20 byte ELF build ID. Code was added to
> +ProcessMinidump.cpp to deal with this and allows partial UUID 
> matching.
> +
> +This t

Re: [Lldb-commits] [lldb] r357482 - Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via lldb-commits]

I'll take a look

> On Apr 2, 2019, at 9:27 AM, Davide Italiano  wrote:
> 
> This broke the green dragon cmake bot
> 
> ==
> FAIL: test_partial_uuid_match (TestMiniDumpUUID.MiniDumpUUIDTestCase)
> --
> Traceback (most recent call last): File
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py",
> line 161, in test_partial_uuid_match
> "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116") File
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py",
> line 33, in verify_module self.assertEqual(verify_path,
> module.GetFileSpec().fullpath) AssertionError: 'libuuidmatch.so' !=
> '/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/functionalities/postmortem/minidump-new/TestMiniDumpUUID.test_partial_uuid_match/libuuidmatch.so'
> Config=x86_64-/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang-9
> 
> Seems relatively straightforward to fix, can you take a look?
> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/22921/console
> 
> (Also, change your reply to, it's still using your @apple address, you
> can just mail Chris Lattner).
> 
> --
> Davide
> 
> On Tue, Apr 2, 2019 at 8:39 AM Greg Clayton via lldb-commits
>  wrote:
>> 
>> Author: gclayton
>> Date: Tue Apr  2 08:40:54 2019
>> New Revision: 357482
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=357482&view=rev
>> Log:
>> Allow partial UUID matching in Minidump core file plug-in
>> 
>> Breakpad had bugs in earlier versions where it would take a 20 byte ELF 
>> build ID and put it into the minidump file as a 16 byte PDB70 UUID with an 
>> age of zero. This would make it impossible to do postmortem debugging with 
>> one of these older minidump files.
>> 
>> This fix allows partial matching of UUIDs. To do this we first try and match 
>> with the full UUID value, and then fall back to removing the original 
>> directory path from the module specification and we remove the UUID 
>> requirement, and then manually do the matching ourselves. This allows 
>> scripts to find symbols files using a symbol server, place them all in a 
>> directory, use the "setting set target.exec-search-paths" setting to specify 
>> the directory, and then load the core file. The Target::GetSharedModule() 
>> can then find the correct file without doing any other matching and load it.
>> 
>> Tests were added to cover a partial UUID match where the breakpad file has a 
>> 16 byte UUID and the actual file on disk has a 20 byte UUID, both where the 
>> first 16 bytes match, and don't match.
>> 
>> Differential Revision: https://reviews.llvm.org/D60001
>> 
>> 
>> Added:
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp
>>(with props)
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
>>(with props)
>> Modified:
>>
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
>>lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
>> 
>> Modified: 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357482&r1=357481&r2=357482&view=diff
>> ==
>> --- 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
>>  (original)
>> +++ 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
>>  Tue Apr  2 08:40:54 2019
>> @@ -8,6 +8,7 @@ from six import iteritems
>> import shutil
>> 
>> import lldb
>> +import os
>> from lldbsuite.test.decorators import *
>> from lldbsuite.test.lldbtest import *
>> from lldbsuite.test import lldbutil
>> @@ -132,3 +133,54 @@ class MiniDumpUUIDTestCase(TestBase):
>> self.assertEqual(2, len(modules))
>> self.verify_module(modules[0], "/not/exist/a", None)
>> self.verify_module(modules[1], "/not/exist/b", None)
>> +
>> +def test_partial_uuid_match(self):
>> +"""
>> +Breakpad has been known to create minidump files using CvRecord 
>> in each
>> +module whose signature is set to PDB70 where the UUID only 
>> cont

[Lldb-commits] [lldb] r357491 - Fix buildbot where paths were not matching up.

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Tue Apr  2 09:36:34 2019
New Revision: 357491

URL: http://llvm.org/viewvc/llvm-project?rev=357491&view=rev
Log:
Fix buildbot where paths were not matching up.


Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357491&r1=357490&r2=357491&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Tue Apr  2 09:36:34 2019
@@ -30,7 +30,10 @@ class MiniDumpUUIDTestCase(TestBase):
 
 def verify_module(self, module, verify_path, verify_uuid):
 uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
+fullpath = module.GetFileSpec().fullpath
+msg = 'Verify path ("%s") is contained in the fullpath ("%s")' % (
+verify_path, fullpath)
+self.assertTrue(verify_path in fullpath, msg)
 self.assertEqual(verify_uuid, uuid)
 
 def test_zero_uuid_modules(self):


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

[Lldb-commits] [lldb] r357495 - Fix typo; NFC

[Aaron Smith via lldb-commits]

Author: asmith
Date: Tue Apr  2 10:02:21 2019
New Revision: 357495

URL: http://llvm.org/viewvc/llvm-project?rev=357495&view=rev
Log:
Fix typo; NFC

Modified:
lldb/trunk/include/lldb/Target/Platform.h

Modified: lldb/trunk/include/lldb/Target/Platform.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Platform.h?rev=357495&r1=357494&r2=357495&view=diff
==
--- lldb/trunk/include/lldb/Target/Platform.h (original)
+++ lldb/trunk/include/lldb/Target/Platform.h Tue Apr  2 10:02:21 2019
@@ -596,7 +596,7 @@ public:
   virtual uint64_t WriteFile(lldb::user_id_t fd, uint64_t offset,
  const void *src, uint64_t src_len, Status &error) 
{
 error.SetErrorStringWithFormat(
-"Platform::ReadFile() is not supported in the %s platform",
+"Platform::WriteFile() is not supported in the %s platform",
 GetName().GetCString());
 return -1;
   }


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

[Lldb-commits] [lldb] r357496 - [lldb-server] Use llgs namespace to avoid conflicts with Win32 API

[Aaron Smith via lldb-commits]

Author: asmith
Date: Tue Apr  2 10:10:12 2019
New Revision: 357496

URL: http://llvm.org/viewvc/llvm-project?rev=357496&view=rev
Log:
[lldb-server] Use llgs namespace to avoid conflicts with Win32 API

Modified:
lldb/trunk/tools/lldb-server/lldb-server.cpp

Modified: lldb/trunk/tools/lldb-server/lldb-server.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-server.cpp?rev=357496&r1=357495&r2=357496&view=diff
==
--- lldb/trunk/tools/lldb-server/lldb-server.cpp (original)
+++ lldb/trunk/tools/lldb-server/lldb-server.cpp Tue Apr  2 10:10:12 2019
@@ -36,6 +36,7 @@ static void display_usage(const char *pr
 int main_gdbserver(int argc, char *argv[]);
 int main_platform(int argc, char *argv[]);
 
+namespace llgs {
 static void initialize() {
   if (auto e = g_debugger_lifetime->Initialize(
   llvm::make_unique(), nullptr))
@@ -43,6 +44,7 @@ static void initialize() {
 }
 
 static void terminate() { g_debugger_lifetime->Terminate(); }
+} // namespace llgs
 
 //--
 // main
@@ -61,14 +63,14 @@ int main(int argc, char *argv[]) {
 
   switch (argv[1][0]) {
   case 'g':
-initialize();
+llgs::initialize();
 main_gdbserver(argc, argv);
-terminate();
+llgs::terminate();
 break;
   case 'p':
-initialize();
+llgs::initialize();
 main_platform(argc, argv);
-terminate();
+llgs::terminate();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());


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

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for lldb-server on Windows

[Aaron Smith via Phabricator via lldb-commits]

asmith marked 5 inline comments as done.
asmith added a comment.

Committed a few NFC changes


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56233/new/

https://reviews.llvm.org/D56233



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

[Lldb-commits] [lldb] r357504 - Clean up windows build bot.

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Tue Apr  2 10:50:08 2019
New Revision: 357504

URL: http://llvm.org/viewvc/llvm-project?rev=357504&view=rev
Log:
Clean up windows build bot. 

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357504&r1=357503&r2=357504&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Tue Apr  2 10:50:08 2019
@@ -137,6 +137,7 @@ class MiniDumpUUIDTestCase(TestBase):
 self.verify_module(modules[0], "/not/exist/a", None)
 self.verify_module(modules[1], "/not/exist/b", None)
 
+@skipIf(oslist=['windows'])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each
@@ -163,6 +164,7 @@ class MiniDumpUUIDTestCase(TestBase):
"libuuidmatch.so", 
"7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")
 
+@skipIf(oslist=['windows'])
 def test_partial_uuid_mismatch(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each


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

[Lldb-commits] [lldb] r357507 - [Reproducers] Print warning when generating the reproducer.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Apr  2 11:23:16 2019
New Revision: 357507

URL: http://llvm.org/viewvc/llvm-project?rev=357507&view=rev
Log:
[Reproducers] Print warning when generating the reproducer.

Encourage users to look at the directory so they know what data they'd
be sharing by uploading the reproducer.

Modified:
lldb/trunk/source/Commands/CommandObjectReproducer.cpp

Modified: lldb/trunk/source/Commands/CommandObjectReproducer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectReproducer.cpp?rev=357507&r1=357506&r2=357507&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectReproducer.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectReproducer.cpp Tue Apr  2 11:23:16 
2019
@@ -49,6 +49,9 @@ protected:
 
 result.GetOutputStream()
 << "Reproducer written to '" << r.GetReproducerPath() << "'\n";
+result.GetOutputStream()
+<< "Please have a look at the directory to assess if you're willing to 
"
+   "share the contained information.\n";
 
 result.SetStatus(eReturnStatusSuccessFinishResult);
 return result.Succeeded();


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

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Since this commit, TestMiniDumpUUID_py is failing on green dragon.

Could you please revert the change or commit a fix?

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastCompletedBuild/testReport/lldb-Suite/functionalities_postmortem_minidump-new/TestMiniDumpUUID_py/


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60001/new/

https://reviews.llvm.org/D60001



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

[Lldb-commits] [lldb] r357513 - [NativePDB] Don't fail on import modules.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Tue Apr  2 12:39:45 2019
New Revision: 357513

URL: http://llvm.org/viewvc/llvm-project?rev=357513&view=rev
Log:
[NativePDB] Don't fail on import modules.

A recent patch to LLD started emitting information about import modules.
These are represented as compile units in the PDB, but with no
additional debug info.  This was confusing the native pdb reader, who
expected that the debug info stream be present.

This should fix failing tests on the Windows bots.

Modified:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp?rev=357513&r1=357512&r2=357513&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp Tue Apr 
 2 12:39:45 2019
@@ -124,11 +124,20 @@ CompilandIndexItem &CompileUnitIndex::Ge
   uint16_t stream = descriptor.getModuleStreamIndex();
   std::unique_ptr stream_data =
   m_index.pdb().createIndexedStream(stream);
+
+
+  std::unique_ptr& cci = result.first->second;
+
+  if (!stream_data) {
+llvm::pdb::ModuleDebugStreamRef debug_stream(descriptor, nullptr);
+cci = llvm::make_unique(PdbCompilandId{ modi }, 
debug_stream, std::move(descriptor));
+return *cci;
+  }
+
   llvm::pdb::ModuleDebugStreamRef debug_stream(descriptor,
std::move(stream_data));
-  cantFail(debug_stream.reload());
 
-  std::unique_ptr &cci = result.first->second;
+  cantFail(debug_stream.reload());
 
   cci = llvm::make_unique(
   PdbCompilandId{modi}, std::move(debug_stream), std::move(descriptor));


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

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth created this revision.
amccarth added reviewers: zturner, labath, rnk.

Spotted some problems in the Driver's PrepareCommandsForSourcing while
helping a colleague track another problem.

1. One error case was not handled because there was no else clause.

Fixed by switching to llvm's early-out style instead of nested
`if (succes) { } else { }` cases.  This keeps error handling close
to the actual error.

2. It used `fdopen` on Windows, which is a deprecated name.  In other

places the function used `#ifdef _WIN32` to use the proper names
for Windows and Posix.  For readability and consistency, I factored
these out into small static functions.

3. One call-site failed to call the clean-up function.  I solved this

by simplifying the API.  PrepareCommandsForSourcing no longer requires
the caller to provide a buffer for the pipe's file descriptors and to
call a separate clean-up function later.  PrepareCommandsForSourcing
now ensures the file descriptors are handled before returning.
(The read end of the pipe is held open by the returned FILE * as
before.)

I also eliminated an unnecessary local, shorted the lifetime of another,
and tried to improve the comments.


https://reviews.llvm.org/D60152

Files:
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -469,85 +469,79 @@
   return error;
 }
 
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size, int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
+static inline int OpenPipe(int fds[2], std::size_t size) {
+#ifdef _WIN32
+  return _pipe(fds, size, O_BINARY);
+#else
+  (void) size;
+  return pipe(fds);
+#endif
+}
+
 
-  ::FILE *commands_file = nullptr;
-  fds[0] = -1;
-  fds[1] = -1;
-  int err = 0;
+static inline ::FILE *OpenFileDescriptor(int fd, const char *mode) {
 #ifdef _WIN32
-  err = _pipe(fds, commands_size, O_BINARY);
+  return _fdopen(fd, mode);
 #else
-  err = pipe(fds);
+  return fdopen(fd, mode);
 #endif
-  if (err == 0) {
-ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-if (nrwr < 0) {
-  WithColor::error()
-  << format(
- "write(%i, %p, %" PRIu64
- ") failed (errno = %i) when trying to open LLDB commands pipe",
- fds[WRITE], static_cast(commands_data),
- static_cast(commands_size), errno)
-  << '\n';
-} else if (static_cast(nrwr) == commands_size) {
-// Close the write end of the pipe so when we give the read end to
-// the debugger/command interpreter it will exit when it consumes all
-// of the data
+}
+
+static inline int CloseFileDescriptor(int &fd) {
+  int result =
 #ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
+  _close(fd);
 #else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
+  close(fd);
 #endif
-  // Now open the read file descriptor in a FILE * that we can give to
-  // the debugger as an input handle
-  commands_file = fdopen(fds[READ], "r");
-  if (commands_file != nullptr) {
-fds[READ] = -1; // The FILE * 'commands_file' now owns the read
-// descriptor Hand ownership if the FILE * over to the
-// debugger for "commands_file".
-  } else {
-WithColor::error() << format("fdopen(%i, \"r\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-  }
-}
-  } else {
+  fd = -1;
+  return result;
+}
+
+static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
+  size_t commands_size) {
+  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+  int fds[2] = {-1, -1};
+
+  if (OpenPipe(fds, commands_size) != 0) {
 WithColor::error()
 << "can't create pipe file descriptors for LLDB commands\n";
+return nullptr;
   }
 
-  return commands_file;
-}
+  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
+  if (nrwr != commands_size) {
+WithColor::error()
+<< format(
+"write(%i, %p, %" PRIu64
+") failed (errno = %i) when trying to open LLDB commands pipe",
+fds[WRITE], static_cast(commands_data),
+static_cast(commands_size), errno)
+<< '\n';
+CloseFileDescriptor(fds[READ]);
+CloseFileDescriptor(fds[WRITE]);
+return nullptr;
+  }
 
-void CleanupAfterCommandSourcing(int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
+  // Close the write end of the pipe, so that the command interpreter will exit
+  // when it consumes all the data.
+  CloseFileDescriptor(fds[WRITE]);
 
-  // Close any pi

[Lldb-commits] [PATCH] D60153: Re-enable most lldb-vscode tests on Linux.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: zturner, clayborg.
Herald added a project: LLDB.

After https://reviews.llvm.org/D59828 and https://reviews.llvm.org/D59849,
I believe the problems with these tests hanging have been solved.

I tried enabling all of them on my machine, and got two failures:

- One of them was spawning a child process that lives for 5 seconds, waited for 
5 seconds to attach to the child, and failed because the child wasn't there.

- The other one was a legit failure because shell expansion of arguments 
doesn't work on Linux.

This tests enables all lldb-vscode tests on Linux except for "launch process
with shell expansion of args" (which doesn't work), and fixes the other broken
test by reducing the time it waits before attaching to its child process.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60153

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py

Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -21,7 +21,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_default(self):
 '''
@@ -41,7 +40,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_stopOnEntry(self):
 '''
@@ -63,7 +61,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_cwd(self):
 '''
@@ -92,7 +89,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_debuggerRoot(self):
 '''
@@ -122,7 +118,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_sourcePath(self):
 '''
@@ -150,7 +145,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_disableSTDIO(self):
 '''
@@ -167,7 +161,7 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
+@skipIfLinux # shell argument expansion doesn't seem to work on Linux
 @expectedFailureNetBSD
 @no_debug_info_test
 def test_shellExpandArguments_enabled(self):
@@ -194,7 +188,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_shellExpandArguments_disabled(self):
 '''
@@ -222,7 +215,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_args(self):
 '''
@@ -250,7 +242,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_environment(self):
 '''
@@ -285,7 +276,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_commands(self):
 '''
Index: lldb/packages/Python/lldbsuite/test/tools/ll

[Lldb-commits] [lldb] r357534 - Revert r357504, r357491, r357482 because of bot breakage.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Apr  2 15:03:22 2019
New Revision: 357534

URL: http://llvm.org/viewvc/llvm-project?rev=357534&view=rev
Log:
Revert r357504, r357491, r357482 because of bot breakage.

See discussion in https://reviews.llvm.org/D60001.

Revert Clean up windows build bot.
This reverts r357504 (git commit 380c2420ecb0c3e809b04f385d37b89800df1ecf)
Revert Fix buildbot where paths were not matching up.
This reverts r357491 (git commit 5050586860140b55a0cc68c77dd1438f44a23ca5)
Revert Allow partial UUID matching in Minidump core file plug-in
This reverts r357482 (git commit 838bba9c34bf1e5500c2e100327bc764afc8d367)

Removed:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357534&r1=357533&r2=357534&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Tue Apr  2 15:03:22 2019
@@ -8,7 +8,6 @@ from six import iteritems
 import shutil
 
 import lldb
-import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -30,10 +29,7 @@ class MiniDumpUUIDTestCase(TestBase):
 
 def verify_module(self, module, verify_path, verify_uuid):
 uuid = module.GetUUIDString()
-fullpath = module.GetFileSpec().fullpath
-msg = 'Verify path ("%s") is contained in the fullpath ("%s")' % (
-verify_path, fullpath)
-self.assertTrue(verify_path in fullpath, msg)
+self.assertEqual(verify_path, module.GetFileSpec().fullpath)
 self.assertEqual(verify_uuid, uuid)
 
 def test_zero_uuid_modules(self):
@@ -136,56 +132,3 @@ class MiniDumpUUIDTestCase(TestBase):
 self.assertEqual(2, len(modules))
 self.verify_module(modules[0], "/not/exist/a", None)
 self.verify_module(modules[1], "/not/exist/b", None)
-
-@skipIf(oslist=['windows'])
-def test_partial_uuid_match(self):
-"""
-Breakpad has been known to create minidump files using CvRecord in 
each
-module whose signature is set to PDB70 where the UUID only 
contains the
-first 16 bytes of a 20 byte ELF build ID. Code was added to 
-ProcessMinidump.cpp to deal with this and allows partial UUID 
matching. 
-
-This test verifies that if we have a minidump with a 16 byte UUID, 
that
-we are able to associate a symbol file with a 20 byte UUID only if 
the
-first 16 bytes match. In this case we will see the path from the 
file
-we found in the test directory and the 20 byte UUID from the actual
-file, not the 16 byte shortened UUID from the minidump.
-"""
-so_path = self.getBuildArtifact("libuuidmatch.so")
-self.yaml2obj("libuuidmatch.yaml", so_path)
-self.dbg.CreateTarget(None)
-self.target = self.dbg.GetSelectedTarget()
-cmd = 'settings set target.exec-search-paths "%s"' % 
(os.path.dirname(so_path))
-self.dbg.HandleCommand(cmd)
-self.process = 
self.target.LoadCore("linux-arm-partial-uuids-match.dmp")
-modules = self.target.modules
-self.assertEqual(1, len(modules))
-self.verify_module(modules[0],
-   "libuuidmatch.so", 
-   "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")
-
-@skipIf(oslist=['windows'])
-def test_partial_uuid_mismatch(self):
-"""
-Breakpad has been known to create minidump files using CvRecord in 
each
-module whose signature is set to PDB70 where the UUID only 
contains the
-first 16 bytes of a 20 byte ELF build ID. Code was added to 
-ProcessMinidump.cpp to deal with this and allows partial UUID 
matching. 
-
-This test verifies that if we have a minidump with a 16 byte UUID, 
that
-we are not able to associate a symbol file with a 20 byte UUID 
only if
-any of

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

The bots were broken for more than six hours so I took the liberty to revert 
the change in r357534. Sorry for the inconvenience!


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60001/new/

https://reviews.llvm.org/D60001



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Hey Tatyana, what's the plan with regards to testing this?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55718/new/

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via lldb-commits]

rnk updated this revision to Diff 193388.
rnk marked 4 inline comments as done.
rnk added a comment.

- Add RecordPrefix ctor, remove all dummy RecordLen assignments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60018/new/

https://reviews.llvm.org/D60018

Files:
  lld/COFF/PDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
  llvm/include/llvm/DebugInfo/CodeView/RecordSerialization.h
  llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h
  llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
  llvm/lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
  llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
  llvm/lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp
  llvm/lib/DebugInfo/CodeView/GlobalTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp
  llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
  llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
  llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
  llvm/lib/DebugInfo/CodeView/TypeTableCollection.cpp
  llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
  llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
  llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
  llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

Index: llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
===
--- llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
+++ llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
@@ -42,8 +42,6 @@
 }
 
 inline bool operator==(const CVType &R1, const CVType &R2) {
-  if (R1.Type != R2.Type)
-return false;
   if (R1.RecordData != R2.RecordData)
 return false;
   return true;
@@ -107,7 +105,7 @@
   GlobalState->Records.push_back(AR);
   GlobalState->Indices.push_back(Builder.writeLeafType(AR));
 
-  CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+  CVType Type(Builder.records().back());
   GlobalState->TypeVector.push_back(Type);
 
   GlobalState->AllOffsets.push_back(
@@ -369,11 +367,10 @@
   TypeIndex IndexOne = Builder.writeLeafType(Modifier);
 
   // set up a type stream that refers to the above two serialized records.
-  std::vector TypeArray;
-  TypeArray.push_back(
-  CVType(static_cast(Class.Kind), Builder.records()[0]));
-  TypeArray.push_back(
-  CVType(static_cast(Modifier.Kind), Builder.records()[1]));
+  std::vector TypeArray = {
+  {Builder.records()[0]},
+  {Builder.records()[1]},
+  };
   BinaryItemStream ItemStream(llvm::support::little);
   ItemStream.setItems(TypeArray);
   VarStreamArray TypeStream(ItemStream);
Index: llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -224,7 +224,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}",
fmt_align(Index, AlignStyle::Right, Width),
-   formatTypeLeafKind(Record.Type), Record.length());
+   formatTypeLeafKind(Record.kind()), Record.length());
   if (Hashes) {
 std::string H;
 if (Index.toArrayIndex() >= HashValues.size()) {
Index: llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
@@ -331,7 +331,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}]",
fmt_align(Offset, AlignStyle::Right, 6),
-   formatSymbolKind(Record.Type), Record.length());
+   formatSymbolKind(Record.kind()), Record.length());
   P.Indent();
   return Error::success();
 }
Index: llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -98,7 +98,7 @@
 
   CVType toCodeViewRecord(AppendingTypeTableBuilder &TS) const override {
 TS.writeLeafType(Record);
-return CVType(Kind, TS.records().back());
+return CVType(TS.records().back());
   }
 
   mutable T Record;
@@ -496,7 +496,7 @@
 Member.Member->writeTo(CRB);
   }
   TS.insertRecord(CRB);
-  return CVType(Kind, TS.records().back());
+  return CVType(TS.records().back());
 }
 
 void MappingTraits::mapping(IO &io, OneMethodRecord &Record) {
Index: llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
@@ -248,7

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via lldb-commits]

rnk updated this revision to Diff 193390.
rnk marked an inline comment as done.
rnk added a comment.

- one more change


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60018/new/

https://reviews.llvm.org/D60018

Files:
  lld/COFF/PDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
  llvm/include/llvm/DebugInfo/CodeView/RecordSerialization.h
  llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h
  llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
  llvm/lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
  llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
  llvm/lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp
  llvm/lib/DebugInfo/CodeView/GlobalTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp
  llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
  llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
  llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
  llvm/lib/DebugInfo/CodeView/TypeTableCollection.cpp
  llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
  llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
  llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
  llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

Index: llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
===
--- llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
+++ llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
@@ -42,8 +42,6 @@
 }
 
 inline bool operator==(const CVType &R1, const CVType &R2) {
-  if (R1.Type != R2.Type)
-return false;
   if (R1.RecordData != R2.RecordData)
 return false;
   return true;
@@ -107,7 +105,7 @@
   GlobalState->Records.push_back(AR);
   GlobalState->Indices.push_back(Builder.writeLeafType(AR));
 
-  CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+  CVType Type(Builder.records().back());
   GlobalState->TypeVector.push_back(Type);
 
   GlobalState->AllOffsets.push_back(
@@ -369,11 +367,10 @@
   TypeIndex IndexOne = Builder.writeLeafType(Modifier);
 
   // set up a type stream that refers to the above two serialized records.
-  std::vector TypeArray;
-  TypeArray.push_back(
-  CVType(static_cast(Class.Kind), Builder.records()[0]));
-  TypeArray.push_back(
-  CVType(static_cast(Modifier.Kind), Builder.records()[1]));
+  std::vector TypeArray = {
+  {Builder.records()[0]},
+  {Builder.records()[1]},
+  };
   BinaryItemStream ItemStream(llvm::support::little);
   ItemStream.setItems(TypeArray);
   VarStreamArray TypeStream(ItemStream);
Index: llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -224,7 +224,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}",
fmt_align(Index, AlignStyle::Right, Width),
-   formatTypeLeafKind(Record.Type), Record.length());
+   formatTypeLeafKind(Record.kind()), Record.length());
   if (Hashes) {
 std::string H;
 if (Index.toArrayIndex() >= HashValues.size()) {
Index: llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
@@ -331,7 +331,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}]",
fmt_align(Offset, AlignStyle::Right, 6),
-   formatSymbolKind(Record.Type), Record.length());
+   formatSymbolKind(Record.kind()), Record.length());
   P.Indent();
   return Error::success();
 }
Index: llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -98,7 +98,7 @@
 
   CVType toCodeViewRecord(AppendingTypeTableBuilder &TS) const override {
 TS.writeLeafType(Record);
-return CVType(Kind, TS.records().back());
+return CVType(TS.records().back());
   }
 
   mutable T Record;
@@ -496,7 +496,7 @@
 Member.Member->writeTo(CRB);
   }
   TS.insertRecord(CRB);
-  return CVType(Kind, TS.records().back());
+  return CVType(TS.records().back());
 }
 
 void MappingTraits::mapping(IO &io, OneMethodRecord &Record) {
Index: llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
@@ -248,7 +248,7 @@
 uint8_t *Buffer = Allocator.Al

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via lldb-commits]

rnk added inline comments.



Comment at: llvm/include/llvm/DebugInfo/CodeView/CVRecord.h:29
+/// Carrying the size separately instead of trusting the size stored in the
+/// record prefix provides some extra safety and flexibility.
 template  class CVRecord {

aganea wrote:
> aganea wrote:
> > To add to what you said in a comment above, do you think that if we could 
> > add `assert(Data.size() == ((RecordPrefix 
> > *)RecordData.data())->RecordPrefix + 2)` at relevant places below; and then 
> > after a while we could simply switch to `RecordPrefix *`, once issues are 
> > ironed out?
> I didn't mean now.
Eventually, yes, I think it's possible.



Comment at: llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h:56
+RecordPrefix *Prefix = reinterpret_cast(&PreBytes[0]);
+Prefix->RecordLen = 0;
+Prefix->RecordKind = uint16_t(Sym.Kind);

aganea wrote:
> rnk wrote:
> > aganea wrote:
> > > Should be 2.
> > I could say 2, but this is the seralizer, so really it just gets 
> > overwritten. Do you think I should leave it uninitialized, 2, -1, 0?
> I'd say "2" because you want as much as possible to keep data coherent in 
> time. Regardless of what comes after. I think the code should be correct 
> before being optimal. If someone looks for `RecordPrefix` and then tries to 
> understand how it works, it'd be nice if the code displays the same, correct, 
> behavior everywhere.
> 
> But then, you can argue that `RecordPrefix.RecordLen` is simply a constrait, 
> tied to the amount data that comes after. And maybe the calculation logic for 
> that `.RecordLen` field should be hidden inside the `RecordPrefix` class. In 
> that case, to avoid screwing the init order, ideally you could simply say:
> ```
> RecordPrefix Prefix{uint16_t(Sym.Kind)};
> CVSymbol Result(&Prefix);
> ```
> ...and let `RecordPrefix` (or `CVSymbol`) handle sizing?
Makes sense. I'll try to standardize on 2, and I like the logic that the data 
becomes trustworthy.



Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:40
+  static CVSymbol Tombstone(
+  ArrayRef(DenseMapInfo::getTombstoneKey(), 1));
   return Tombstone;

aganea wrote:
> I suppose you've chosen 1 because that's a invalid record size? Comment maybe?
Actually, the reason I did this was to avoid ambiguity between the `T* begin, 
T* end` ctor and the `T* base, size_t count` ctor. If I use zero, it's 
ambiguous. Implicit null strikes again. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60018/new/

https://reviews.llvm.org/D60018



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

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Alexandre Ganea via Phabricator via lldb-commits]

aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

LGTM. With a few minor comments:




Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h:122
   ~FieldListDeserializer() override {
-CVType FieldList;
-FieldList.Type = TypeLeafKind::LF_FIELDLIST;
+RecordPrefix Pre;
+Pre.RecordLen = 2;

`RecordPrefix Pre(TypeLeafKind::LF_FIELDLIST);` like the other places? And 
above.



Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:40
+  static CVSymbol Tombstone(
+  ArrayRef(DenseMapInfo::getTombstoneKey(), 1));
   return Tombstone;

rnk wrote:
> aganea wrote:
> > I suppose you've chosen 1 because that's a invalid record size? Comment 
> > maybe?
> Actually, the reason I did this was to avoid ambiguity between the `T* begin, 
> T* end` ctor and the `T* base, size_t count` ctor. If I use zero, it's 
> ambiguous. Implicit null strikes again. :)
Oh I see. Why not `static CVSymbol 
Tombstone(DenseMapInfo>::getTombstoneKey());` then?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60018/new/

https://reviews.llvm.org/D60018



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

[Lldb-commits] [lldb] r357553 - Avoid macro redefinition error if HAVE_LIBCOMPRESSION

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Apr  2 18:16:54 2019
New Revision: 357553

URL: http://llvm.org/viewvc/llvm-project?rev=357553&view=rev
Log:
Avoid macro redefinition error if HAVE_LIBCOMPRESSION
is already defined.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=357553&r1=357552&r2=357553&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Tue Apr  2 18:16:54 2019
@@ -37,7 +37,9 @@
 #include "llvm/ADT/StringSwitch.h"
 
 #if defined(__APPLE__)
+#ifndef HAVE_LIBCOMPRESSION
 #define HAVE_LIBCOMPRESSION
+#endif
 #include 
 #endif
 


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

[Lldb-commits] [lldb] r357554 - [lldb-dotest] Print dotest.py invocation.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Apr  2 18:26:38 2019
New Revision: 357554

URL: http://llvm.org/viewvc/llvm-project?rev=357554&view=rev
Log:
[lldb-dotest] Print dotest.py invocation.

In order to debug a failing python test, you need to debug Python
instead of the wrapper. For a while I've been adding and removing this,
but I think it could be useful for everyone.

Modified:
lldb/trunk/utils/lldb-dotest/lldb-dotest.in

Modified: lldb/trunk/utils/lldb-dotest/lldb-dotest.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/utils/lldb-dotest/lldb-dotest.in?rev=357554&r1=357553&r2=357554&view=diff
==
--- lldb/trunk/utils/lldb-dotest/lldb-dotest.in (original)
+++ lldb/trunk/utils/lldb-dotest/lldb-dotest.in Tue Apr  2 18:26:38 2019
@@ -13,4 +13,5 @@ if __name__ == '__main__':
 cmd.extend(dotest_args)
 cmd.extend(wrapper_args)
 # Invoke dotest.py and return exit code.
+print(' '.join(cmd))
 sys.exit(subprocess.call(cmd))


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

[Lldb-commits] [lldb] r357555 - [lit] Use 10 minute timeout by default.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Apr  2 18:26:41 2019
New Revision: 357555

URL: http://llvm.org/viewvc/llvm-project?rev=357555&view=rev
Log:
[lit] Use 10 minute timeout by default.

Lit has the ability to set a timeout for individual tests. This patch
enables that functionality with a default of 10 minutes.

Currently we rely on the bots to kill the whole test suite. However this
doesn't tell us which test caused the timeout. Furthermore, when running
the test suite during development, I have to manually kill the tests
that time out to get the lit output at then end. This fixes both
inconveniences.

Modified:
lldb/trunk/lit/lit.site.cfg.py.in

Modified: lldb/trunk/lit/lit.site.cfg.py.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.site.cfg.py.in?rev=357555&r1=357554&r2=357555&view=diff
==
--- lldb/trunk/lit/lit.site.cfg.py.in (original)
+++ lldb/trunk/lit/lit.site.cfg.py.in Tue Apr  2 18:26:41 2019
@@ -18,6 +18,7 @@ config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
+config.maxIndividualTestTime = 600
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.


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

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: clayborg, jingham.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

I'm addressing a perf issue where DynamicLoaderDarwin has been notified that a 
batch of solibs have been loaded.  It adds them to the target one-by-one with 
Target::GetSharedModule(), and then calls Target::ModulesDidLoad() to give 
breakpoints a chance to evaluate for new locations, etc.  One of the things we 
do on ModulesDidLoad is call ProcessGDBRemote::ModulesDidLoad which will send 
the qSymbol packet to the gdb remote serial protocol stub if it indicates it 
wants to know a symbol address.  Currently, because Target::GetSharedModule() 
calls ModulesDidLoad, we will send the qSymbol packet many times - an app with 
hundreds of solibs are not unusual.

This patch renames Target::GetSharedModule to Target::AddModule() which better 
reflects what it actually does -- given a ModuleSpec set of criteria, it finds 
that binary on the local system and adds it to the Target, if it isn't already 
present.  This method name has confused all of us at one point or another.  As 
DynamicLoaderWindowsDYLD notes,

  // Confusingly, there is no Target::AddSharedModule.  Instead, calling
  // GetSharedModule() with a new module will add it to the module list and
  // return a corresponding ModuleSP.

It adds a flag to Target::AddModule to suppress notification of the new Module 
(i.e. don't call ModulesDidLoad) - if the caller does this, the caller must 
call Target::ModulesDidLoad before resuming execution.  I added a description 
of the method in Target.h to make this clear.

I also had to add flag to ModuleList::Append and ModuleList::AppendIfNeeded.  I 
made these have default values because many uses of this are in a loop creating 
a standalone ModuleList, and forcing all of them to specify the notify boolean 
was nonintuitive for those users.  When a ModuleList is a part of the Target, 
it has notifier callback functions, but when it is a standalone object, it 
doesn't.

I'm trying to think of how to test this -- but the problem I'm directly trying 
to address, the qSymbol packet being sent for every solib, instead of the # of 
groups of solib's that are loaded, is something we don't track today.  I'll 
continue to think about it.

rdar://problem/48293064


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D60172

Files:
  include/lldb/Core/ModuleList.h
  include/lldb/Target/Target.h
  source/API/SBTarget.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/DynamicLoader.cpp
  source/Core/ModuleList.cpp
  source/Expression/FunctionCaller.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -1442,7 +1442,8 @@
"Target::SetExecutableModule (executable = '%s')",
executable_sp->GetFileSpec().GetPath().c_str());
 
-m_images.Append(executable_sp); // The first image is our executable file
+const bool notify = true;
+m_images.Append(executable_sp, notify); // The first image is our executable file
 
 // If we haven't set an architecture yet, reset our architecture based on
 // what we found in the executable module.
@@ -1482,7 +1483,8 @@
   platform_dependent_file_spec = dependent_file_spec;
 
 ModuleSpec module_spec(platform_dependent_file_spec, m_arch.GetSpec());
-ModuleSP image_module_sp(GetSharedModule(module_spec));
+const bool notify = true;
+ModuleSP image_module_sp(AddModule(module_spec, notify));
 if (image_module_sp) {
   ObjectFile *objfile = image_module_sp->GetObjectFile();
   if (objfile)
@@ -1984,8 +1986,8 @@
   return false;
 }
 
-ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec,
- Status *error_ptr) {
+ModuleSP Target::AddModule(const ModuleSpec &module_spec, bool notify,
+   Status *error_ptr) {
   ModuleSP module_sp;
 
   Status error;
@@ -2118,8 +2120,9 @@
   Module *old_module_ptr = old_module_sp.get();
   old_module_sp.reset();
   ModuleList::RemoveSharedModuleIfOrphaned(old_module_ptr);
-} else
-  m_images.Append(module_sp);
+} else {
+  m_images.Append(module_sp, notify);
+}
   } else
 module_sp.reset();
 }
Index:

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: include/lldb/Target/Target.h:535
+   bool notify,
+   Status *error_ptr = nullptr);
 

Pavel had questions about this error. If we specify an error when we call this, 
is there a way to get a valid module shared pointer back and still get an 
error? Maybe this should be one of the llvm::ErrorOr return types?



Comment at: source/Commands/CommandObjectTarget.cpp:399-400
+  const bool notify = true;
+  ModuleSP module_sp = target_sp->AddModule(main_module_spec,
+  notify);
   if (module_sp)

Remove all "const bool notify = true; "statements and Inline with comment?

```
ModuleSP module_sp = target_sp->AddModule(main_module_spec, 
  true /*notify*/);
```
This would apply everywhere in this patch if so.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60172/new/

https://reviews.llvm.org/D60172



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

Re: [Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via lldb-commits]

No worries. I tried to get things working with a few patches. Is there a way to 
try out a patch on green dragon other than just submitting the changes? 

> On Apr 2, 2019, at 3:03 PM, Adrian Prantl via Phabricator 
>  wrote:
> 
> aprantl added a comment.
> 
> The bots were broken for more than six hours so I took the liberty to revert 
> the change in r357534. Sorry for the inconvenience!
> 
> 
> Repository:
>  rL LLVM
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D60001/new/
> 
> https://reviews.llvm.org/D60001
> 
> 
> 

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

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda marked 2 inline comments as done.
jasonmolenda added a comment.

Thanks for looking the patch over.




Comment at: include/lldb/Target/Target.h:535
+   bool notify,
+   Status *error_ptr = nullptr);
 

clayborg wrote:
> Pavel had questions about this error. If we specify an error when we call 
> this, is there a way to get a valid module shared pointer back and still get 
> an error? Maybe this should be one of the llvm::ErrorOr return types?
There's only a handful of AddModule callers that pass in a Status object - the 
Windows ones, the CommandObjectTarget command, and the Minidump.  I'll look 
through the Platform GetSharedModules implementations tomorrow to see who might 
put useful things in the error object, but in general either AddModule finds a 
binary that matches the ModuleSpec, and returns a shared pointer to it, or 
finds nothing and returns an empty shared pointer.  The error object may have a 
message from a Platform indicating why the search failed ("could not find any 
SDK directories" or something) but I suspect this optional Status arg isn't 
doing anything.



Comment at: source/Commands/CommandObjectTarget.cpp:399-400
+  const bool notify = true;
+  ModuleSP module_sp = target_sp->AddModule(main_module_spec,
+  notify);
   if (module_sp)

clayborg wrote:
> Remove all "const bool notify = true; "statements and Inline with comment?
> 
> ```
> ModuleSP module_sp = target_sp->AddModule(main_module_spec, 
>   true /*notify*/);
> ```
> This would apply everywhere in this patch if so.
Can do.  I don't have a preference.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60172/new/

https://reviews.llvm.org/D60172



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

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, friss.
Herald added a reviewer: jfb.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

For some reason I had convinced myself that functions returning by pointer or 
reference do not require recording their result. However, after further 
considering I don't see how that could work, at least not with the current 
implementation. Interestingly enough, the reproducer instrumentation already 
(mostly) accounts for this, though the lldb-instr tool did not.

This patch adds the missing macros and updates the lldb-instr tool.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D60178

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBAddress.cpp
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBBlock.cpp
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointName.cpp
  lldb/source/API/SBBroadcaster.cpp
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/API/SBCompileUnit.cpp
  lldb/source/API/SBData.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBDeclaration.cpp
  lldb/source/API/SBError.cpp
  lldb/source/API/SBEvent.cpp
  lldb/source/API/SBExecutionContext.cpp
  lldb/source/API/SBExpressionOptions.cpp
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBFileSpecList.cpp
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBFunction.cpp
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/API/SBLineEntry.cpp
  lldb/source/API/SBListener.cpp
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBProcessInfo.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBSection.cpp
  lldb/source/API/SBSourceManager.cpp
  lldb/source/API/SBStringList.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBSymbol.cpp
  lldb/source/API/SBSymbolContext.cpp
  lldb/source/API/SBSymbolContextList.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/API/SBThread.cpp
  lldb/source/API/SBThreadCollection.cpp
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/API/SBType.cpp
  lldb/source/API/SBTypeCategory.cpp
  lldb/source/API/SBTypeEnumMember.cpp
  lldb/source/API/SBTypeFilter.cpp
  lldb/source/API/SBTypeFormat.cpp
  lldb/source/API/SBTypeNameSpecifier.cpp
  lldb/source/API/SBTypeSummary.cpp
  lldb/source/API/SBTypeSynthetic.cpp
  lldb/source/API/SBUnixSignals.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/API/SBVariablesOptions.cpp
  lldb/source/API/SBWatchpoint.cpp
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/tools/lldb-instr/Instrument.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -89,6 +89,8 @@
   /// {
   InstrumentedBar();
   InstrumentedFoo GetInstrumentedFoo();
+  InstrumentedFoo& GetInstrumentedFooRef();
+  InstrumentedFoo* GetInstrumentedFooPtr();
   void SetInstrumentedFoo(InstrumentedFoo *foo);
   void SetInstrumentedFoo(InstrumentedFoo &foo);
   void Validate();
@@ -201,6 +203,22 @@
   return LLDB_RECORD_RESULT(InstrumentedFoo(0));
 }
 
+InstrumentedFoo& InstrumentedBar::GetInstrumentedFooRef() {
+  LLDB_RECORD_METHOD_NO_ARGS(InstrumentedFoo&, InstrumentedBar,
+ GetInstrumentedFooRef);
+  InstrumentedFoo* foo = new InstrumentedFoo(0);
+  m_get_instrumend_foo_called = true;
+  return LLDB_RECORD_RESULT(*foo);
+}
+
+InstrumentedFoo* InstrumentedBar::GetInstrumentedFooPtr() {
+  LLDB_RECORD_METHOD_NO_ARGS(InstrumentedFoo*, InstrumentedBar,
+ GetInstrumentedFooPtr);
+  InstrumentedFoo* foo = new InstrumentedFoo(0);
+  m_get_instrumend_foo_called = true;
+  return LLDB_RECORD_RESULT(foo);
+}
+
 void InstrumentedBar::SetInstrumentedFoo(InstrumentedFoo *foo) {
   LLDB_RECORD_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
  (InstrumentedFoo *), foo);
@@ -239,6 +257,10 @@
   LLDB_REGISTER_CONSTRUCTOR(InstrumentedBar, ());
   LLDB_REGISTER_METHOD(InstrumentedFoo, InstrumentedBar, GetInstrumentedFoo,
());
+  LLDB_REGISTER_METHOD(InstrumentedFoo&, InstrumentedBar, GetInstrumentedFooRef,
+   ());
+  LLDB_REGISTER_METHOD(InstrumentedFoo*, InstrumentedBar, GetInstrumentedFooPtr,
+   ());
   LLDB_REGISTER_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
(InstrumentedFoo *));
   LLDB_REGISTER_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
@@ -487,6 +509,83 @@
   {
 InstrumentedBar bar;
 InstrumentedFoo foo = bar.GetInstrumentedF

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.
Herald added a subscriber: dexonsmith.



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:679
+  template  const Result &RecordResult(const Result &r) {
+UpdateBoundary();
+if (ShouldCapture()) {

Let's cast this or hide it behind a macro to prevent code duplication. 



Comment at: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp:552
+#if 0
+InstrumentedFoo* foo_ptr = bar.GetInstrumentedFooPtr();
+#endif

Oops, this should be removed.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60178/new/

https://reviews.llvm.org/D60178



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

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: compnerd, sgraenitz, labath.
Herald added subscribers: kristof.beyls, javed.absar, mgorny.
xiaobai added a reviewer: mgorny.

This line is unnecessary because add_llvm_executable will handle
linking the correct LLVM libraries for you. LLDB standalone builds are totally
find without this.

In the best case, having this line here is harmless. In the worst case it can
cause link issues.

If you build lldb-server for android using the standalone build, this line
will cause LLVM_LIBRARY_DIR to be the first place you look for libraries.
This is an issue because if you built libc++, it will try to link against
that one instead of the one from the android NDK.  Meanwhile, the LLVM libraries
you're linking against were linked against the libc++ from the NDK.

Ideally, we would take advantage of the AFTER option for link_directories(), but
that was not available in LLDB's minimum supported version of CMake (CMake 
3.4.3).


https://reviews.llvm.org/D60180

Files:
  cmake/modules/LLDBStandalone.cmake


Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -21,7 +21,6 @@
 
   set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR} CACHE PATH "Path to LLVM 
source tree")
   set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to 
llvm/include")
-  set(LLVM_LIBRARY_DIR ${LLVM_LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
   set(lit_file_name "llvm-lit")
@@ -95,8 +94,6 @@
 "${LLVM_INCLUDE_DIRS}"
 "${CLANG_INCLUDE_DIRS}")
 
-  link_directories("${LLVM_LIBRARY_DIR}")
-
   set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
   set(CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
   set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})


Index: cmake/modules/LLDBStandalone.cmake
===
--- cmake/modules/LLDBStandalone.cmake
+++ cmake/modules/LLDBStandalone.cmake
@@ -21,7 +21,6 @@
 
   set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
   set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to llvm/include")
-  set(LLVM_LIBRARY_DIR ${LLVM_LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
   set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
   set(lit_file_name "llvm-lit")
@@ -95,8 +94,6 @@
 "${LLVM_INCLUDE_DIRS}"
 "${CLANG_INCLUDE_DIRS}")
 
-  link_directories("${LLVM_LIBRARY_DIR}")
-
   set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
   set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
   set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits