[Lldb-commits] [lldb] 20f8425 - [lldb] Fix GetRemoteSharedModule fallback logic

2020-09-23 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2020-09-23T06:00:50-07:00
New Revision: 20f84257ac4ac54ceb5f581a6081fac6eff2a5a1

URL: 
https://github.com/llvm/llvm-project/commit/20f84257ac4ac54ceb5f581a6081fac6eff2a5a1
DIFF: 
https://github.com/llvm/llvm-project/commit/20f84257ac4ac54ceb5f581a6081fac6eff2a5a1.diff

LOG: [lldb] Fix GetRemoteSharedModule fallback logic

When the various methods of locating the module in GetRemoteSharedModule
fail, make sure we pass the original module spec to the bail-out call to
the provided resolver function.

Also make sure we consistently use the resolved module spec from the
various success paths.

Thanks to what appears to have been an accidentally inverted condition
(commit 85967fa applied the new condition to a path where GetModuleSpec
returns false, but should have applied it when GetModuleSpec returns
true), without this fix we only pass the original module spec in the
fallback if the original spec has no uuid (or has a uuid that somehow
matches the resolved module's uuid despite the call to GetModuleSpec
failing).  This manifested as a bug when processing a minidump file with
a user-provided sysroot, since in that case the resolver call was being
applied to resolved_module_spec (despite resolution failing), which did
not have the path of its file_spec set.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Target/Platform.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py

Removed: 




diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 7416ea6dd40c..7e1b7450de0e 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1580,21 +1580,29 @@ Status Platform::GetRemoteSharedModule(const ModuleSpec 
&module_spec,
   if (error.Success() && module_sp)
 break;
 }
