Author: Joseph Tremoulet Date: 2020-12-11T17:17:24-05:00 New Revision: abeec5d081f0dfbee305b70d7641c8e5af9a9335
URL: https://github.com/llvm/llvm-project/commit/abeec5d081f0dfbee305b70d7641c8e5af9a9335 DIFF: https://github.com/llvm/llvm-project/commit/abeec5d081f0dfbee305b70d7641c8e5af9a9335.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 (cherry picked from commit d20aa7ca422145fb4d07e16c1d0aa7de9e3554ea) 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<lldb::ModuleSP> *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 76a861e33d0d..1701cb56338e 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<lldb::ModuleSP> *old_modules) { if (module_sp) { std::lock_guard<std::recursive_mutex> 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); @@ -810,7 +815,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; } } @@ -847,7 +852,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(); } } @@ -945,7 +950,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 619f94a2cbb0..60ec48b459a9 100644 --- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py +++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py @@ -31,10 +31,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.LoadCore(minidump_path) return self.target.modules @@ -265,6 +265,24 @@ def test_breakpad_overflow_hash_match(self): # will check that this matches. self.verify_module(modules[0], so_path, "48EB9FD7") + def test_breakpad_hash_match_exe_outside_sysroot(self): + """ + Check that we can match the breakpad .text section hash when the + module is specified as the exe during launch, and a syroot is + provided, which does not contain the exe. + """ + sysroot_path = os.path.join(self.getBuildDir(), "mock_sysroot") + lldbutil.mkdir_p(sysroot_path) + so_dir = os.path.join(self.getBuildDir(), "binary") + so_path = os.path.join(so_dir, "libbreakpad.so") + lldbutil.mkdir_p(so_dir) + self.yaml2obj("libbreakpad.yaml", so_path) + self.runCmd("platform select remote-linux --sysroot '%s'" % sysroot_path) + modules = self.get_minidump_modules("linux-arm-breakpad-uuid-match.yaml", so_path) + self.assertEqual(1, len(modules)) + # LLDB makes up its own UUID as well when there is no build ID so we + # will check that this matches. + self.verify_module(modules[0], so_path, "D9C480E8") def test_facebook_hash_match(self): """ _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits