[Lldb-commits] [lldb] [lldb] Preserve TargetSP in Platform module lookup (PR #186323)
Emre Kultursay via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 13 11:49:02 PDT 2026
https://github.com/emrekultursay updated https://github.com/llvm/llvm-project/pull/186323
>From 89de82b9ebb69665e104b7b62c4332d5d4adf092 Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultursay at google.com>
Date: Fri, 13 Mar 2026 06:08:32 +0000
Subject: [PATCH 1/5] [lldb] Preserve TargetSP in Platform module lookup
When Platform::GetRemoteSharedModule resolves a module specification
by calling GetModuleSpec on the process, the target context
(TargetSP) is lost. This happens because the underlying process
plugin populates a new ModuleSpec object that does not inherit
the target context.
This causes failures in subsequent symbol resolution steps. For
example, when ModuleList relies on the TargetSP to query
target-specific settings like target.exec-search-paths and
target.symbols-search-paths, the lookup fails because TargetSP
evaluates to null. This prevents B from finding local unstripped
libraries.
This commit fixes the issue by transferring the target context from
the original module_spec over to the resolved_module_spec before
proceeding with the lookup.
Additionally, this commit avoids redundant calls to the locate module
callback. Since the platform already handles the callback explicitly,
passing the target context to subsequent ModuleList::GetSharedModule
calls would normally trigger the callback again redundantly. This is
suppressed by passing invoke_locate_callback=false in
Platform::GetSharedModule and ModuleCache::Get. This also avoids
triggering the callback with incomplete module specifications (e.g.
cleared UUIDs) during cache lookups.
---
lldb/source/Target/ModuleCache.cpp | 2 +-
lldb/source/Target/Platform.cpp | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Target/ModuleCache.cpp b/lldb/source/Target/ModuleCache.cpp
index 8160ab1fbb2c3..9978946105456 100644
--- a/lldb/source/Target/ModuleCache.cpp
+++ b/lldb/source/Target/ModuleCache.cpp
@@ -255,7 +255,7 @@ Status ModuleCache::Get(const FileSpec &root_dir_spec, const char *hostname,
cached_module_spec.GetPlatformFileSpec() = module_spec.GetFileSpec();
error = ModuleList::GetSharedModule(cached_module_spec, cached_module_sp,
- nullptr, did_create_ptr);
+ nullptr, did_create_ptr, false);
if (error.Fail())
return error;
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index c47ef47b0f60c..2978eeef07cbd 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -182,14 +182,14 @@ Status Platform::GetSharedModule(
resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
// Try to get shared module with resolved spec.
error = ModuleList::GetSharedModule(resolved_spec, module_sp, old_modules,
- did_create_ptr);
+ did_create_ptr, false);
}
// If we don't have sysroot or it didn't work then
// try original module spec.
if (!error.Success()) {
resolved_spec = spec;
error = ModuleList::GetSharedModule(resolved_spec, module_sp, old_modules,
- did_create_ptr);
+ did_create_ptr, false);
}
if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
@@ -1525,6 +1525,10 @@ Status Platform::GetRemoteSharedModule(const ModuleSpec &module_spec,
resolved_module_spec.GetUUID() = module_spec.GetUUID();
}
+ // Retain the target context from the original module_spec since
+ // process->GetModuleSpec might have cleared it.
+ resolved_module_spec.SetTarget(module_spec.GetTargetSP());
+
// Call locate module callback if set. This allows users to implement their
// own module cache system. For example, to leverage build system artifacts,
// to bypass pulling files from remote platform, or to search symbol files
>From efb1a783fafc29e296dcab60e2bc3c21272b9993 Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultursay at google.com>
Date: Fri, 13 Mar 2026 17:14:27 +0000
Subject: [PATCH 2/5] Add inline comments with the argument name
---
lldb/source/Target/ModuleCache.cpp | 3 ++-
lldb/source/Target/Platform.cpp | 10 ++++++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Target/ModuleCache.cpp b/lldb/source/Target/ModuleCache.cpp
index 9978946105456..3d2812b0966e4 100644
--- a/lldb/source/Target/ModuleCache.cpp
+++ b/lldb/source/Target/ModuleCache.cpp
@@ -255,7 +255,8 @@ Status ModuleCache::Get(const FileSpec &root_dir_spec, const char *hostname,
cached_module_spec.GetPlatformFileSpec() = module_spec.GetFileSpec();
error = ModuleList::GetSharedModule(cached_module_spec, cached_module_sp,
- nullptr, did_create_ptr, false);
+ nullptr, did_create_ptr,
+ /*invoke_locate_callback=*/false);
if (error.Fail())
return error;
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 2978eeef07cbd..e79b888c0fc38 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -181,15 +181,17 @@ Status Platform::GetSharedModule(
resolved_spec = spec;
resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
// Try to get shared module with resolved spec.
- error = ModuleList::GetSharedModule(resolved_spec, module_sp, old_modules,
- did_create_ptr, false);
+ error = ModuleList::GetSharedModule(
+ resolved_spec, module_sp, old_modules, did_create_ptr,
+ /*invoke_locate_callback=*/false);
}
// If we don't have sysroot or it didn't work then
// try original module spec.
if (!error.Success()) {
resolved_spec = spec;
- error = ModuleList::GetSharedModule(resolved_spec, module_sp, old_modules,
- did_create_ptr, false);
+ error = ModuleList::GetSharedModule(
+ resolved_spec, module_sp, old_modules, did_create_ptr,
+ /*invoke_locate_callback=*/false);
}
if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
>From 615615e2fc7545e292961bf6a247f808c5d86d5c Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultursay at google.com>
Date: Fri, 13 Mar 2026 18:30:51 +0000
Subject: [PATCH 3/5] Fix formatting issues
---
lldb/source/Target/Platform.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index e79b888c0fc38..647f8389dc41b 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -181,17 +181,17 @@ Status Platform::GetSharedModule(
resolved_spec = spec;
resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
// Try to get shared module with resolved spec.
- error = ModuleList::GetSharedModule(
- resolved_spec, module_sp, old_modules, did_create_ptr,
- /*invoke_locate_callback=*/false);
+ error = ModuleList::GetSharedModule(resolved_spec, module_sp, old_modules,
+ did_create_ptr,
+ /*invoke_locate_callback=*/false);
}
// If we don't have sysroot or it didn't work then
// try original module spec.
if (!error.Success()) {
resolved_spec = spec;
- error = ModuleList::GetSharedModule(
- resolved_spec, module_sp, old_modules, did_create_ptr,
- /*invoke_locate_callback=*/false);
+ error = ModuleList::GetSharedModule(resolved_spec, module_sp, old_modules,
+ did_create_ptr,
+ /*invoke_locate_callback=*/false);
}
if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
>From 82e017057adc2bfec62eda68d21be5a537cdb70d Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultursay at google.com>
Date: Fri, 13 Mar 2026 18:33:34 +0000
Subject: [PATCH 4/5] Add new test case
---
.../Target/LocateModuleCallbackTest.cpp | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/lldb/unittests/Target/LocateModuleCallbackTest.cpp b/lldb/unittests/Target/LocateModuleCallbackTest.cpp
index 94c082b344262..0ac3cab5c5299 100644
--- a/lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ b/lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -1000,3 +1000,41 @@ TEST_F(LocateModuleCallbackTest,
FileSpec(GetInputFilePath(k_module_file)));
ModuleList::RemoveSharedModule(m_module_sp);
}
+
+TEST_F(LocateModuleCallbackTest, ModuleCacheGetDoesNotInvokeCallback) {
+ // The module file is cached. ModuleCache::Get should succeed to return the
+ // module from the cache without calling the locate module callback.
+ FileSpec uuid_view = BuildCacheDir(m_test_dir);
+
+ int callback_call_count = 0;
+ m_platform_sp->SetLocateModuleCallback(
+ [&callback_call_count](const ModuleSpec &module_spec,
+ FileSpec &module_file_spec,
+ FileSpec &symbol_file_spec) {
+ callback_call_count++;
+ return Status();
+ });
+
+ ModuleCache mc;
+ ModuleSP cached_module_sp;
+ bool did_create = false;
+ FileSpec root_dir_spec(m_test_dir);
+ root_dir_spec.AppendPathComponent(k_platform_dir);
+ Status error = mc.GetAndPut(
+ root_dir_spec, "hostname", m_module_spec,
+ [](const ModuleSpec &module_spec,
+ const FileSpec &tmp_download_file_spec) {
+ return Status::FromErrorString("Should not be called");
+ },
+ [](const ModuleSP &module_sp, const FileSpec &tmp_download_file_spec) {
+ return Status::FromErrorString("Should not be called");
+ },
+ cached_module_sp, &did_create);
+
+ ASSERT_TRUE(error.Success());
+ ASSERT_TRUE(cached_module_sp);
+ // The locate module callback should not be called.
+ EXPECT_EQ(callback_call_count, 0);
+
+ ModuleList::RemoveSharedModule(cached_module_sp);
+}
>From cd89e36835db6e7650fcb9cdd75578f485ba4c9a Mon Sep 17 00:00:00 2001
From: Emre Kultursay <emrekultursay at google.com>
Date: Fri, 13 Mar 2026 18:44:58 +0000
Subject: [PATCH 5/5] Add missing include
---
lldb/unittests/Target/LocateModuleCallbackTest.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/unittests/Target/LocateModuleCallbackTest.cpp b/lldb/unittests/Target/LocateModuleCallbackTest.cpp
index 0ac3cab5c5299..c6ca6a56b1001 100644
--- a/lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ b/lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -16,6 +16,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/ModuleCache.h"
#include "lldb/Target/Target.h"
#include "gmock/gmock.h"
More information about the lldb-commits
mailing list