-if (module_sp)
+if (module_sp) {
+  resolved_module_spec = arch_module_spec;
   got_module_spec = true;
+}
   }
 
   if (!got_module_spec) {
 // Get module information from a target.
-if (!GetModuleSpec(module_spec.GetFileSpec(), 
module_spec.GetArchitecture(),
-   resolved_module_spec)) {
+if (GetModuleSpec(module_spec.GetFileSpec(), module_spec.GetArchitecture(),
+  resolved_module_spec)) {
   if (!module_spec.GetUUID().IsValid() ||
   module_spec.GetUUID() == resolved_module_spec.GetUUID()) {
-return module_resolver(module_spec);
+got_module_spec = true;
   }
 }
   }
 
+  if (!got_module_spec) {
+// Fall back to the given module resolver, which may have its own
+// search logic.
+return module_resolver(module_spec);
+  }
+
   // If we are looking for a specific UUID, make sure resolved_module_spec has
   // the same one before we search.
   if (module_spec.GetUUID().IsValid()) {

diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 012f9b67d9e3..9d2daec67698 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -455,3 +455,29 @@ def check_region(index, start, end, read, write, execute, 
mapped, name):
 check_region(17, 0x40169000, 0x4016b000, True,  True,  False, True,  d)
 check_region(18, 0x4016b000, 0x40176000, True,  True,  False, True,  n)
 check_region(-1, 0x40176000, max_int,False, False, False, False, n)
+
+@skipIfLLVMTargetMissing("X86")
+def test_minidump_sysroot(self):
+"""Test that lldb can find a module referenced in an i386 linux 
minidump using the sysroot."""
+
+# Copy linux-x86_64 executable to tmp_sysroot/temp/test/ (since it was 
compiled as
+# /tmp/test/linux-x86_64)
+tmp_sysroot = os.path.join(
+self.getBuildDir(), "lldb_i386_mock_sysroot")
+executable = os.path.join(
+tmp_sysroot, "tmp", "test", "linux-x86_64")
+exe_dir = os.path.dirname(executable)
+lldbutil.mkdir_p(exe_dir)
+shutil.copyfile("linux-x86_64", executable)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" %
+tmp_sysroot)
+self.process_from_yaml("linux-x86_64.yaml")
+self.check_state()
+
+# Check that we loaded the module from the sysroot
+self.assertEqual(self.target.GetNumModules(), 1)
+module = self.target.GetModuleAtIndex(0)
+spec = module.GetFileSpec()
+self.assertEqual(spec.GetDirectory(), exe_dir)



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


[Lldb-commits] [lldb] 4a55c98 - [lldb] Normalize paths in new test

2020-09-23 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2020-09-23T12:41:47-07:00
New Revision: 4a55c98fa7bee1e5ab1504db20ca4d7c8a083111

URL: 
https://github.com/llvm/llvm-project/commit/4a55c98fa7bee1e5ab1504db20ca4d7c8a083111
DIFF: 
https://github.com/llvm/llvm-project/commit/4a55c98fa7bee1e5ab1504db20ca4d7c8a083111.diff

LOG: [lldb] Normalize paths in new test

The minidump-sysroot test I added in commit 20f84257 compares two paths
using a string comparison.  This causes the Windows buildbot to fail
because of mismatched forward slashes and backslashes.  Use
os.path.normcase to normalize before comparing.

Added: 


Modified: 
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 9d2daec67698..103e86efc54d 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -479,5 +479,6 @@ def test_minidump_sysroot(self):
 # Check that we loaded the module from the sysroot
 self.assertEqual(self.target.GetNumModules(), 1)
 module = self.target.GetModuleAtIndex(0)
-spec = module.GetFileSpec()
-self.assertEqual(spec.GetDirectory(), exe_dir)
+spec_dir_norm = os.path.normcase(module.GetFileSpec().GetDirectory())
+exe_dir_norm = os.path.normcase(exe_dir)
+self.assertEqual(spec_dir_norm, exe_dir_norm)



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


[Lldb-commits] [lldb] d30797b - [lldb] Minidump: check for .text hash match with directory

2020-10-16 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2020-10-16T09:32:08-04:00
New Revision: d30797b4041ffe215b92d376af60c4f26a0555ae

URL: 
https://github.com/llvm/llvm-project/commit/d30797b4041ffe215b92d376af60c4f26a0555ae
DIFF: 
https://github.com/llvm/llvm-project/commit/d30797b4041ffe215b92d376af60c4f26a0555ae.diff

LOG: [lldb] Minidump: check for .text hash match with directory

When opening a minidump, we might discover that it reports a UUID for a
module that doesn't match the build ID, but rather a hash of the .text
section (according to either of two different hash functions, used by
breakpad and Facebook respectively).  The current logic searches for a
module by filename only to check the hash; this change updates it to
first search by directory+filename.  This is important when the
directory specified in the minidump must be interpreted relative to a
user-provided sysoort, as the leaf directory won't be in the search path
in that case.

Also add a regression test; without this change, module validation fails
because we have just the placeholder module which reports as its path
the platform path in the minidump.

Reviewed By: clayborg

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

Added: 
lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-decoy.yaml

Modified: 
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp 
b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 17fdfdb4f345..26c56e38819f 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -484,6 +484,53 @@ bool ProcessMinidump::UpdateThreadList(ThreadList 
&old_thread_list,
   return new_thread_list.GetSize(false) > 0;
 }
 
+ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid,
+llvm::StringRef name,
+ModuleSpec module_spec) {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  Status error;
+
+  ModuleSP module_sp =
+  GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
+  if (!module_sp)
+return 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 = minidump_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) {
+LLDB_LOG(log, "Partial uuid match for {0}.", name);
+return module_sp;
+  }
+
+  // Breakpad generates minindump files, and if there is no GNU build
+  // ID in the binary, it will calculate a UUID by hashing first 4096
+  // bytes of the .text section and using that as the UUID for a module
+  // in the minidump. Facebook uses a modified breakpad client that
+  // uses a slightly modified this hash to avoid collisions. Check for
+  // UUIDs from the minindump that match these cases and accept the
+  // module we find if they do match.
+  std::vector breakpad_uuid;
+  std::vector facebook_uuid;
+  HashElfTextSection(module_sp, breakpad_uuid, facebook_uuid);
+  if (dmp_bytes == llvm::ArrayRef(breakpad_uuid)) {
+LLDB_LOG(log, "Breakpad .text hash match for {0}.", name);
+return module_sp;
+  }
+  if (dmp_bytes == llvm::ArrayRef(facebook_uuid)) {
+LLDB_LOG(log, "Facebook .text hash match for {0}.", name);
+return module_sp;
+  }
+  // The UUID wasn't a partial match and didn't match the .text hash
+  // so remove the module from the target, we will need to create a
+  // placeholder object file.
+  GetTarget().GetImages().Remove(module_sp);
+  module_sp.reset();
+  return module_sp;
+}
+
 void ProcessMinidump::ReadModuleList() {
   std::vector filtered_modules =
   m_minidump_parser->GetFilteredModuleList();
@@ -513,54 +560,22 @@ void ProcessMinidump::ReadModuleList() {
 // add the module to the target if it finds one.
 lldb::ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec,
  true /* notify */, 
&error);
-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

[Lldb-commits] [lldb] r359120 - [lldb] Use local definition of get_cpuid_count

2019-10-04 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Wed Apr 24 11:00:12 2019
New Revision: 359120

URL: http://llvm.org/viewvc/llvm-project?rev=359120&view=rev
Log:
[lldb] Use local definition of get_cpuid_count

Summary:
This is needed for gcc/cstdlib++ 5.4.0, where __get_cpuid_count is not
defined in cpuid.h.

Reviewers: labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:

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

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=359120&r1=359119&r2=359120&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
Wed Apr 24 11:00:12 2019
@@ -21,6 +21,23 @@
 #include 
 #include 
 
+// Newer toolchains define __get_cpuid_count in cpuid.h, but some
+// older-but-still-supported ones (e.g. gcc 5.4.0) don't, so we
+// define it locally here, following the definition in clang/lib/Headers.
+static inline int get_cpuid_count(unsigned int __leaf,
+  unsigned int __subleaf,
+  unsigned int *__eax, unsigned int *__ebx,
+  unsigned int *__ecx, unsigned int *__edx)
+{
+unsigned int __max_leaf = __get_cpuid_max(__leaf & 0x8000, 0);
+
+if (__max_leaf == 0 || __max_leaf < __leaf)
+return 0;
+
+__cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
+return 1;
+}
+
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
 
@@ -269,7 +286,7 @@ static std::size_t GetXSTATESize() {
 return sizeof(FPR);
 
   // Then fetch the maximum size of the area.
-  if (!__get_cpuid_count(0x0d, 0, &eax, &ebx, &ecx, &edx))
+  if (!get_cpuid_count(0x0d, 0, &eax, &ebx, &ecx, &edx))
 return sizeof(FPR);
   return std::max(ecx, sizeof(FPR));
 }


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


[Lldb-commits] [lldb] r366580 - Support Linux signal return trampolines in frame initialization

2019-10-04 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Fri Jul 19 07:05:55 2019
New Revision: 366580

URL: http://llvm.org/viewvc/llvm-project?rev=366580&view=rev
Log:
Support Linux signal return trampolines in frame initialization

Summary:
Add __kernel_rt_sigreturn to the list of trap handlers for Linux (it's
used as such on aarch64 at least), and __restore_rt as well (used on
x86_64).

Skip decrement-and-recompute for trap handlers in
InitializeNonZerothFrame, as signal dispatch may point the child frame's
return address to the start of the return trampoline.

Parse the 'S' flag for signal handlers from eh_frame augmentation, and
propagate it to the unwind plan.

Reviewers: labath, jankratochvil, compnerd, jfb, jasonmolenda

Reviewed By: jasonmolenda

Subscribers: clayborg, MaskRay, wuzish, nemanjai, kbarton, jrtc27, atanasyan, 
jsji, javed.absar, kristof.beyls, lldb-commits

Tags: #lldb

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

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/

lldb/trunk/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/TestHandleAbort.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/main.c
Modified:
lldb/trunk/include/lldb/Symbol/UnwindPlan.h
lldb/trunk/source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
lldb/trunk/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
lldb/trunk/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
lldb/trunk/source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
lldb/trunk/source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
lldb/trunk/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
lldb/trunk/source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
lldb/trunk/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/trunk/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
lldb/trunk/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
lldb/trunk/source/Symbol/ArmUnwindInfo.cpp
lldb/trunk/source/Symbol/CompactUnwindInfo.cpp
lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp

Modified: lldb/trunk/include/lldb/Symbol/UnwindPlan.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/UnwindPlan.h?rev=366580&r1=366579&r2=366580&view=diff
==
--- lldb/trunk/include/lldb/Symbol/UnwindPlan.h (original)
+++ lldb/trunk/include/lldb/Symbol/UnwindPlan.h Fri Jul 19 07:05:55 2019
@@ -370,6 +370,7 @@ public:
 m_return_addr_register(LLDB_INVALID_REGNUM), m_source_name(),
 m_plan_is_sourced_from_compiler(eLazyBoolCalculate),
 m_plan_is_valid_at_all_instruction_locations(eLazyBoolCalculate),
+m_plan_is_for_signal_trap(eLazyBoolCalculate),
 m_lsda_address(), m_personality_func_addr() {}
 
   // Performs a deep copy of the plan, including all the rows (expensive).
@@ -463,6 +464,17 @@ public:
 m_plan_is_valid_at_all_instruction_locations = valid_at_all_insn;
   }
 
