date:20190401

[Lldb-commits] [lldb] r357376 - [Linux/x86] Fix writing of non-gpr registers on newer processors

2019-04-01 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Apr  1 01:11:46 2019
New Revision: 357376

URL: http://llvm.org/viewvc/llvm-project?rev=357376&view=rev
Log:
[Linux/x86] Fix writing of non-gpr registers on newer processors

Summary:
We're using ptrace(PTRACE_SETREGSET, NT_X86_XSTATE) to write all non-gpt
registers on x86 linux. Unfortunately, this method has a quirk, where
the kernel rejects all attempts to write to this area if one supplies a
buffer which is smaller than the area size (even though the kernel will
happily accept partial reads from it).

This means that if the CPU supports some new registers/extensions that
we don't know about (in my case it was the PKRU extension), we will fail
to write *any* non-gpr registers, even those that we know about.

Since this is a situation that's likely to appear again and again, I add
code to NativeRegisterContextLinux_x86_64 to detect the runtime size of
the area, and allocate an appropriate buffer. This does not mean that we
will start automatically supporting all new extensions, but it does mean
that the new extensions will not prevent the old ones from working.

This fixes tests attempting to write to non-gpr registers on new intel
processors (cca Kaby Lake Refresh).

Reviewers: jankratochvil, davezarzycki

Subscribers: lldb-commits

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

Modified:

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp?rev=357376&r1=357375&r2=357376&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
Mon Apr  1 01:11:46 2019
@@ -18,7 +18,7 @@
 
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
-
+#include 
 #include 
 
 using namespace lldb_private;
@@ -267,12 +267,29 @@ CreateRegisterInfoInterface(const ArchSp
   }
 }
 
+// Return the size of the XSTATE area supported on this cpu. It is necessary to
+// allocate the full size of the area even if we do not use/recognise all of it
+// because ptrace(PTRACE_SETREGSET, NT_X86_XSTATE) will refuse to write to it 
if
+// we do not pass it a buffer of sufficient size. The size is always at least
+// sizeof(FPR) so that the allocated buffer can be safely cast to FPR*.
+static std::size_t GetXSTATESize() {
+  unsigned int eax, ebx, ecx, edx;
+  // First check whether the XSTATE are is supported at all.
+  if (!__get_cpuid(1, &eax, &ebx, &ecx, &edx) || !(ecx & bit_XSAVE))
+return sizeof(FPR);
+
+  // Then fetch the maximum size of the area.
+  if (!__get_cpuid_count(0x0d, 0, &eax, &ebx, &ecx, &edx))
+return sizeof(FPR);
+  return std::max(ecx, sizeof(FPR));
+}
+
 NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextLinux(native_thread,
  CreateRegisterInfoInterface(target_arch)),
-  m_xstate_type(XStateType::Invalid), m_fpr(), m_iovec(), m_ymm_set(),
-  m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
+  m_xstate_type(XStateType::Invalid), m_ymm_set(), m_mpx_set(),
+  m_reg_info(), m_gpr_x86_64() {
   // Set up data about ranges of valid registers.
   switch (target_arch.GetMachine()) {
   case llvm::Triple::x86:
@@ -328,14 +345,13 @@ NativeRegisterContextLinux_x86_64::Nativ
 break;
   }
 
-  // Initialize m_iovec to point to the buffer and buffer size using the
-  // conventions of Berkeley style UIO structures, as required by PTRACE
-  // extensions.
-  m_iovec.iov_base = &m_fpr;
-  m_iovec.iov_len = sizeof(m_fpr);
+  std::size_t xstate_size = GetXSTATESize();
+  m_xstate.reset(static_cast(std::malloc(xstate_size)));
+  m_iovec.iov_base = m_xstate.get();
+  m_iovec.iov_len = xstate_size;
 
   // Clear out the FPR state.
-  ::memset(&m_fpr, 0, sizeof(m_fpr));
+  ::memset(m_xstate.get(), 0, xstate_size);
 
   // Store byte offset of fctrl (i.e. first register of FPR)
   const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
@@ -438,14 +454,17 @@ NativeRegisterContextLinux_x86_64::ReadR
 
 if (byte_order != lldb::eByteOrderInvalid) {
   if (reg >= m_reg_info.first_st && reg <= m_reg_info.last_st)
-reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_st].bytes,
-   reg_info->byte_size, byte_order);
+reg_value.SetBytes(
+m_xstate->fxsave.stmm[reg - m_reg_info.first_st].bytes,
+reg_info->byte_size, byte_order);
   if (reg >= m_reg_info.first_mm

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

2019-04-01 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357376: [Linux/x86] Fix writing of non-gpr registers on 
newer processors (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59991

Files:
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
@@ -108,7 +108,8 @@
 
   // Private member variables.
   mutable XStateType m_xstate_type;
-  FPR m_fpr; // Extended States Area, named FPR for historical reasons.
+  std::unique_ptr
+  m_xstate; // Extended States Area, named FPR for historical reasons.
   struct iovec m_iovec;
   YMM m_ymm_set;
   MPX m_mpx_set;
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -18,7 +18,7 @@
 
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
-
+#include 
 #include 
 
 using namespace lldb_private;
@@ -267,12 +267,29 @@
   }
 }
 
+// Return the size of the XSTATE area supported on this cpu. It is necessary to
+// allocate the full size of the area even if we do not use/recognise all of it
+// because ptrace(PTRACE_SETREGSET, NT_X86_XSTATE) will refuse to write to it if
+// we do not pass it a buffer of sufficient size. The size is always at least
+// sizeof(FPR) so that the allocated buffer can be safely cast to FPR*.
+static std::size_t GetXSTATESize() {
+  unsigned int eax, ebx, ecx, edx;
+  // First check whether the XSTATE are is supported at all.
+  if (!__get_cpuid(1, &eax, &ebx, &ecx, &edx) || !(ecx & bit_XSAVE))
+return sizeof(FPR);
+
+  // Then fetch the maximum size of the area.
+  if (!__get_cpuid_count(0x0d, 0, &eax, &ebx, &ecx, &edx))
+return sizeof(FPR);
+  return std::max(ecx, sizeof(FPR));
+}
+
 NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextLinux(native_thread,
  CreateRegisterInfoInterface(target_arch)),
-  m_xstate_type(XStateType::Invalid), m_fpr(), m_iovec(), m_ymm_set(),
-  m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
+  m_xstate_type(XStateType::Invalid), m_ymm_set(), m_mpx_set(),
+  m_reg_info(), m_gpr_x86_64() {
   // Set up data about ranges of valid registers.
   switch (target_arch.GetMachine()) {
   case llvm::Triple::x86:
@@ -328,14 +345,13 @@
 break;
   }
 
-  // Initialize m_iovec to point to the buffer and buffer size using the
-  // conventions of Berkeley style UIO structures, as required by PTRACE
-  // extensions.
-  m_iovec.iov_base = &m_fpr;
-  m_iovec.iov_len = sizeof(m_fpr);
+  std::size_t xstate_size = GetXSTATESize();
+  m_xstate.reset(static_cast(std::malloc(xstate_size)));
+  m_iovec.iov_base = m_xstate.get();
+  m_iovec.iov_len = xstate_size;
 
   // Clear out the FPR state.
-  ::memset(&m_fpr, 0, sizeof(m_fpr));
+  ::memset(m_xstate.get(), 0, xstate_size);
 
   // Store byte offset of fctrl (i.e. first register of FPR)
   const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
@@ -438,14 +454,17 @@
 
 if (byte_order != lldb::eByteOrderInvalid) {
   if (reg >= m_reg_info.first_st && reg <= m_reg_info.last_st)
-reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_st].bytes,
-   reg_info->byte_size, byte_order);
+reg_value.SetBytes(
+m_xstate->fxsave.stmm[reg - m_reg_info.first_st].bytes,
+reg_info->byte_size, byte_order);
   if (reg >= m_reg_info.first_mm && reg <= m_reg_info.last_mm)
-reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_mm].bytes,
-   reg_info->byte_size, byte_order);
+reg_value.SetBytes(
+m_xstate->fxsave.stmm[reg - m_reg_info.first_mm].bytes,
+reg_info->byte_size, byte_order);
   if (reg >= m_reg_info.first_xmm && reg <= m_reg_info.last_xmm)
-reg_value.SetBytes(m_fpr.fxsave.xmm[reg - m_reg_info.first_xmm].bytes,
-   reg_info->byte_size, byte_order);
+reg_value.SetBytes(
+m_xstate->fxsave.xmm[reg - m_reg_info.first_xmm].bytes,
+reg_info->byte_s

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

2019-04-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thank you for the quick review.

In D59991#1448852 , @davezarzycki 
wrote:

> Can we cherry-pick this into a release branch? Skylake (and newer) CPUs are 
> far from rare these days, especially on cloud hosting providers.

I think cherry-picking this would be a good idea (though I'd let it sit on the 
master branch for a bit of time first). However, I think the only release we 
can cherry-pick this to is 8.0.1, and that one is still a couple of months away 
(we've literally just wrapped 8.0). Nonetheless, I've filed a bug to do that 
https://bugs.llvm.org/show_bug.cgi?id=41330. If you're interested, you could 
also try to talk to the distro of your choice to see if they can roll out a 
quicker fix.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59991

___
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

2019-04-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.

Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:394
+if (error.Fail()) {
+  GetTarget().GetImages().Remove(module_sp);
+  module_sp.reset();

clayborg wrote:
> labath wrote:
> > Under what circumstances can `GetSharedModule` fail, and still produce a 
> > valid module_sp ?
> Sometimes you can create a module with a specification, but it won't have a 
> valid object file is my understanding.
Ok, but if that's the what the function is supposed to do in that case, then 
why should it return an error?

To me, this looks like a bug in that function (I don't see any justification 
for GetSharedModule to modify the module list of a target, but still return an 
error), and it should be fixed there (and not worked around here). Or, if you 
don't know how to trigger this condition, we can just drop this code here.

Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:397
+}
+if (!module_sp) {
+  // Try and find a module without specifying the UUID and only looking for

clayborg wrote:
> labath wrote:
> > I am wondering if there's any way to narrow down the scope of this so we 
> > don't do this second check for every module out there. If I understood the 
> > description correctly, it should be sufficient to do this for linux 
> > minidumps with PDB70 uuid records. Since PDBs aren't a thing on linux, and 
> > later breakpad versions used used ElfBuildId records, this should be a 
> > pretty exact match for the situation we want to capture. WDYT?
> > 
> > (Getting the platform of the minidump should be easy, for the UUID record 
> > type, we might need a small refactor of the MinidumpParser class.)
> We really do want this. Why? Because many times with minidumps you have to go 
> out and find the symbol files in some alternate location, put them in a 
> directory, then we want lldb to find them automatically. If you have a module 
> spec with a full path and no UUID, the path will need to match. If you just 
> specify a basename with no UUID, it allows us to use that file. Also with our 
> UUID story being a bit weak with ELF 20 bytes build IDs versus 4 bytes GNU 
> build IDs,. we might need to pull some tricks and strip out a UUID from a 
> binary so we can load it in LLDB. So this basename only search allows us to 
> use a symbol file with no UUID or if the minidump didn't contain a UUID and 
> somehow we knew where the right symbol file was. So this does catch my case, 
> but will also find any files in the exec search paths.
Ok, so I see that this patch does a bit more than it may seem at first glance. 
I think it would be good to enumerate the list of scenarios that you're 
interested in here, because there are multiple quirks of different components 
that we are working with here, and each may require a fix elsewhere. The 
scenarios I know about (but there may be others) are:
1. breakpad truncating long build-ids
2. LLDB using a a crc checksum as a build-id, when the real build-id is missing
3. breakpad using some other arbitrary checksum when a real build-id is missing

Of these, 1. and 3. are a problem in the producer, and we cannot do anything 
about that except work around them. Doing that close to the source (i.e. 
ProcessMinidump) is reasonable. However, 2. is an lldb problem (I would say 
even a "bug"), and we certainly can do something to fix it instead of just 
covering it out. So, if there's any way to cut complexity here by fixing elf 
UUID computation, I think we should do that instead.

Now, cases 1. and 3. do result in UUIDs that are incompatible with how we use 
UUIDs elsewhere in lldb, so working around this by retrying the search without 
a UUID and then doing some auxiliary matching seems like a reasonable 
compromise. However, it's not clear to me why you need to strip the path too. 
Based on my reading of ModuleList::GetSharedModule 
,
 the directory component is ignored when searching "exec-search-paths", and so 
we shouldn't need to do anything special to make this case work. And if that's 
the case (if it isn't, then I'd like to understand why), then it seems 
reasonable to restrict the UUID-less retry to the cases where we know breakpad 
might have generated incompatible UUIDs. Restricting it to `ElfBuildId`-records 
seems like a good first-order approximation, but if there are other clues we 
could use for that, I'd say we should use them too.

So I'd propose to split this patch into two (or more?) independent changes:
1. See what it takes to make exec-search-paths lookup work in the "normal" 
(i.e., the file has a regular build-id, and everybody agrees on what it is) 
case. I did some simple experiments, and it seemed to work for me without any 
special modifications. So if there's a case that doesn't work it could be a bug 
elsewhere that needs fixing.
2. im

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

2019-04-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

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.


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] D60068: PDBFPO: Refactor register reference resolution

2019-04-01 Thread Pavel Labath via Phabricator via lldb-commits

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

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.


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:

[Lldb-commits] [lldb] r357399 - [lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

2019-04-01 Thread Michal Gorny via lldb-commits

Author: mgorny
Date: Mon Apr  1 08:08:24 2019
New Revision: 357399

URL: http://llvm.org/viewvc/llvm-project?rev=357399&view=rev
Log:
[lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

Include support for NetBSD core dumps from evbarm/aarch64 system,
and matching test cases for them.

Based on earlier work by Kamil Rytarowski.

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

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64.core
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64.core
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_t2_SIGSEGV.aarch64
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_t2_SIGSEGV.aarch64.core
   (with props)
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/trunk/source/Plugins/Process/elf-core/RegisterUtilities.h
lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64?rev=357399&view=auto
==
Binary file - no diff available.

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64
--
svn:executable = *

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64
--
svn:mime-type = application/x-executable

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64.core
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64.core?rev=357399&view=auto
==
Binary file - no diff available.

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64.core
--
svn:mime-type = application/x-coredump

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64?rev=357399&view=auto
==
Binary file - no diff available.

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64
--
svn:executable = *

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64
--
svn:mime-type = application/x-executable

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64.core
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64.core?rev=357399&view=auto
==
Binary file - no diff available.

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64.core
--
svn:mime-type = application/x-coredump

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_t2_SIGSEGV.aarch64
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_t2_SIGSEGV.aarch64?rev=357399&view=auto
==
Binary file - no diff available.

Propchange: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/ne

[Lldb-commits] [PATCH] D60034: [lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

2019-04-01 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB357399: [lldb] [Process/elf-core] Support aarch64 NetBSD 
core dumps (authored by mgorny, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60034?vs=193020&id=193092#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60034

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/1lwp_SIGSEGV.aarch64.core
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.aarch64.core
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_t2_SIGSEGV.aarch64
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_t2_SIGSEGV.aarch64.core
  
packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/Process/elf-core/RegisterUtilities.h
  source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
@@ -168,6 +168,11 @@
 backtrace = ["bar", "foo", "main"]
 self.check_backtrace(thread, filename, backtrace)
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64(self):
+"""Test single-threaded aarch64 core dump."""
+self.do_test("1lwp_SIGSEGV.aarch64", pid=8339, region_count=32)
+
 @skipIfLLVMTargetMissing("X86")
 def test_amd64(self):
 """Test single-threaded amd64 core dump."""
@@ -193,6 +198,11 @@
 self.assertEqual(thread.GetStopReasonDataCount(), 1)
 self.assertEqual(thread.GetStopReasonDataAtIndex(0), 0)
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64(self):
+"""Test double-threaded aarch64 core dump where thread 2 is signalled."""
+self.do_test("2lwp_t2_SIGSEGV.aarch64", pid=14142, region_count=31)
+
 @skipIfLLVMTargetMissing("X86")
 def test_amd64(self):
 """Test double-threaded amd64 core dump where thread 2 is signalled."""
@@ -218,6 +228,11 @@
 self.assertEqual(thread.GetStopReasonDataCount(), 1)
 self.assertEqual(thread.GetStopReasonDataAtIndex(0), signal.SIGSEGV)
 
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64(self):
+"""Test double-threaded aarch64 core dump where process is signalled."""
+self.do_test("2lwp_process_SIGSEGV.aarch64", pid=1403, region_count=30)
+
 @skipIfLLVMTargetMissing("X86")
 def test_amd64(self):
 """Test double-threaded amd64 core dump where process is signalled."""
Index: source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- source/Plugins/Process/elf-core/RegisterUtilities.h
+++ source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -57,6 +57,10 @@
   NT_PROCINFO_CPI_SIGLWP_SIZE = 4,
 };
 
+namespace AARCH64 {
+enum { NT_REGS = 32, NT_FPREGS = 34 };
+}
+
 namespace AMD64 {
 enum { NT_REGS = 33, NT_FPREGS = 35 };
 }
@@ -124,6 +128,7 @@
 // The result from FXSAVE is in NT_PRXFPREG for i386 core files
 {llvm::Triple::Linux, llvm::Triple::x86, LINUX::NT_PRXFPREG},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, LINUX::NT_FPREGSET},
+{llvm::Triple::NetBSD, llvm::Triple::aarch64, NETBSD::AARCH64::NT_FPREGS},
 {llvm::Triple::NetBSD, llvm::Triple::x86_64, NETBSD::AMD64::NT_FPREGS},
 {llvm::Triple::OpenBSD, llvm::Triple::UnknownArch, OPENBSD::NT_FPREGS},
 };
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -628,6 +628,32 @@
 llvm::inconvertibleErrorCode());
 
   switch (GetArchitecture().GetMachine()) {
+  case llvm::Triple::aarch64: {
+// Assume order PT_GETREGS, PT_GETFPREGS
+if (note.info.n_type == NETBSD::AARCH64::NT_REGS) {
+  // If this is the next thread, push the previous one first.
+  if (had_nt_regs) {
+m_thread_data.push_back(thread_data);
+thread_data = ThreadData();
+had_nt_regs = false;
+  }
+
+  thread_data.gpregset = note.data;
+  thread_data.tid = tid;
+  if (thread_data.gpregset.GetByteSize() == 0)
+return llvm::make_error(
+"Could not find general purpose regist

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

2019-04-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 193125.
clayborg added a comment.

Fixed the requested issues. This patch is small and self contained and fixes 
the issue with partial UUID matching so I would like to start with this and 
iterate on the other features you mentioned with subsequent patches if 
possible. Let me know.


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

https://reviews.llvm.org/D60001

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

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -351,6 +351,8 @@
   std::vector filtered_modules =
   m_minidump_parser->GetFilteredModuleList();
 
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
+
   for (auto module : filtered_modules) {
 llvm::Optional name =
 m_minidump_parser->GetMinidumpString(module->module_name_rva);
@@ -358,7 +360,6 @@
 if (!name)
   continue;
 
-Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
 if (log) {
   log->Printf("ProcessMinidump::%s found module: name: %s %#010" PRIx64
   "-%#010" PRIx64 " size: %" PRIu32,
@@ -374,14 +375,49 @@
   m_is_wow64 = true;
 }
 
+if (log) {
+  log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__,
+  name.getValue().c_str());
+}
+
 const auto uuid = m_minidump_parser->GetModuleUUID(module);
 auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple());
 FileSystem::Instance().Resolve(file_spec);
 ModuleSpec module_spec(file_spec, uuid);
 module_spec.GetArchitecture() = GetArchitecture();
 Status error;
+// Try and find a module with a full UUID that matches. This function will
+// add the module to the target if it finds one, so we might need to remove
+// it if there is an error.
 lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
-if (!module_sp || error.Fail()) {
+if (!module_sp) {
+  // Try and find a module without specifying the UUID and only looking for
+  // the file given a basename. We then will look for a partial UUID match
+  // if we find any matches. This function will add the module to the
+  // target if it finds one, so we might need to remove it if there is an
+  // error.
+  ModuleSpec basename_module_spec(module_spec);
+  basename_module_spec.GetUUID().Clear();
+  basename_module_spec.GetFileSpec().GetDirectory().Clear();
+  module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);
+  if (module_sp) {
+// Verify that the first bytes of the UUID matches.
+const auto dmp_bytes = uuid.GetBytes();
+const auto mod_bytes = module_sp->GetUUID().GetBytes();
+// We have a match if either UUID is empty or as long as the actual
+// UUID starts with the minidump UUID
+const bool match = dmp_bytes.empty() || mod_bytes.empty() ||
+mod_bytes.take_front(dmp_bytes.size()) == dmp_bytes;
+// We have a valid UUID in the minidump, now check if the module
+// has a UUID. If the module doesn't have a UUID then we can call
+// this a mtach
+if (!match) {
+GetTarget().GetImages().Remove(module_sp);
+module_sp.reset();
+}
+  }
+}
+if (!module_sp) {
   // We failed to locate a matching local object file. Fortunately, the
   // minidump format encodes enough information about each module's memory
   // range to allow us to create placeholder modules.
@@ -400,11 +436,6 @@
   GetTarget().GetImages().Append(module_sp);
 }
 
-if (log) {
-  log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__,
-  name.getValue().c_str());
-}
-
 bool load_addr_changed = false;
 module_sp->SetLoadAddress(GetTarget(), module->base_of_image, false,
   load_addr_changed);
Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
+++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
@@ -0,0

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

2019-04-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

All of these are in SBProcess. No need to change everywhere else, I just see a 
ton of duplicated code here in the SBProcess.cpp. If we contain those in a 
small struct/class, then we can easily make changes as needed without 50 diffs 
in this file each time. So no need to fix all the call sites in this patch, 
just the ones in SBProcess.cpp


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D53412



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

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

2019-04-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Added Jim Ingham in case he wants to chime in here.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D53412



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

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

2019-04-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In general any call that is doing anything through the public API needs to take 
the target API mutex. It is ok for quick accessors or other things to not do 
so, but anything that would need to keep the process in its current state, like 
asking a frame for a register value, should take the API mutex to ensure one 
thread doesn't say "run" and another says "what is the PC from frame 0 of 
thread 1".


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D53412



___
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

2019-04-01 Thread Jason Molenda via Phabricator via lldb-commits

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



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)

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.


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] D60022: [Process] Fix WriteMemory return value

2019-04-01 Thread Davide Italiano via Phabricator via lldb-commits

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

LGTM


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] D60022: [Process] Fix WriteMemory return value

2019-04-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357420: [Process] Fix WriteMemory return value (authored by 
mib, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60022

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
  lldb/trunk/source/Target/Process.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
@@ -0,0 +1,29 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestWriteMemory(GDBRemoteTestBase):
+
+def test(self):
+
+class MyResponder(MockGDBServerResponder):
+def setBreakpoint(self, packet):
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+
+bp = target.BreakpointCreateByAddress(0x1000)
+self.assertTrue(bp.IsValid())
+self.assertEqual(bp.GetNumLocations(), 1)
+bp.SetEnabled(True)
+self.assertTrue(bp.IsEnabled())
+
+err = lldb.SBError()
+data = str("\x01\x02\x03\x04")
+result = process.WriteMemory(0x1000, data, err)
+self.assertEqual(result, 4)
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -2261,8 +2261,9 @@
   });
 
   if (bytes_written < size)
-WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
-   size - bytes_written, error);
+return bytes_written + WriteMemoryPrivate(addr + bytes_written,
+  ubuf + bytes_written,
+  size - bytes_written, error);
 }
   } else {
 return WriteMemoryPrivate(addr, buf, size, error);


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py
@@ -0,0 +1,29 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestWriteMemory(GDBRemoteTestBase):
+
+def test(self):
+
+class MyResponder(MockGDBServerResponder):
+def setBreakpoint(self, packet):
+return "OK"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+process = self.connect(target)
+
+bp = target.BreakpointCreateByAddress(0x1000)
+self.assertTrue(bp.IsValid())
+self.assertEqual(bp.GetNumLocations(), 1)
+bp.SetEnabled(True)
+self.assertTrue(bp.IsEnabled())
+
+err = lldb.SBError()
+data = str("\x01\x02\x03\x04")
+result = process.WriteMemory(0x1000, data, err)
+self.assertEqual(result, 4)
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -2261,8 +2261,9 @@
   });
 
   if (bytes_written < size)
-WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
-   size - bytes_written, error);
+return bytes_written + WriteMemoryPrivate(addr + bytes_written,
+  ubuf + bytes_written,
+  size - bytes_written, error);
 }
   } else {
 return WriteMemoryPrivate(addr, buf, size, error);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

2019-04-01 Thread Cameron via Phabricator via lldb-commits

cameron314 added a comment.

In D53412#1450182 , @clayborg wrote:

> All of these are in SBProcess. No need to change everywhere else, I just see 
> a ton of duplicated code here in the SBProcess.cpp. If we contain those in a 
> small struct/class, then we can easily make changes as needed without 50 
> diffs in this file each time. So no need to fix all the call sites in this 
> patch, just the ones in SBProcess.cpp

Ah, that makes more sense :-) Sorry I misinterpreted your suggestion.

I can add a RAII lock-helper struct in SBProcess, but the order that locks are 
taken within it must not be modified without changing *all* the functions that 
take both locks (both directly and indirectly), which includes a significant 
portion of the SB API (and not just SBProcess).

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D53412

___
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

2019-04-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:390-391
+// Try and find a module with a full UUID that matches. This function will
+// add the module to the target if it finds one, so we might need to remove
+// it if there is an error.
 lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, 
&error);

This comment (the part about removing in case of error) is no longer true. 
Please delete it.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:396-397
+  // the file given a basename. We then will look for a partial UUID match
+  // if we find any matches. This function will add the module to the
+  // target if it finds one, so we might need to remove it if there is an
+  // error.

