From e58ab432e05c870ee760c60053bc3291dce8735a Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Mon, 7 Jul 2025 11:53:45 -0700 Subject: [PATCH 1/2] [lldb] Replace PathMappingList::GetPathsAtIndex with thread-safe alternative Iteration by index is not thread-safe with PathMapingList's design. Instead, we can expose a `ForEach` method that grabs the PathMappingList's lock before the callback. There is a potential pitfall here where the callback tries to call another PathMappingList method (causing a deadlock), but that seems a little to debug than the PathMappingList getting updated in the middle of a loop somewhere. --- lldb/include/lldb/Target/PathMappingList.h | 6 ++++-- lldb/source/Commands/CommandObjectTarget.cpp | 20 +++++++++++--------- lldb/source/Target/PathMappingList.cpp | 13 +++++-------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h index 2cf9569358f52..02fee4d8ed45f 100644 --- a/lldb/include/lldb/Target/PathMappingList.h +++ b/lldb/include/lldb/Target/PathMappingList.h @@ -61,8 +61,10 @@ class PathMappingList { return m_pairs.size(); } - bool GetPathsAtIndex(uint32_t idx, ConstString &path, - ConstString &new_path) const; + /// Invokes callback for each pair of paths in the list. The callback can + /// return false to immediately stop iteration. + void + ForEach(std::function callback) const; void Insert(llvm::StringRef path, llvm::StringRef replacement, uint32_t insert_idx, bool notify); diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index fe421adfe8709..e8ebcb8008ff9 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -1139,15 +1139,17 @@ class CommandObjectTargetModulesSearchPathsInsert : public CommandObjectParsed { Target *target = m_exe_ctx.GetTargetPtr(); const PathMappingList &list = target->GetImageSearchPathList(); - const size_t num = list.GetSize(); - ConstString old_path, new_path; - for (size_t i = 0; i < num; ++i) { - if (!list.GetPathsAtIndex(i, old_path, new_path)) - break; - StreamString strm; - strm << old_path << " -> " << new_path; - request.TryCompleteCurrentArg(std::to_string(i), strm.GetString()); - } + + StreamString strm; + size_t index = 0; + list.ForEach([&](llvm::StringRef orig_path, llvm::StringRef remap_path) { + strm << orig_path << " -> " << remap_path; + request.TryCompleteCurrentArg(std::to_string(index), strm.GetString()); + + index++; + strm.Clear(); + return true; + }); } protected: diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp index 0465b4280bbb1..d8dd9dde7a307 100644 --- a/lldb/source/Target/PathMappingList.cpp +++ b/lldb/source/Target/PathMappingList.cpp @@ -346,15 +346,12 @@ PathMappingList::FindIteratorForPath(ConstString path) { return pos; } -bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path, - ConstString &new_path) const { +void PathMappingList::ForEach( + std::function callback) const { std::lock_guard lock(m_pairs_mutex); - if (idx < m_pairs.size()) { - path = m_pairs[idx].first; - new_path = m_pairs[idx].second; - return true; - } - return false; + for (const auto &[original, replacement] : m_pairs) + if (!callback(original, replacement)) + break; } uint32_t From 7e87cf822392ec94000f8d6a8980261d184a24da Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Mon, 7 Jul 2025 14:00:35 -0700 Subject: [PATCH 2/2] fixup! [lldb] Replace PathMappingList::GetPathsAtIndex with thread-safe alternative --- lldb/include/lldb/Target/PathMappingList.h | 4 ++-- lldb/source/Commands/CommandObjectTarget.cpp | 5 ++--- lldb/source/Target/PathMappingList.cpp | 6 ++++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h index 02fee4d8ed45f..82c87ebbbcda2 100644 --- a/lldb/include/lldb/Target/PathMappingList.h +++ b/lldb/include/lldb/Target/PathMappingList.h @@ -63,8 +63,8 @@ class PathMappingList { /// Invokes callback for each pair of paths in the list. The callback can /// return false to immediately stop iteration. - void - ForEach(std::function callback) const; + void ForEach(std::function + callback) const; void Insert(llvm::StringRef path, llvm::StringRef replacement, uint32_t insert_idx, bool notify); diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index e8ebcb8008ff9..2eb58471c2960 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -1141,12 +1141,11 @@ class CommandObjectTargetModulesSearchPathsInsert : public CommandObjectParsed { const PathMappingList &list = target->GetImageSearchPathList(); StreamString strm; - size_t index = 0; - list.ForEach([&](llvm::StringRef orig_path, llvm::StringRef remap_path) { + list.ForEach([&](size_t index, llvm::StringRef orig_path, + llvm::StringRef remap_path) { strm << orig_path << " -> " << remap_path; request.TryCompleteCurrentArg(std::to_string(index), strm.GetString()); - index++; strm.Clear(); return true; }); diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp index d8dd9dde7a307..00ca72b4faed7 100644 --- a/lldb/source/Target/PathMappingList.cpp +++ b/lldb/source/Target/PathMappingList.cpp @@ -347,10 +347,12 @@ PathMappingList::FindIteratorForPath(ConstString path) { } void PathMappingList::ForEach( - std::function callback) const { + std::function callback) + const { std::lock_guard lock(m_pairs_mutex); + size_t index = 0; for (const auto &[original, replacement] : m_pairs) - if (!callback(original, replacement)) + if (!callback(index++, original, replacement)) break; }