+  // Is this UnwindPlan for a signal trap frame?  If so, then its saved pc
+  // may have been set manually by the signal dispatch code and therefore
+  // not follow a call to the child frame.
+  lldb_private::LazyBool GetUnwindPlanForSignalTrap() const {
+return m_plan_is_for_signal_trap;
+  }
+
+  void SetUnwindPlanForSignalTrap(lldb_private::LazyBool is_for_signal_trap) {
+m_plan_is_for_signal_trap = is_for_signal_trap;
+  }
+
   int GetRowCount() const;
 
   void Clear() {
@@ -472,6 +484,7 @@ public:
 m_source_name.Clear();
 m_plan_is_sourced_from_compiler = eLazyBoolCalculate;
 m_plan_is_valid_at_all_instruction_locations = eLazyBoolCalculate;
+m_plan_is_for_signal_trap = eLazyBoolCalculate;
 m_lsda_address.Clear();
 m_personality_func_addr.Clear();
   }
@@ -502,6 +515,7 @@ private:
   m_source_name; // for logging, where this UnwindPlan originated from
   lldb_private::LazyBool m_plan_is_sourced_from_compiler;
   lldb_private::LazyBool m_plan_is_valid_at_all_instruction_locations;
+  lldb_private::LazyBool m_plan_is_for_signal_trap;
 
   Address m_lsda_address; // Where the language s

[Lldb-commits] [lldb] r375242 - Update MinidumpYAML to use minidump::Exception for exception stream

2019-10-18 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Fri Oct 18 07:56:19 2019
New Revision: 375242

URL: http://llvm.org/viewvc/llvm-project?rev=375242&view=rev
Log:
Update MinidumpYAML to use minidump::Exception for exception stream

Reviewers: labath, jhenderson, clayborg, MaskRay, grimar

Reviewed By: grimar

Subscribers: lldb-commits, grimar, MaskRay, hiraditya, llvm-commits

Tags: #llvm, #lldb

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

Modified:

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

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml?rev=375242&r1=375241&r2=375242&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
 Fri Oct 18 07:56:19 2019
@@ -14,7 +14,10 @@ Streams:
 Module Name: '/tmp/test/linux-x86_64'
 CodeView Record: 4C457042E35C283BC327C28762DB788BF5A4078BE2351448
   - Type:Exception
-Content: 
DD740B00D004F831
+Thread ID:   0x74DD
+Exception Record:
+  Exception Code:  0x000B
+Thread Context:  
   - Type:SystemInfo
 Processor Arch:  AMD64
 Processor Level: 6


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


[Lldb-commits] [lldb] r375243 - LLDB: Use LLVM's type for minidump ExceptionStream [NFC]

2019-10-18 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Fri Oct 18 07:59:10 2019
New Revision: 375243

URL: http://llvm.org/viewvc/llvm-project?rev=375243&view=rev
Log:
LLDB: Use LLVM's type for minidump ExceptionStream [NFC]

Summary: The types defined for it in LLDB are now redundant with core types.

Reviewers: labath, clayborg

Reviewed By: clayborg

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=375243&r1=375242&r2=375243&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Fri Oct 18 
07:59:10 2019
@@ -314,13 +314,15 @@ std::vector Mi
   return filtered_modules;
 }
 
