Skip to content

Commit abeec5d

Browse files
JosephTremoulettstellar
authored andcommitted
[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 d20aa7c)
1 parent b618cf7 commit abeec5d

File tree

3 files changed

+39
-10
lines changed

3 files changed

+39
-10
lines changed

lldb/include/lldb/Core/ModuleList.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,13 @@ class ModuleList {
139139
///
140140
/// \param[in] module_sp
141141
/// A shared pointer to a module to replace in this collection.
142-
void ReplaceEquivalent(const lldb::ModuleSP &module_sp);
142+
///
143+
/// \param[in] old_modules
144+
/// Optional pointer to a vector which, if provided, will have shared
145+
/// pointers to the replaced module(s) appended to it.
146+
void ReplaceEquivalent(
147+
const lldb::ModuleSP &module_sp,
148+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules = nullptr);
143149

144150
/// Append a module to the module list, if it is not already there.
145151
///

lldb/source/Core/ModuleList.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,9 @@ void ModuleList::Append(const ModuleSP &module_sp, bool notify) {
171171
AppendImpl(module_sp, notify);
172172
}
173173

174-
void ModuleList::ReplaceEquivalent(const ModuleSP &module_sp) {
174+
void ModuleList::ReplaceEquivalent(
175+
const ModuleSP &module_sp,
176+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules) {
175177
if (module_sp) {
176178
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
177179

@@ -184,11 +186,14 @@ void ModuleList::ReplaceEquivalent(const ModuleSP &module_sp) {
184186

185187
size_t idx = 0;
186188
while (idx < m_modules.size()) {
187-
ModuleSP module_sp(m_modules[idx]);
188-
if (module_sp->MatchesModuleSpec(equivalent_module_spec))
189+
ModuleSP test_module_sp(m_modules[idx]);
190+
if (test_module_sp->MatchesModuleSpec(equivalent_module_spec)) {
191+
if (old_modules)
192+
old_modules->push_back(test_module_sp);
189193
RemoveImpl(m_modules.begin() + idx);
190-
else
194+
} else {
191195
++idx;
196+
}
192197
}
193198
// Now add the new module to the list
194199
Append(module_sp);
@@ -810,7 +815,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
810815
*did_create_ptr = true;
811816
}
812817

813-
shared_module_list.ReplaceEquivalent(module_sp);
818+
shared_module_list.ReplaceEquivalent(module_sp, old_modules);
814819
return error;
815820
}
816821
}
@@ -847,7 +852,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
847852
if (did_create_ptr)
848853
*did_create_ptr = true;
849854

850-
shared_module_list.ReplaceEquivalent(module_sp);
855+
shared_module_list.ReplaceEquivalent(module_sp, old_modules);
851856
return Status();
852857
}
853858
}
@@ -945,7 +950,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
945950
if (did_create_ptr)
946951
*did_create_ptr = true;
947952

948-
shared_module_list.ReplaceEquivalent(module_sp);
953+
shared_module_list.ReplaceEquivalent(module_sp, old_modules);
949954
}
950955
} else {
951956
located_binary_modulespec.GetFileSpec().GetPath(path, sizeof(path));

lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ def verify_module(self, module, verify_path, verify_uuid):
3131
os.path.normcase(module.GetFileSpec().dirname or ""))
3232
self.assertEqual(verify_uuid, module.GetUUIDString())
3333

34-
def get_minidump_modules(self, yaml_file):
34+
def get_minidump_modules(self, yaml_file, exe = None):
3535
minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
3636
self.yaml2obj(yaml_file, minidump_path)
37-
self.target = self.dbg.CreateTarget(None)
37+
self.target = self.dbg.CreateTarget(exe)
3838
self.process = self.target.LoadCore(minidump_path)
3939
return self.target.modules
4040

@@ -265,6 +265,24 @@ def test_breakpad_overflow_hash_match(self):
265265
# will check that this matches.
266266
self.verify_module(modules[0], so_path, "48EB9FD7")
267267

268+
def test_breakpad_hash_match_exe_outside_sysroot(self):
269+
"""
270+
Check that we can match the breakpad .text section hash when the
271+
module is specified as the exe during launch, and a syroot is
272+
provided, which does not contain the exe.
273+
"""
274+
sysroot_path = os.path.join(self.getBuildDir(), "mock_sysroot")
275+
lldbutil.mkdir_p(sysroot_path)
276+
so_dir = os.path.join(self.getBuildDir(), "binary")
277+
so_path = os.path.join(so_dir, "libbreakpad.so")
278+
lldbutil.mkdir_p(so_dir)
279+
self.yaml2obj("libbreakpad.yaml", so_path)
280+
self.runCmd("platform select remote-linux --sysroot '%s'" % sysroot_path)
281+
modules = self.get_minidump_modules("linux-arm-breakpad-uuid-match.yaml", so_path)
282+
self.assertEqual(1, len(modules))
283+
# LLDB makes up its own UUID as well when there is no build ID so we
284+
# will check that this matches.
285+
self.verify_module(modules[0], so_path, "D9C480E8")
268286

269287
def test_facebook_hash_match(self):
270288
"""

0 commit comments

Comments
 (0)