Skip to content

Commit 44a1f7e

Browse files
authored
[lldb] Fix unsafe map mutation in ProcessElfCore::FindModuleUUID (#159444)
The `ProcessElfCore::FindModuleUUID` function can be called by multiple threads at the same time when `target.parallel-module-load` is true. We were using the `operator[]` to lookup the UUID which will mutate the map when the key is not present. This is unsafe in a multi-threaded contex so we now use a read-only `find` operation and explicitly return an invalid UUID when the key is not present. The `m_uuids` map can follow a create-then-query pattern. It is populated in the `DoLoadCore` function which looks like it is only called in a single-threaded context so we do not need extra locking as long as we keep the other accesses read-only. Other fixes I considered * Use a lock to protect access - We don't need to modify the map after creation so we can allow concurrent read-only access. * Store the map in a const pointer container to prevent accidental mutation in other places. - Only accessed in one place currently so just added a comment. * Store the UUID in the NT_FILE_Entry after building the mapping correctly in `UpdateBuildIdForNTFileEntries`. - The map lets us avoid a linear search in `FindModuleUUID`. This commit also reverts the temporary workaround from #159395 which disabled parallel module loading to avoid the test failure. Fixes #159377
1 parent 6e47bff commit 44a1f7e

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,13 @@ llvm::StringRef ProcessElfCore::GetMainExecutablePath() {
308308
}
309309

310310
UUID ProcessElfCore::FindModuleUUID(const llvm::StringRef path) {
311-
return m_uuids[std::string(path)];
311+
// Lookup the UUID for the given path in the map.
312+
// Note that this could be called by multiple threads so make sure
313+
// we access the map in a thread safe way (i.e. don't use operator[]).
314+
auto it = m_uuids.find(std::string(path));
315+
if (it != m_uuids.end())
316+
return it->second;
317+
return UUID();
312318
}
313319

314320
lldb_private::DynamicLoader *ProcessElfCore::GetDynamicLoader() {

lldb/test/API/functionalities/postmortem/netbsd-core/TestNetBSDCore.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,6 @@ def check_backtrace(self, thread, filename, backtrace):
117117
)
118118

119119
def do_test(self, filename, pid, region_count):
120-
# Temporary workaround for https://github.com/llvm/llvm-project/issues/159377
121-
self.runCmd("settings set target.parallel-module-load false")
122120
target = self.dbg.CreateTarget(filename)
123121
process = target.LoadCore(filename + ".core")
124122

0 commit comments

Comments
 (0)