-const MinidumpExceptionStream *MinidumpParser::GetExceptionStream() {
-  llvm::ArrayRef data = GetStream(StreamType::Exception);
+const minidump::ExceptionStream *MinidumpParser::GetExceptionStream() {
+  auto ExpectedStream = GetMinidumpFile().getExceptionStream();
+  if (ExpectedStream)
+return &*ExpectedStream;
 
-  if (data.size() == 0)
-return nullptr;
-
-  return MinidumpExceptionStream::Parse(data);
+  LLDB_LOG_ERROR(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS),
+ ExpectedStream.takeError(),
+ "Failed to read minidump exception stream: {0}");
+  return nullptr;
 }
 
 llvm::Optional

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h?rev=375243&r1=375242&r2=375243&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h Fri Oct 18 
07:59:10 2019
@@ -82,7 +82,7 @@ public:
   // have the same name, it keeps the copy with the lowest load address.
   std::vector GetFilteredModuleList();
 
-  const MinidumpExceptionStream *GetExceptionStream();
+  const llvm::minidump::ExceptionStream *GetExceptionStream();
 
   llvm::Optional FindMemoryRange(lldb::addr_t addr);
 

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp?rev=375243&r1=375242&r2=375243&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp Fri Oct 18 
07:59:10 2019
@@ -57,17 +57,6 @@ LinuxProcStatus::Parse(llvm::ArrayRef &data) {
-  const MinidumpExceptionStream *exception_stream = nullptr;
-  Status error = consumeObject(data, exception_stream);
-  if (error.Fail())
-return nullptr;
-
-  return exception_stream;
-}
-
 std::pair, uint64_t>
 MinidumpMemoryDescriptor64::ParseMemory64List(llvm::ArrayRef &data) {
   const llvm::support::ulittle64_t *mem_ranges_count;

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h?rev=375243&r1=375242&r2=375243&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h Fri Oct 18 
07:59:10 2019
@@ -118,35 +118,6 @@ private:
   LinuxProcStatus() = default;
 };
 
