Skip to content

[lldb] Replace PathMappingList::GetPathsAtIndex with thread-safe alternative #147396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+21 −19
Open

[lldb] Replace PathMappingList::GetPathsAtIndex with thread-safe alternative #147396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e58ab43
[lldb] Replace PathMappingList::GetPathsAtIndex with thread-safe alte…
bulbazord Jul 7, 2025
7e87cf8
fixup! [lldb] Replace PathMappingList::GetPathsAtIndex with thread-sa…
bulbazord Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lldb/include/lldb/Target/PathMappingList.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool(size_t, llvm::StringRef, llvm::StringRef)>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method used a lot? Could we migrate to using the LockingAdaptedIterable instead of match how other classes handle this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only used in one place. I'll look at LockingAdaptedIterable and see how bad it is.

callback) const;

void Insert(llvm::StringRef path, llvm::StringRef replacement,
uint32_t insert_idx, bool notify);
Expand Down
19 changes: 10 additions & 9 deletions lldb/source/Commands/CommandObjectTarget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,15 +1139,16 @@ 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;
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());

strm.Clear();
return true;
});
}

protected:
Expand Down
15 changes: 7 additions & 8 deletions lldb/source/Target/PathMappingList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,14 @@ PathMappingList::FindIteratorForPath(ConstString path) {
return pos;
}

bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
ConstString &new_path) const {
void PathMappingList::ForEach(
std::function<bool(size_t, llvm::StringRef, llvm::StringRef)> callback)
const {
std::lock_guard<std::mutex> lock(m_pairs_mutex);
if (idx < m_pairs.size()) {
path = m_pairs[idx].first;
new_path = m_pairs[idx].second;
return true;
}
return false;
size_t index = 0;
for (const auto &[original, replacement] : m_pairs)
if (!callback(index++, original, replacement))
break;
}

uint32_t
Expand Down
Loading