Same here.



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);

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?



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:407-408
+const auto mod_bytes = module_sp->GetUUID().GetBytes();
+// We have a match if either UUID is empty or as long as the actual
+// UUID starts with the minidump UUID
+const bool match = dmp_bytes.empty() || mod_bytes.empty() ||

These comments no longer make sense like this as the checking happens all at 
once. Please rephrase them. Maybe something like "We consider the module to be 
a match if the minidump UUID is a prefix of the module UUID, or if one of the 
uuid's is empty". ?


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] r357424 - [API] Add SBReproducer to LLDB.h

2019-04-01 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Apr  1 12:56:15 2019
New Revision: 357424

URL: http://llvm.org/viewvc/llvm-project?rev=357424&view=rev
Log:
[API] Add SBReproducer to LLDB.h

Modified:
lldb/trunk/include/lldb/API/LLDB.h

Modified: lldb/trunk/include/lldb/API/LLDB.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/LLDB.h?rev=357424&r1=357423&r2=357424&view=diff
==
--- lldb/trunk/include/lldb/API/LLDB.h (original)
+++ lldb/trunk/include/lldb/API/LLDB.h Mon Apr  1 12:56:15 2019
@@ -48,6 +48,7 @@
 #include "lldb/API/SBProcessInfo.h"
 #include "lldb/API/SBQueue.h"
 #include "lldb/API/SBQueueItem.h"
+#include "lldb/API/SBReproducer.h"
 #include "lldb/API/SBSection.h"
 #include "lldb/API/SBSourceManager.h"
 #include "lldb/API/SBStream.h"


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

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

2019-04-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: mib, davide.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

I found the code of Process::WriteMemory particularly hard to follow when 
reviewing Ismail's change in D60022 . This 
simplifies the code and hopefully prevents similar oversigh in the future


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D60092

Files:
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2209,68 +2209,64 @@
   // may have placed in our tasks memory.
 
   BreakpointSiteList bp_sites_in_range;
+  if (!m_breakpoint_site_list.FindInRange(addr, addr + size, bp_sites_in_range))
+return WriteMemoryPrivate(addr, buf, size, error);
 