-// Exception stuff
-struct MinidumpException {
-  enum : unsigned {
-ExceptonInfoMaxParams = 15,
-DumpRequested = 0x,
-  };
-
-  llvm::support::ulittle32_t exception_code;
-  llvm::support::ulittle32_t exception_flags;
-  llvm::support::ulittle64_t exception_record;
-  llvm::support::ulittle64_t exception_address;
-  llvm::support::ulittle32_t number_parameters;
-  llvm::support::ulittle32_t unused_alignment;
-  llvm::support::ulittle64_t exception_information[ExceptonInfoMaxParams];
-};
-static_assert(sizeof(MinidumpException) == 152,
-  "sizeof MinidumpException is not correct!");
-
-struct MinidumpExceptionStream {
-  llvm::support::ulittle32_t thread_id;
-  llvm::support::ulittle32_t alignment;
-  MinidumpException exception_r

[Lldb-commits] [lldb] r375244 - ProcessMinidump: Suppress reporting stop for signal '0'

2019-10-18 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Fri Oct 18 08:02:16 2019
New Revision: 375244

URL: http://llvm.org/viewvc/llvm-project?rev=375244&view=rev
Log:
ProcessMinidump: Suppress reporting stop for signal '0'

Summary:
The minidump exception stream can report an exception record with
signal 0.  If we try to create a stop reason with signal zero, processing
of the stop event won't find anything, and the debugger will hang.
So, simply early-out of RefreshStateAfterStop in this case.

Also set the UnixSignals object in DoLoadCore as is done for
ProcessElfCore.

Reviewers: labath, clayborg, jfb

Reviewed By: labath, clayborg

Subscribers: dexonsmith, lldb-commits

Tags: #lldb

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

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_null_signal.yaml
Modified:

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

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=375244&r1=375243&r2=375244&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 Fri Oct 18 08:02:16 2019
@@ -153,9 +153,9 @@ class MiniDumpNewTestCase(TestBase):
 self.assertTrue(eip.IsValid())
 self.assertEqual(pc, eip.GetValueAsUnsigned())
 
-def test_snapshot_minidump(self):
+def test_snapshot_minidump_dump_requested(self):
 """Test that if we load a snapshot minidump file (meaning the process
-did not crash) there is no stop reason."""
+did not crash) with exception code "DUMP_REQUESTED" there is no stop 
reason."""
 # target create -c linux-x86_64_not_crashed.dmp
 self.dbg.CreateTarget(None)
 self.target = self.dbg.GetSelectedTarget()
@@ -163,6 +163,17 @@ class MiniDumpNewTestCase(TestBase):
 self.check_state()
 self.assertEqual(self.process.GetNumThreads(), 1)
 thread = self.process.GetThreadAtIndex(0)
+self.assertEqual(thread.GetStopReason(), lldb.eStopReasonNone)
+stop_description = thread.GetStopDescription(256)
+self.assertEqual(stop_description, "")
+
+def test_snapshot_minidump_null_exn_code(self):
+"""Test that if we load a snapshot minidump file (meaning the process
+did not crash) with exception code zero there is no stop reason."""
+self.process_from_yaml("linux-x86_64_null_signal.yaml")
+self.check_state()
+self.assertEqual(self.process.GetNumThreads(), 1)
+thread = self.process.GetThreadAtIndex(0)
 self.assertEqual(thread.GetStopReason(), lldb.eStopReasonNone)
 stop_description = thread.GetStopDescription(256)
 self.assertEqual(stop_description, "")

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_null_signal.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_null_signal.yaml?rev=375244&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_null_signal.yaml
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64_null_signal.yaml
 Fri Oct 18 08:02:16 2019
@@ -0,0 +1,25 @@
+--- !minidump
+Streams:
+  - Type:ThreadList
+Threads:
+  - Thread Id:   0x2177
+Context: 
+Stack:
+  Start of Memory Range: 0x7FFE2F689000
+  Content: 
+  - Type:Exception
+Thread ID:   0x2177
+Exception Record:
+  Exception Code:  0x
+  Exception Address: 0x00400582
+Thread Context:  
+  - Type:SystemInfo
+Processor Arch:  AMD64
+Platform ID: Linux
+  - Type:LinuxProcStatus
+Text: |
+  Name:busyloop
+  Umask:   0002
+  State:   t (tracing stop)
+  Pid: 8567
+...

Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=375244&r1=375243&r2=375244&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original)
+++ lldb/trunk/source/Plugi

[Lldb-commits] [lldb] r367691 - Fix PC adjustment in StackFrame::GetSymbolContext

2019-08-02 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Fri Aug  2 09:53:42 2019
New Revision: 367691

URL: http://llvm.org/viewvc/llvm-project?rev=367691&view=rev
Log:
Fix PC adjustment in StackFrame::GetSymbolContext

Summary:
Update StackFrame::GetSymbolContext to mirror the logic in
RegisterContextLLDB::InitializeNonZerothFrame that knows not to do the
pc decrement when the given frame is a signal trap handler frame or the
parent of one, because the pc may not follow a call in these frames.
Accomplish this by adding a behaves_like_zeroth_frame field to
lldb_private::StackFrame, set to true for the zeroth frame, for
signal handler frames, and for parents of signal handler frames.

Also add logic to propagate the signal handler flag from UnwindPlan to
the FrameType on the RegisterContextLLDB it generates, and factor out a
helper to resolve symbol and address range for an Address now that we
need to invoke it in four places.

Reviewers: jasonmolenda, clayborg, jfb

Reviewed By: jasonmolenda

Subscribers: labath, dexonsmith, lldb-commits

Tags: #lldb

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

Added:
lldb/trunk/lit/Unwind/Inputs/trap_frame_sym_ctx.s
lldb/trunk/lit/Unwind/trap_frame_sym_ctx.test
Modified:
lldb/trunk/include/lldb/Core/Address.h
lldb/trunk/include/lldb/Target/StackFrame.h
lldb/trunk/include/lldb/Target/Unwind.h
lldb/trunk/source/Core/Address.cpp
lldb/trunk/source/Plugins/Process/Utility/HistoryUnwind.cpp
lldb/trunk/source/Plugins/Process/Utility/HistoryUnwind.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.h
lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.cpp
lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.h
lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
lldb/trunk/source/Target/StackFrame.cpp
lldb/trunk/source/Target/StackFrameList.cpp

Modified: lldb/trunk/include/lldb/Core/Address.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Address.h?rev=367691&r1=367690&r2=367691&view=diff
==
--- lldb/trunk/include/lldb/Core/Address.h (original)
+++ lldb/trunk/include/lldb/Core/Address.h Fri Aug  2 09:53:42 2019
@@ -338,6 +338,23 @@ public:
   bool ResolveAddressUsingFileSections(lldb::addr_t addr,
const SectionList *sections);
 
+  /// Resolve this address to its containing function and optionally get
+  /// that function's address range.
+  ///
+  /// \param[out] sym_ctx
+  /// The symbol context describing the function in which this address lies
+  ///
+  /// \parm[out] addr_range_ptr
+  /// Pointer to the AddressRange to fill in with the function's address
+  /// range.  Caller may pass null if they don't need the address range.
+  ///
+  /// \return
+  /// Returns \b false if the function/symbol could not be resolved
+  /// or if the address range was requested and could not be resolved;
+  /// returns \b true otherwise.
+  bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx,
+lldb_private::AddressRange *addr_range_ptr = 
nullptr);
+
   /// Set the address to represent \a load_addr.
   ///
   /// The address will attempt to find a loaded section within \a target that

Modified: lldb/trunk/include/lldb/Target/StackFrame.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/StackFrame.h?rev=367691&r1=367690&r2=367691&view=diff
==
--- lldb/trunk/include/lldb/Target/StackFrame.h (original)
+++ lldb/trunk/include/lldb/Target/StackFrame.h Fri Aug  2 09:53:42 2019
@@ -108,17 +108,19 @@ public:
   StackFrame(const lldb::ThreadSP &thread_sp, lldb::user_id_t frame_idx,
  lldb::user_id_t concrete_frame_idx, lldb::addr_t cfa,
  bool cfa_is_valid, lldb::addr_t pc, Kind frame_kind,
- const SymbolContext *sc_ptr);
+ bool behaves_like_zeroth_frame, const SymbolContext *sc_ptr);
 
   StackFrame(const lldb::ThreadSP &thread_sp, lldb::user_id_t frame_idx,
  lldb::user_id_t concrete_frame_idx,
  const lldb::RegisterContextSP ®_context_sp, lldb::addr_t cfa,
- lldb::addr_t pc, const SymbolContext *sc_ptr);
+ lldb::addr_t pc, bool behaves_like_zeroth_frame,
+ const SymbolContext *sc_ptr);
 
   StackFrame(const lldb::ThreadSP &thread_sp, lldb::user_id_t frame_idx,
  lldb::user_id_t concrete_frame_idx,
  const lldb::RegisterContextSP ®_context_sp, lldb::addr_t cfa,
- const Address &pc, const SymbolContext *sc_ptr);
+ const Address &pc, bool behaves_like_zeroth_frame,
+ const SymbolContext *sc_ptr);
 
   ~StackFrame() override;
 
@@ -

[Lldb-commits] [lldb] r367706 - Use rip-relative addressing in asm test

2019-08-02 Thread Joseph Tremoulet via lldb-commits
Author: josepht
Date: Fri Aug  2 12:06:15 2019
New Revision: 367706

URL: http://llvm.org/viewvc/llvm-project?rev=367706&view=rev
Log:
Use rip-relative addressing in asm test

The absolute form is an error when targeting Darwin.

Modified:
lldb/trunk/lit/Unwind/Inputs/trap_frame_sym_ctx.s

Modified: lldb/trunk/lit/Unwind/Inputs/trap_frame_sym_ctx.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Unwind/Inputs/trap_frame_sym_ctx.s?rev=367706&r1=367705&r2=367706&view=diff
==
--- lldb/trunk/lit/Unwind/Inputs/trap_frame_sym_ctx.s (original)
+++ lldb/trunk/lit/Unwind/Inputs/trap_frame_sym_ctx.s Fri Aug  2 12:06:15 2019
@@ -18,7 +18,7 @@ asm_main:
 
 # install tramp as return address
 # (similar to signal return trampolines on some platforms)
-leaqtramp, %rax
+leaqtramp(%rip), %rax
 pushq   %rax
 jmp bar # call, with return address pointing to tramp
 


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


[Lldb-commits] [lldb] 61bfc70 - [lldb] GetSharedModule: Collect old modules in SmallVector

2020-10-30 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2020-10-30T15:14:31-04:00
New Revision: 61bfc703c3d36fbefc476cd3829065d983c1c792

URL: 
https://github.com/llvm/llvm-project/commit/61bfc703c3d36fbefc476cd3829065d983c1c792
DIFF: 
https://github.com/llvm/llvm-project/commit/61bfc703c3d36fbefc476cd3829065d983c1c792.diff

LOG: [lldb] GetSharedModule: Collect old modules in SmallVector

The various GetSharedModule methods have an optional out parameter for
the old module when a file has changed or been replaced, which the
Target uses to keep its module list current/correct.  We've been using
a single ModuleSP to track "the" old module, and this change switches
to using a SmallVector of ModuleSP, which has a couple benefits:
 - There are multiple codepaths which may discover an old module, and
   this centralizes the code for how to handle multiples in one place,
   in the Target code.  With the single ModuleSP, each place that may
   discover an old module is responsible for how it handles multiples,
   and the current code is inconsistent (some code paths drop the first
   old module, others drop the second).
 - The API will be more natural for identifying old modules in routines
   that work on sets, like ModuleList::ReplaceEquivalent (which I plan
   on updating to report old module(s) in a subsequent change to fix a
   bug).

I'm not convinced we can ever actually run into the case that multiple
old modules are found in the same GetOrCreateModule call, but I think
this change makes sense regardless, in light of the above.

When an old module is reported, Target::GetOrCreateModule calls
m_images.ReplaceModule, which doesn't allow multiple "old" modules; the
new code calls ReplaceModule for the first "old" module, and for any
subsequent old modules it logs the event and calls m_images.Remove.

Reviewed By: jingham

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

Added: 


Modified: 
lldb/include/lldb/Core/ModuleList.h
lldb/include/lldb/Target/Platform.h
lldb/source/Core/ModuleList.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
lldb/source/Target/Platform.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index ae1f6fdb20a2..c62021b4bf6b 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -443,12 +443,11 @@ class ModuleList {
 
   static bool ModuleIsInCache(const Module *module_ptr);
 
-  static Status GetSharedModule(const ModuleSpec &module_spec,
-lldb::ModuleSP &module_sp,
-const FileSpecList *module_search_paths_ptr,
-lldb::ModuleSP *old_module_sp_ptr,
-bool *did_create_ptr,
-bool always_create = false);
+  static Status
+  GetSharedModule(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
+  const FileSpecList *module_search_paths_ptr,
+  llvm::SmallVectorImpl *old_modules,
+  bool *did_create_ptr, bool always_create = false);
 
   static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
 

diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 3c480641a275..df46466655c3 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -301,11 +301,10 @@ class Platform : public PluginInterface {
   LocateExecutableScriptingResources(Target *target, Module &module,
  Stream *feedback_stream);
 
-  virtual Status GetSharedModule(const ModuleSpec &module_spec,
- Process *process, lldb::ModuleSP &module_sp,
- const FileSpecList *module_search_paths_ptr,
- lldb::ModuleSP *old_module_sp_ptr,
- bool *did_create_ptr);
+  virtual Status GetSharedModule(
+  const ModuleSpec &module_spec, Process *process,
+  lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr,
+  llvm::SmallVectorImpl *old_modules, bool 
*did_create_ptr);
 
   virtual bool GetModuleSpec(const FileSpec &module_file_spec,
  const ArchSpec &arch, Modu

[Lldb-commits] [lldb] d20aa7c - [lldb] Report old modules from ModuleList::ReplaceEquivalent

2020-10-30 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2020-10-30T15:14:32-04:00
New Revision: d20aa7ca422145fb4d07e16c1d0aa7de9e3554ea

URL: 
https://github.com/llvm/llvm-project/commit/d20aa7ca422145fb4d07e16c1d0aa7de9e3554ea
DIFF: 
https://github.com/llvm/llvm-project/commit/d20aa7ca422145fb4d07e16c1d0aa7de9e3554ea.diff

LOG: [lldb] Report old modules from ModuleList::ReplaceEquivalent

This allows the Target to update its module list when loading a shared
module replaces an equivalent one.

A testcase is added which hits this codepath -- without the fix, the
target reports libbreakpad.so twice in its module list.

Reviewed By: jingham

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

Added: 


Modified: 
lldb/include/lldb/Core/ModuleList.h
lldb/source/Core/ModuleList.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index c62021b4bf6b..d90b27e474ac 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -139,7 +139,13 @@ class ModuleList {
   ///
   /// \param[in] module_sp
   /// A shared pointer to a module to replace in this collection.
-  void ReplaceEquivalent(const lldb::ModuleSP &module_sp);
+  ///
+  /// \param[in] old_modules
+  /// Optional pointer to a vector which, if provided, will have shared
+  /// pointers to the replaced module(s) appended to it.
+  void ReplaceEquivalent(
+  const lldb::ModuleSP &module_sp,
+  llvm::SmallVectorImpl *old_modules = nullptr);
 
   /// Append a module to the module list, if it is not already there.
   ///

diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index a4b3b2b92f83..cf276ba65931 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -171,7 +171,9 @@ void ModuleList::Append(const ModuleSP &module_sp, bool 
notify) {
   AppendImpl(module_sp, notify);
 }
 
-void ModuleList::ReplaceEquivalent(const ModuleSP &module_sp) {
+void ModuleList::ReplaceEquivalent(
+const ModuleSP &module_sp,
+llvm::SmallVectorImpl *old_modules) {
   if (module_sp) {
 std::lock_guard guard(m_modules_mutex);
 
@@ -184,11 +186,14 @@ void ModuleList::ReplaceEquivalent(const ModuleSP 
&module_sp) {
 
 size_t idx = 0;
 while (idx < m_modules.size()) {
-  ModuleSP module_sp(m_modules[idx]);
-  if (module_sp->MatchesModuleSpec(equivalent_module_spec))
+  ModuleSP test_module_sp(m_modules[idx]);
+  if (test_module_sp->MatchesModuleSpec(equivalent_module_spec)) {
+if (old_modules)
+  old_modules->push_back(test_module_sp);
 RemoveImpl(m_modules.begin() + idx);
-  else
+  } else {
 ++idx;
+  }
 }
 // Now add the new module to the list
 Append(module_sp);
@@ -820,7 +825,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, 
ModuleSP &module_sp,
   *did_create_ptr = true;
 }
 
-shared_module_list.ReplaceEquivalent(module_sp);
+shared_module_list.ReplaceEquivalent(module_sp, old_modules);
 return error;
   }
 }
@@ -857,7 +862,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, 
ModuleSP &module_sp,
 if (did_create_ptr)
   *did_create_ptr = true;
 
-shared_module_list.ReplaceEquivalent(module_sp);
+shared_module_list.ReplaceEquivalent(module_sp, old_modules);
 return Status();
   }
 }
@@ -955,7 +960,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, 
ModuleSP &module_sp,
   if (did_create_ptr)
 *did_create_ptr = true;
 
-  shared_module_list.ReplaceEquivalent(module_sp);
+  shared_module_list.ReplaceEquivalent(module_sp, old_modules);
 }
   } else {
 located_binary_modulespec.GetFileSpec().GetPath(path, sizeof(path));

diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
index abd62500e7d6..dc28ec6d7acc 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -32,10 +32,10 @@ def verify_module(self, module, verify_path, verify_uuid):
 os.path.normcase(module.GetFileSpec().dirname or ""))
 self.assertEqual(verify_uuid, module.GetUUIDString())
 
-def get_minidump_modules(self, yaml_file):
+def get_minidump_modules(self, yaml_file, exe = None):
 minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + 
".dmp")
 self.yaml2obj(yaml_file, minidump_path)
-self.target = self.dbg.CreateTarget(None)
+self.target = self.dbg.CreateTarget(exe)
 self.process = self.target.L

[Lldb-commits] [lldb] 85dfcaa - [LLDB] MinidumpParser: Prefer executable module even at higher address

2021-01-14 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2021-01-14T13:17:57-05:00
New Revision: 85dfcaadc5f0920dc8ecbece6c786701b8f45ab4

URL: 
https://github.com/llvm/llvm-project/commit/85dfcaadc5f0920dc8ecbece6c786701b8f45ab4
DIFF: 
https://github.com/llvm/llvm-project/commit/85dfcaadc5f0920dc8ecbece6c786701b8f45ab4.diff

LOG: [LLDB] MinidumpParser: Prefer executable module even at higher address

When a program maps one of its own modules for reading, and then
crashes, breakpad can emit two entries for that module in the
ModuleList.  We have logic to identify this case by checking permissions
on mapped memory regions and report just the module with an executable
region.  As currently written, though, the check is asymmetric -- the
entry with the executable region must be the second one encountered for
the preference to kick in.

This change makes the logic symmetric, so that the first-encountered
module will similarly be preferred if it has an executable region but
the second-encountered module does not.  This happens for example when
the module in question is the executable itself, which breakpad likes to
report first -- we need to ignore the other entry for that module when
we see it later, even though it may be mapped at a lower virtual
address.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp 
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index e16f86cca1c2..61106ebcc430 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -391,19 +391,23 @@ std::vector 
MinidumpParser::GetFilteredModuleList() {
   filtered_modules.push_back(&module);
 } else {
   // We have a duplicate module entry. Check the linux regions to see if
-  // the module we already have is not really a mapped executable. If it
-  // isn't check to see if the current duplicate module entry is a real
-  // mapped executable, and if so, replace it. This can happen when a
-  // process mmap's in the file for an executable in order to read bytes
-  // from the executable file. A memory region mapping will exist for the
-  // mmap'ed version and for the loaded executable, but only one will have
-  // a consecutive region that is executable in the memory regions.
+  // either module is not really a mapped executable. If one but not the
+  // other is a real mapped executable, prefer the executable one. This
+  // can happen when a process mmap's in the file for an executable in
+  // order to read bytes from the executable file. A memory region mapping
+  // will exist for the mmap'ed version and for the loaded executable, but
+  // only one will have a consecutive region that is executable in the
+  // memory regions.
   auto dup_module = filtered_modules[iter->second];
   ConstString name(*ExpectedName);
-  if (!CheckForLinuxExecutable(name, linux_regions,
-   dup_module->BaseOfImage) &&
-  CheckForLinuxExecutable(name, linux_regions, module.BaseOfImage)) {
-filtered_modules[iter->second] = &module;
+  bool is_executable =
+  CheckForLinuxExecutable(name, linux_regions, module.BaseOfImage);
+  bool dup_is_executable =
+  CheckForLinuxExecutable(name, linux_regions, 
dup_module->BaseOfImage);
+
+  if (is_executable != dup_is_executable) {
+if (is_executable)
+  filtered_modules[iter->second] = &module;
 continue;
   }
   // This module has been seen. Modules are sometimes mentioned multiple

diff  --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp 
b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index 69046af283eb..e3f23c5fe33a 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -792,6 +792,47 @@ TEST_F(MinidumpParserTest, 
MinidumpDuplicateModuleMappedSecond) {
   EXPECT_EQ(0x400du, filtered_modules[0]->BaseOfImage);
 }
 
+TEST_F(MinidumpParserTest, MinidumpDuplicateModuleMappedSecondHigh) {
+  ASSERT_THAT_ERROR(SetUpFromYaml(R"(
+--- !minidump
+Streams:
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x400d3000
+Size of Image:   0x2000
+Module Name: '/usr/lib/libc.so'
+CodeView Record: ''
+  - Base of Image:   0x400d
+Size of Image:   0x1000
+Module Name: '/usr/lib/libc.so'
+CodeView Record: ''
+  - Type:LinuxMaps
+Text: |
+  400d-400d2000 r--p  b3:04 227/usr/lib/libc.so
+  400d2000-400d3000 rw-p 

[Lldb-commits] [lldb] 95b2e51 - Change Target::FindBreakpointsByName to return Expected

2019-12-04 Thread Joseph Tremoulet via lldb-commits

Author: Joseph Tremoulet
Date: 2019-12-04T09:57:15-05:00
New Revision: 95b2e516bd3e4587953e44bf062054ff84f2b057

URL: 
https://github.com/llvm/llvm-project/commit/95b2e516bd3e4587953e44bf062054ff84f2b057
DIFF: 
https://github.com/llvm/llvm-project/commit/95b2e516bd3e4587953e44bf062054ff84f2b057.diff

LOG: Change Target::FindBreakpointsByName to return Expected

Summary:
Using a BreakpointList corrupts the breakpoints' IDs because
BreakpointList::Add sets the ID, so use a vector instead, and
update the signature to return the vector wrapped in an
llvm::Expected which can propagate any error from the inner
call to StringIsBreakpointName.

Note that, despite the similar name, SBTarget::FindBreakpointsByName
doesn't suffer the same problem, because it uses a SBBreakpointList,
which is more like a BreakpointIDList than a BreakpointList under the
covers.

Add a check to TestBreakpointNames that, without this fix, notices the
ID getting mutated and fails.

Reviewers: jingham, JDevlieghere

Reviewed By: JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/BreakpointList.h

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
lldb/source/API/SBTarget.cpp
lldb/source/Breakpoint/BreakpointList.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/BreakpointList.h 
b/lldb/include/lldb/Breakpoint/BreakpointList.h
index 110e8d41f36b..ad68151fefc7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -67,8 +67,10 @@ class BreakpointList {
   ///   The breakpoint name for which to search.
   ///
   /// \result
-  ///   \bfalse if the input name was not a legal breakpoint name.
-  bool FindBreakpointsByName(const char *name, BreakpointList &matching_bps);
+  ///   error if the input name was not a legal breakpoint name, vector
+  ///   of breakpoints otherwise.
+  llvm::Expected>
+  FindBreakpointsByName(const char *name);
 
   /// Returns the number of elements in this breakpoint list.
   ///

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
index 4a5ed87e330f..9513278ba084 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -155,8 +155,13 @@ def do_check_illegal_names(self):
 def do_check_using_names(self):
 """Use Python APIs to check names work in place of breakpoint ID's."""
 
