[Lldb-commits] [lldb] 20f8425 - [lldb] Fix GetRemoteSharedModule fallback logic
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
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
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
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
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
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]
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'
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
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
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
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
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
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
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