-  if (m_breakpoint_site_list.FindInRange(addr, addr + size,
- bp_sites_in_range)) {
-// No breakpoint sites overlap
-if (bp_sites_in_range.IsEmpty())
-  return WriteMemoryPrivate(addr, buf, size, error);
-else {
-  const uint8_t *ubuf = (const uint8_t *)buf;
-  uint64_t bytes_written = 0;
+  // No breakpoint sites overlap
+  if (bp_sites_in_range.IsEmpty())
+return WriteMemoryPrivate(addr, buf, size, error);
 
-  bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
- &error](BreakpointSite *bp) -> void {
+  const uint8_t *ubuf = (const uint8_t *)buf;
+  uint64_t bytes_written = 0;
 
-if (error.Success()) {
-  addr_t intersect_addr;
-  size_t intersect_size;
-  size_t opcode_offset;
-  const bool intersects = bp->IntersectsRange(
-  addr, size, &intersect_addr, &intersect_size, &opcode_offset);
-  UNUSED_IF_ASSERT_DISABLED(intersects);
-  assert(intersects);
-  assert(addr <= intersect_addr && intersect_addr < addr + size);
-  assert(addr < intersect_addr + intersect_size &&
- intersect_addr + intersect_size <= addr + size);
-  assert(opcode_offset + intersect_size <= bp->GetByteSize());
-
-  // Check for bytes before this breakpoint
-  const addr_t curr_addr = addr + bytes_written;
-  if (intersect_addr > curr_addr) {
-// There are some bytes before this breakpoint that we need to just
-// write to memory
-size_t curr_size = intersect_addr - curr_addr;
-size_t curr_bytes_written = WriteMemoryPrivate(
-curr_addr, ubuf + bytes_written, curr_size, error);
-bytes_written += curr_bytes_written;
-if (curr_bytes_written != curr_size) {
-  // We weren't able to write all of the requested bytes, we are
-  // done looping and will return the number of bytes that we have
-  // written so far.
-  if (error.Success())
-error.SetErrorToGenericError();
-}
-  }
-  // Now write any bytes that would cover up any software breakpoints
-  // directly into the breakpoint opcode buffer
-  ::memcpy(bp->GetSavedOpcodeBytes() + opcode_offset,
-   ubuf + bytes_written, intersect_size);
-  bytes_written += intersect_size;
-}
-  });
+  bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
+ &error](BreakpointSite *bp) -> void {
+if (error.Fail())
+  return;
 
-  if (bytes_written < size)
-return bytes_written + WriteMemoryPrivate(addr + bytes_written,
-  ubuf + bytes_written,
-  size - bytes_written, error);
+addr_t intersect_addr;
+size_t intersect_size;
+size_t opcode_offset;
+const bool intersects = bp->IntersectsRange(
+addr, size, &intersect_addr, &intersect_size, &opcode_offset);
+UNUSED_IF_ASSERT_DISABLED(intersects);
+assert(intersects);
+assert(addr <= intersect_addr && intersect_addr < addr + size);
+assert(addr < intersect_addr + intersect_size &&
+   intersect_addr + intersect_size <= addr + size);
+assert(opcode_offset + intersect_size <= bp->GetByteSize());
+
+// Check for bytes before this breakpoint
+const addr_t curr_addr = addr + bytes_written;
+if (intersect_addr > curr_addr) {
+  // There are some bytes before this breakpoint that we need to just
+  // write to memory
+  size_t curr_size = intersect_addr - curr_addr;
+  size_t curr_bytes_written =
+  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error);
+  bytes_written += curr_bytes_written;
+  if (curr_bytes_written != curr_size) {
+// We weren't able to write all of the requested bytes, we are
+// done looping and will return

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

2019-04-01 Thread Davide Italiano via Phabricator via lldb-commits

davide added a comment.

LGTM. Med, what do you think?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60092



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

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

2019-04-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

LGTM!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60092



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

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

2019-04-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60092



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

[Lldb-commits] [lldb] r357426 - [lldb-vscode] Add logic to handle EOF when reading from lldb-vscode stdout.

2019-04-01 Thread Jorge Gorbe Moya via lldb-commits

Author: jgorbe
Date: Mon Apr  1 13:37:22 2019
New Revision: 357426

URL: http://llvm.org/viewvc/llvm-project?rev=357426&view=rev
Log:
[lldb-vscode] Add logic to handle EOF when reading from lldb-vscode stdout.

Summary:
This change prevents the lldb-vscode test harness from hanging up waiting for
new messages when the lldb-vscode subprocess crashes.

Now, when an EOF from the subprocess pipe is detected we enqueue a `None` packet
in the received packets list. Then, during the message processing loop, we can
use this `None` packet to tell apart the case where lldb-vscode has terminated
unexpectedly from the normal situation where no pending messages means blocking
and waiting for more data.

I believe this should be enough to fix the issues with these tests hanging on
multiple platforms. Once this lands, I'll prepare and test a separate change
removing the @skipIfLinux annotations.

Reviewers: clayborg, zturner

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py?rev=357426&r1=357425&r2=357426&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py Mon 
Apr  1 13:37:22 2019
@@ -52,11 +52,11 @@ def dump_memory(base_addr, data, num_per
 
 def read_packet(f, verbose=False, trace_file=None):
 '''Decode a JSON packet that starts with the content length and is
-   followed by the JSON bytes from a file 'f'
+   followed by the JSON bytes from a file 'f'. Returns None on EOF.
 '''
 line = f.readline().decode("utf-8")
 if len(line) == 0:
-return None
+return None  # EOF.
 
 # Watch for line that starts with the prefix
 prefix = 'Content-Length: '
@@ -91,10 +91,10 @@ def read_packet_thread(vs_comm):
 done = False
 while not done:
 packet = read_packet(vs_comm.recv, trace_file=vs_comm.trace_file)
-if packet:
-done = not vs_comm.handle_recv_packet(packet)
-else:
-done = True
+# `packet` will be `None` on EOF. We want to pass it down to
+# handle_recv_packet anyway so the main thread can handle unexpected
+# termination of lldb-vscode and stop waiting for new packets.
+done = not vs_comm.handle_recv_packet(packet)
 
 
 class DebugCommunication(object):
@@ -146,6 +146,12 @@ class DebugCommunication(object):
 self.output_condition.release()
 return output
 
+def enqueue_recv_packet(self, packet):
+self.recv_condition.acquire()
+self.recv_packets.append(packet)
+self.recv_condition.notify()
+self.recv_condition.release()
+
 def handle_recv_packet(self, packet):
 '''Called by the read thread that is waiting for all incoming packets
to store the incoming packet in "self.recv_packets" in a thread safe
@@ -153,6 +159,11 @@ class DebugCommunication(object):
indicate a new packet is available. Returns True if the caller
should keep calling this function for more packets.
 '''
+# If EOF, notify the read thread by enqueing a None.
+if not packet:
+self.enqueue_recv_packet(None)
+return False
+
 # Check the packet to see if is an event packet
 keepGoing = True
 packet_type = packet['type']
@@ -191,10 +202,7 @@ class DebugCommunication(object):
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
 keepGoing = False
-self.recv_condition.acquire()
-self.recv_packets.append(packet)
-self.recv_condition.notify()
-self.recv_condition.release()
+self.enqueue_recv_packet(packet)
 return keepGoing
 
 def send_packet(self, command_dict, set_sequence=True):
@@ -222,27 +230,33 @@ class DebugCommunication(object):
function will wait for the packet to arrive and return it when
it does.'''
 while True:
-self.recv_condition.acquire()
-packet = None
-while True:
-for (i, curr_packet) in enumerate(self.recv_packets):
-packet_type = curr_packet['type']
-if filter_type is None or packet_type in filter_type:
-if (filter_event is None or
-(packet_type == 'event' and
- curr_packet['event'] in filter_event)):
-packet = self.recv_packets.pop(i)
-break
-if packet:
-

[Lldb-commits] [PATCH] D59849: [lldb-vscode] Add logic to handle EOF when reading from lldb-vscode stdout.

2019-04-01 Thread Jorge Gorbe Moya via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357426: [lldb-vscode] Add logic to handle EOF when reading 
from lldb-vscode stdout. (authored by jgorbe, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59849?vs=192368&id=193170#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59849

Files:
  lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -52,11 +52,11 @@
 
 def read_packet(f, verbose=False, trace_file=None):
 '''Decode a JSON packet that starts with the content length and is
-   followed by the JSON bytes from a file 'f'
+   followed by the JSON bytes from a file 'f'. Returns None on EOF.
 '''
 line = f.readline().decode("utf-8")
 if len(line) == 0:
-return None
+return None  # EOF.
 
 # Watch for line that starts with the prefix
 prefix = 'Content-Length: '
@@ -91,10 +91,10 @@
 done = False
 while not done:
 packet = read_packet(vs_comm.recv, trace_file=vs_comm.trace_file)
-if packet:
-done = not vs_comm.handle_recv_packet(packet)
-else:
-done = True
+# `packet` will be `None` on EOF. We want to pass it down to
+# handle_recv_packet anyway so the main thread can handle unexpected
+# termination of lldb-vscode and stop waiting for new packets.
+done = not vs_comm.handle_recv_packet(packet)
 
 
 class DebugCommunication(object):
@@ -146,6 +146,12 @@
 self.output_condition.release()
 return output
 
+def enqueue_recv_packet(self, packet):
+self.recv_condition.acquire()
+self.recv_packets.append(packet)
+self.recv_condition.notify()
+self.recv_condition.release()
+
 def handle_recv_packet(self, packet):
 '''Called by the read thread that is waiting for all incoming packets
to store the incoming packet in "self.recv_packets" in a thread safe
@@ -153,6 +159,11 @@
indicate a new packet is available. Returns True if the caller
should keep calling this function for more packets.
 '''
+# If EOF, notify the read thread by enqueing a None.
+if not packet:
+self.enqueue_recv_packet(None)
+return False
+
 # Check the packet to see if is an event packet
 keepGoing = True
 packet_type = packet['type']
@@ -191,10 +202,7 @@
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
 keepGoing = False
-self.recv_condition.acquire()
-self.recv_packets.append(packet)
-self.recv_condition.notify()
-self.recv_condition.release()
+self.enqueue_recv_packet(packet)
 return keepGoing
 
 def send_packet(self, command_dict, set_sequence=True):
@@ -222,27 +230,33 @@
function will wait for the packet to arrive and return it when
it does.'''
 while True:
-self.recv_condition.acquire()
-packet = None
-while True:
-for (i, curr_packet) in enumerate(self.recv_packets):
-packet_type = curr_packet['type']
-if filter_type is None or packet_type in filter_type:
-if (filter_event is None or
-(packet_type == 'event' and
- curr_packet['event'] in filter_event)):
-packet = self.recv_packets.pop(i)
-break
-if packet:
-break
-# Sleep until packet is received
-len_before = len(self.recv_packets)
-self.recv_condition.wait(timeout)
-len_after = len(self.recv_packets)
-if len_before == len_after:
-return None  # Timed out
-self.recv_condition.release()
-return packet
+try:
+self.recv_condition.acquire()
+packet = None
+while True:
+for (i, curr_packet) in enumerate(self.recv_packets):
+if not curr_packet:
+raise EOFError
+packet_type = curr_packet['type']
+if filter_type is None or packet_type in filter_type:
+if (filter_event is None or
+(packet_type == 'event' and
+

[Lldb-commits] [lldb] r357428 - [Process] Use early returns in Process::WriteMemory (NFC)

2019-04-01 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Apr  1 13:39:03 2019
New Revision: 357428

URL: http://llvm.org/viewvc/llvm-project?rev=357428&view=rev
Log:
[Process] Use early returns in Process::WriteMemory (NFC)

I found the code of Process::WriteMemory particularly hard to follow
when reviewing Ismail's change in D60022. This simplifies the code and
hopefully prevents similar oversights in the future.

Differential revision: https://reviews.llvm.org/D60092

Modified:
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Target/Process.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=357428&r1=357427&r2=357428&view=diff
==
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Apr  1 13:39:03 2019
@@ -2209,68 +2209,64 @@ size_t Process::WriteMemory(addr_t addr,
   // may have placed in our tasks memory.
 
   BreakpointSiteList bp_sites_in_range;
+  if (!m_breakpoint_site_list.FindInRange(addr, addr + size, 
bp_sites_in_range))
+return WriteMemoryPrivate(addr, buf, size, error);
 
-  if (m_breakpoint_site_list.FindInRange(addr, addr + size,
- bp_sites_in_range)) {
-// No breakpoint sites overlap
-if (bp_sites_in_range.IsEmpty())
-  return WriteMemoryPrivate(addr, buf, size, error);
-else {
-  const uint8_t *ubuf = (const uint8_t *)buf;
-  uint64_t bytes_written = 0;
-
-  bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
- &error](BreakpointSite *bp) -> void {
-
-if (error.Success()) {
-  addr_t intersect_addr;
-  size_t intersect_size;
-  size_t opcode_offset;
-  const bool intersects = bp->IntersectsRange(
-  addr, size, &intersect_addr, &intersect_size, &opcode_offset);
-  UNUSED_IF_ASSERT_DISABLED(intersects);
-  assert(intersects);
-  assert(addr <= intersect_addr && intersect_addr < addr + size);
-  assert(addr < intersect_addr + intersect_size &&
- intersect_addr + intersect_size <= addr + size);
-  assert(opcode_offset + intersect_size <= bp->GetByteSize());
-
-  // Check for bytes before this breakpoint
-  const addr_t curr_addr = addr + bytes_written;
-  if (intersect_addr > curr_addr) {
-// There are some bytes before this breakpoint that we need to just
-// write to memory
-size_t curr_size = intersect_addr - curr_addr;
-size_t curr_bytes_written = WriteMemoryPrivate(
-curr_addr, ubuf + bytes_written, curr_size, error);
-bytes_written += curr_bytes_written;
-if (curr_bytes_written != curr_size) {
-  // We weren't able to write all of the requested bytes, we are
-  // done looping and will return the number of bytes that we have
-  // written so far.
-  if (error.Success())
-error.SetErrorToGenericError();
-}
-  }
-  // Now write any bytes that would cover up any software breakpoints
-  // directly into the breakpoint opcode buffer
-  ::memcpy(bp->GetSavedOpcodeBytes() + opcode_offset,
-   ubuf + bytes_written, intersect_size);
-  bytes_written += intersect_size;
-}
-  });
-
-  if (bytes_written < size)
-return bytes_written + WriteMemoryPrivate(addr + bytes_written,
-  ubuf + bytes_written,
-  size - bytes_written, error);
-}
-  } else {
+  // No breakpoint sites overlap
+  if (bp_sites_in_range.IsEmpty())
 return WriteMemoryPrivate(addr, buf, size, error);
-  }
+
+  const uint8_t *ubuf = (const uint8_t *)buf;
+  uint64_t bytes_written = 0;
+
+  bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
+ &error](BreakpointSite *bp) -> void {
+if (error.Fail())
+  return;
+
+addr_t intersect_addr;
+size_t intersect_size;
+size_t opcode_offset;
+const bool intersects = bp->IntersectsRange(
+addr, size, &intersect_addr, &intersect_size, &opcode_offset);
+UNUSED_IF_ASSERT_DISABLED(intersects);
+assert(intersects);
+assert(addr <= intersect_addr && intersect_addr < addr + size);
+assert(addr < intersect_addr + intersect_size &&
+   intersect_addr + intersect_size <= addr + size);
+assert(opcode_offset + intersect_size <= bp->GetByteSize());
+
+// Check for bytes before this breakpoint
+const addr_t curr_addr = addr + bytes_written;
+if (intersect_addr > curr_addr) {
+  // There are some bytes before this breakpoint that we need to just
+  // write to memory
+  size_t curr_size = intersect_addr - curr_addr;
+  size_t curr_bytes_writt

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

2019-04-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357428: [Process] Use early returns in Process::WriteMemory 
(NFC) (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60092?vs=193164&id=193173#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60092

Files:
  lldb/trunk/source/Target/Process.cpp

Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -2209,68 +2209,64 @@
   // may have placed in our tasks memory.
 
   BreakpointSiteList bp_sites_in_range;
+  if (!m_breakpoint_site_list.FindInRange(addr, addr + size, bp_sites_in_range))
+return WriteMemoryPrivate(addr, buf, size, error);
 
-  if (m_breakpoint_site_list.FindInRange(addr, addr + size,
- bp_sites_in_range)) {
-// No breakpoint sites overlap
-if (bp_sites_in_range.IsEmpty())
-  return WriteMemoryPrivate(addr, buf, size, error);
-else {
-  const uint8_t *ubuf = (const uint8_t *)buf;
-  uint64_t bytes_written = 0;
+  // No breakpoint sites overlap
+  if (bp_sites_in_range.IsEmpty())
+return WriteMemoryPrivate(addr, buf, size, error);
 
-  bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
- &error](BreakpointSite *bp) -> void {
+  const uint8_t *ubuf = (const uint8_t *)buf;
+  uint64_t bytes_written = 0;
 
-if (error.Success()) {
-  addr_t intersect_addr;
-  size_t intersect_size;
-  size_t opcode_offset;
-  const bool intersects = bp->IntersectsRange(
-  addr, size, &intersect_addr, &intersect_size, &opcode_offset);
-  UNUSED_IF_ASSERT_DISABLED(intersects);
-  assert(intersects);
-  assert(addr <= intersect_addr && intersect_addr < addr + size);
-  assert(addr < intersect_addr + intersect_size &&
- intersect_addr + intersect_size <= addr + size);
-  assert(opcode_offset + intersect_size <= bp->GetByteSize());
-
-  // Check for bytes before this breakpoint
-  const addr_t curr_addr = addr + bytes_written;
-  if (intersect_addr > curr_addr) {
-// There are some bytes before this breakpoint that we need to just
-// write to memory
-size_t curr_size = intersect_addr - curr_addr;
-size_t curr_bytes_written = WriteMemoryPrivate(
-curr_addr, ubuf + bytes_written, curr_size, error);
-bytes_written += curr_bytes_written;
-if (curr_bytes_written != curr_size) {
-  // We weren't able to write all of the requested bytes, we are
-  // done looping and will return the number of bytes that we have
-  // written so far.
-  if (error.Success())
-error.SetErrorToGenericError();
-}
-  }
-  // Now write any bytes that would cover up any software breakpoints
-  // directly into the breakpoint opcode buffer
-  ::memcpy(bp->GetSavedOpcodeBytes() + opcode_offset,
-   ubuf + bytes_written, intersect_size);
-  bytes_written += intersect_size;
-}
-  });
+  bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
+ &error](BreakpointSite *bp) -> void {
+if (error.Fail())
+  return;
 
-  if (bytes_written < size)
-return bytes_written + WriteMemoryPrivate(addr + bytes_written,
-  ubuf + bytes_written,
-  size - bytes_written, error);
-}
-  } else {
-return WriteMemoryPrivate(addr, buf, size, error);
-  }
+addr_t intersect_addr;
+size_t intersect_size;
+size_t opcode_offset;
+const bool intersects = bp->IntersectsRange(
+addr, size, &intersect_addr, &intersect_size, &opcode_offset);
+UNUSED_IF_ASSERT_DISABLED(intersects);
+assert(intersects);
+assert(addr <= intersect_addr && intersect_addr < addr + size);
+assert(addr < intersect_addr + intersect_size &&
+   intersect_addr + intersect_size <= addr + size);
+assert(opcode_offset + intersect_size <= bp->GetByteSize());
+
+// Check for bytes before this breakpoint
+const addr_t curr_addr = addr + bytes_written;
+if (intersect_addr > curr_addr) {
+  // There are some bytes before this breakpoint that we need to just
+  // write to memory
+  size_t curr_size = intersect_addr - curr_addr;
+  size_t curr_bytes_written =
+  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error);
+  bytes_written += curr_bytes_written;
+

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

2019-04-01 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth accepted this revision.
amccarth added inline comments.



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());

nit:  As long as you're correcting this comment, can you fix "look up" to be 
two words?


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] r357431 - [CMake] Only the Python scirpt interpreter should link against Python.

2019-04-01 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Apr  1 15:03:04 2019
New Revision: 357431

URL: http://llvm.org/viewvc/llvm-project?rev=357431&view=rev
Log:
[CMake] Only the Python scirpt interpreter should link against Python.

This patch removes spurious links against Python.

Modified:
lldb/trunk/unittests/Interpreter/CMakeLists.txt
lldb/trunk/unittests/ScriptInterpreter/Python/CMakeLists.txt

Modified: lldb/trunk/unittests/Interpreter/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/CMakeLists.txt?rev=357431&r1=357430&r2=357431&view=diff
==
--- lldb/trunk/unittests/Interpreter/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Interpreter/CMakeLists.txt Mon Apr  1 15:03:04 2019
@@ -6,8 +6,3 @@ add_lldb_unittest(InterpreterTests
 lldbInterpreter
 lldbUtilityHelpers
   )
-
-target_link_libraries(InterpreterTests
-  PRIVATE
-  ${PYTHON_LIBRARY}
-  )

Modified: lldb/trunk/unittests/ScriptInterpreter/Python/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/ScriptInterpreter/Python/CMakeLists.txt?rev=357431&r1=357430&r2=357431&view=diff
==
--- lldb/trunk/unittests/ScriptInterpreter/Python/CMakeLists.txt (original)
+++ lldb/trunk/unittests/ScriptInterpreter/Python/CMakeLists.txt Mon Apr  1 
15:03:04 2019
@@ -6,8 +6,6 @@ add_lldb_unittest(ScriptInterpreterPytho
   LINK_LIBS
 lldbHost
 lldbPluginScriptInterpreterPython
-${PYTHON_LIBRARY}
   LINK_COMPONENTS
 Support
-  )
-  
\ No newline at end of file
+  )
\ No newline at end of file


___
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

2019-04-01 Thread Reid Kleckner via Phabricator via lldb-commits

rnk updated this revision to Diff 193191.
rnk marked 2 inline comments as done.
rnk added a comment.
Herald added subscribers: lldb-commits, cfe-commits, kadircet, arphaman, 
jkorous.
Herald added projects: clang, LLDB.

- fix one lldb usage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60018

Files:
  clang-tools-extra/unittests/clangd/JSONTransportTests.cpp
  lld/COFF/PDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  llvm/include/llvm/DebugInfo/CodeView/CVRecord.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/CodeVi

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

2019-04-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 193198.
clayborg marked 2 inline comments as done.
clayborg added a comment.

Fix comments and address review issues.


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

https://reviews.llvm.org/D60001

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

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -351,6 +351,8 @@
   std::vector filtered_modules =
   m_minidump_parser->GetFilteredModuleList();
 
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
+
   for (auto module : filtered_modules) {
 llvm::Optional name =
 m_minidump_parser->GetMinidumpString(module->module_name_rva);
@@ -358,7 +360,6 @@
 if (!name)
   continue;
 
-Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
 if (log) {
   log->Printf("ProcessMinidump::%s found module: name: %s %#010" PRIx64
   "-%#010" PRIx64 " size: %" PRIu32,
@@ -374,14 +375,46 @@
   m_is_wow64 = true;
 }
 
+if (log) {
+  log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__,
+  name.getValue().c_str());
+}
+
 const auto uuid = m_minidump_parser->GetModuleUUID(module);
 auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple());
 FileSystem::Instance().Resolve(file_spec);
 ModuleSpec module_spec(file_spec, uuid);
 module_spec.GetArchitecture() = GetArchitecture();
 Status error;
+// Try and find a module with a full UUID that matches. This function will
+// add the module to the target if it finds one.
 lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
-if (!module_sp || error.Fail()) {
+if (!module_sp) {
+  // Try and find a module without specifying the UUID and only looking for
+  // the file given a basename. We then will look for a partial UUID match
+  // if we find any matches. This function will add the module to the
+  // target if it finds one, so we need to remove the module from the target
+  // if the UUID doesn't match during our manual UUID verification. This
+  // allows the "target.exec-search-paths" setting to specify one or more
+  // directories that contain executables that can be searched for matches.
+  ModuleSpec basename_module_spec(module_spec);
+  basename_module_spec.GetUUID().Clear();
+  basename_module_spec.GetFileSpec().GetDirectory().Clear();
+  module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);
+  if (module_sp) {
+// We consider the module to be a match if the minidump UUID is a
+// prefix of the actual UUID, or if either of the UUIDs are empty.
+const auto dmp_bytes = uuid.GetBytes();
+const auto mod_bytes = module_sp->GetUUID().GetBytes();
+const bool match = dmp_bytes.empty() || mod_bytes.empty() ||
+mod_bytes.take_front(dmp_bytes.size()) == dmp_bytes;
+if (!match) {
+GetTarget().GetImages().Remove(module_sp);
+module_sp.reset();
+}
+  }
+}
+if (!module_sp) {
   // We failed to locate a matching local object file. Fortunately, the
   // minidump format encodes enough information about each module's memory
   // range to allow us to create placeholder modules.
@@ -400,11 +433,6 @@
   GetTarget().GetImages().Append(module_sp);
 }
 
-if (log) {
-  log->Printf("ProcessMinidump::%s load module: name: %s", __FUNCTION__,
-  name.getValue().c_str());
-}
-
 bool load_addr_changed = false;
 module_sp->SetLoadAddress(GetTarget(), module->base_of_image, false,
   load_addr_changed);
Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml
+++ 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_VE

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

2019-04-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg marked 3 inline comments as done.
clayborg added inline comments.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:391
+// add the module to the target if it finds one, so we might need to remove
+// it if there is an error.
 lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, 
&error);