+# Create a dummy breakpoint to use up ID 1
+_ = self.target.BreakpointCreateByLocation(self.main_file_spec, 30)
+
+# Create a breakpiont to test with
 bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
 bkpt_name = "ABreakpoint"
+bkpt_id = bkpt.GetID()
 other_bkpt_name= "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
@@ -169,6 +174,7 @@ def do_check_using_names(self):
 self.assertTrue(bkpts.GetSize() == 1, "One breakpoint matched.")
 found_bkpt = bkpts.GetBreakpointAtIndex(0)
 self.assertTrue(bkpt.GetID() == found_bkpt.GetID(),"The right 
breakpoint.")
+self.assertTrue(bkpt.GetID() == bkpt_id,"With the same ID as before.")
 
 retval = lldb.SBCommandReturnObject()
 self.dbg.GetCommandInterpreter().HandleCommand("break disable 
%s"%(bkpt_name), retval)

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 7013e2b45e5f..312e4df75863 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1176,12 +1176,15 @@ bool SBTarget::FindBreakpointsByName(const char *name,
   TargetSP target_sp(GetSP());
   if (target_sp) {
 std::lock_guard guard(target_sp->GetAPIMutex());
-BreakpointList bkpt_list(false);
-bool is_valid =
-target_sp->GetBreakpointList().FindBreakpointsByName(name, bkpt_list);
-if (!is_valid)
+llvm::Expected> expected_vector =
+target_sp->GetBreakpointList().FindBreakpointsByName(name);
+if (!expected_vector) {
+  LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS),
+   "invalid breakpoint name: {}",
+   llvm::toString(expected_vector.takeError()));
   return false;
-for (BreakpointSP bkpt_sp : bkpt_list.Breakpoints()) {
+}
+for (BreakpointSP bkpt_sp : *expected_vector) {
   bkpts.AppendByID(bkpt_sp->GetID());
 }
   }

diff  --git a/lldb/source/Breakpoint/BreakpointList.cpp 
b/lldb/source/Breakpoint/BreakpointList.cpp
i