Will fix



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:
> 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.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:408
+// We have a match if either UUID is empty or as long as the actual
+// UUID starts with the minidump UUID
+const bool match = dmp_bytes.empty() || mod_bytes.empty() ||

I will change it


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] D60018: [codeview] Remove Type member from CVRecord

2019-04-01 Thread Alexandre Ganea via Phabricator via lldb-commits

aganea 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 {

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?

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);

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?

Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h:125
+Pre.RecordKind = uint16_t(TypeLeafKind::LF_FIELDLIST);
+CVType FieldList(&Pre, sizeof(Pre));
 consumeError(Mapping.Mapping.visitTypeEnd(FieldList));

Applying what I said above would give:
```
RecordPrefix Pre{uint16_t(TypeLeafKind::LF_FIELDLIST)};
CVType FieldList(&Pre);
```

Comment at: llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp:37

+  CVType CVT(ArrayRef(ScratchBuffer.data(), sizeof(RecordPrefix)));
   cantFail(Mapping.visitTypeBegin(CVT));

Here it is the same thing: `writeRecordPrefix()` writes a `.RecordLen = 0`, but 
it could very well write 2 to be coherent, then you would be able to //trust// 
the `RecordPrefix *`.

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

I suppose you've chosen 1 because that's a invalid record size? Comment maybe?

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

2019-04-01 Thread Alexandre Ganea via Phabricator via lldb-commits

aganea marked an inline comment as done.
aganea 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:
> 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.


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] D60068: PDBFPO: Refactor register reference resolution

2019-04-01 Thread Aleksandr Urakov via Phabricator via lldb-commits

aleksandr.urakov accepted this revision.
aleksandr.urakov added a comment.

LGTM!


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] r357376 - [Linux/x86] Fix writing of non-gpr registers on newer processors

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

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

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

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

[Lldb-commits] [lldb] r357399 - [lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

[Lldb-commits] [PATCH] D60034: [lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

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

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

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

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

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

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

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

[Lldb-commits] [lldb] r357424 - [API] Add SBReproducer to LLDB.h

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

[Lldb-commits] [lldb] r357426 - [lldb-vscode] Add logic to handle EOF when reading from lldb-vscode stdout.

[Lldb-commits] [PATCH] D59849: [lldb-vscode] Add logic to handle EOF when reading from lldb-vscode stdout.

[Lldb-commits] [lldb] r357428 - [Process] Use early returns in Process::WriteMemory (NFC)

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

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

[Lldb-commits] [lldb] r357431 - [CMake] Only the Python scirpt interpreter should link against Python.

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

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

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

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

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

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

34 matches

Site Navigation

Mail list logo

Footer information