[lldb] Enable locate module callback for main executable #160199

GeorgeHuyubo · 2025-09-22T20:57:55Z

Main executables were bypassing the locate module callback that shared
libraries use, preventing custom symbol file location logic from working
consistently.

This PR fix this by

Adding target context to ModuleSpec
Leveraging that context to use target search path and platform's locate module callback in ModuleList::GetSharedModule

This ensures both main executables and shared libraries get the same
callback treatment for symbol file resolution.

llvmbot · 2025-09-22T21:54:32Z

@llvm/pr-subscribers-lldb

Author: None (GeorgeHuyubo)

Changes

Main executables were bypassing the locate module callback that shared
libraries use, preventing custom symbol file location logic from working
consistently.

Fixed by modifying Platform::ResolveExecutable() to call
CallLocateModuleCallbackIfSet() first, then fallback to the standard
ModuleList::GetSharedModule() path if needed.

This ensures both main executables and shared libraries get the same
callback treatment for symbol file resolution.

Full diff: https://github.com/llvm/llvm-project/pull/160199.diff

1 Files Affected:

(modified) lldb/source/Target/Platform.cpp (+41-9)

diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 8681adaf5ea76..bbbe066cdea9e 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -750,12 +750,30 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
 
   if (resolved_module_spec.GetArchitecture().IsValid() ||
       resolved_module_spec.GetUUID().IsValid()) {
-    Status error =
-        ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-                                    module_search_paths_ptr, nullptr, nullptr);
+    // Call locate module callback first to give it a chance to find/register
+    // symbol file specs for the main executable, similar to how shared
+    // libraries are handled in Platform::GetRemoteSharedModule()
+    FileSpec symbol_file_spec;
+    CallLocateModuleCallbackIfSet(resolved_module_spec, exe_module_sp,
+                                  symbol_file_spec, nullptr);
 
-    if (exe_module_sp && exe_module_sp->GetObjectFile())
-      return error;
+    Status error;
+    if (!exe_module_sp) {
+      // If locate module callback didn't provide a module, fallback to standard
+      // path
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          module_search_paths_ptr, nullptr,
+                                          nullptr);
+    }
+
+    if (exe_module_sp && exe_module_sp->GetObjectFile()) {
+      // Set the symbol file if locate module callback returned one
+      if (symbol_file_spec) {
+        exe_module_sp->SetSymbolFileFileSpec(symbol_file_spec);
+      }
+      return error; // Return the actual status from GetSharedModule (or success
+                    // from callback)
+    }
     exe_module_sp.reset();
   }
   // No valid architecture was specified or the exact arch wasn't found.
@@ -767,12 +785,26 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
   Status error;
   for (const ArchSpec &arch : GetSupportedArchitectures(process_host_arch)) {
     resolved_module_spec.GetArchitecture() = arch;
-    error =
-        ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-                                    module_search_paths_ptr, nullptr, nullptr);
+
+    // Call locate module callback first, then fallback to standard path
+    FileSpec symbol_file_spec;
+    CallLocateModuleCallbackIfSet(resolved_module_spec, exe_module_sp,
+                                  symbol_file_spec, nullptr);
+
+    if (!exe_module_sp) {
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          module_search_paths_ptr, nullptr,
+                                          nullptr);
+    }
+
     if (error.Success()) {
-      if (exe_module_sp && exe_module_sp->GetObjectFile())
+      if (exe_module_sp && exe_module_sp->GetObjectFile()) {
+        // Set the symbol file if locate module callback returned one
+        if (symbol_file_spec) {
+          exe_module_sp->SetSymbolFileFileSpec(symbol_file_spec);
+        }
         break;
+      }
       error = Status::FromErrorString("no exe object file");
     }

jeffreytan81 · 2025-09-22T22:19:20Z

This works functionality wise but I have two concerns:

How will this play with dap module reuse feature? We seem to prioritize module locate callback (which usually consults symbol server) than global module list. That means if we enabled module locate callback, we probably won't reuse loaded module for incremental debug session? I wonder if we should move the module locate callback to the end of the list only when other approaches fail? @clayborg, what do you think?
It sucks we have to have two code paths for locating modules, one for shared libraries, one for main executable. I wonder if we should consolidate to a central place? Like ModuleList::GetSharedModule?

jimingham · 2025-09-22T23:07:37Z

When you are asked to resolve a shared library, it's always real when you first hear about it, because it's always the dynamic loader or the main executable's load commands that tell you about it. So at that point, all you need to do is GetSharedModule.
Finding the main executable is harder because there isn't a single authoritative source for how you are going to build the ModuleSpec that you intend to pass to GetSharedModule.. But in the end, we do go through GetSharedModule to actually realize its lldb_private::Module.
So I'm not sure how much sharing you'd get from addressing point (2).

jimingham · 2025-09-22T23:12:22Z

For point 1) lldb's current assumption is if two binaries have the same UUID they are interchangeable. So it's always preferable to go with the one you've already ingested. Any fancier searches should be fallbacks to that.
It's only if you can't determine the UUID for the incoming executable location request that you should use any of the fancier methods.

GeorgeHuyubo · 2025-09-23T03:39:09Z

@jimingham
Actually CallLocateModuleCallbackIfSet also go through ModuleList::GetSharedModule eventually. But it will give the callback function a chance to locate us the stripped out debuginfo symbol file if available

GeorgeHuyubo · 2025-09-23T03:45:07Z

It sucks we have to have two code paths for locating modules, one for shared libraries, one for main executable. I wonder if we should consolidate to a central place? Like ModuleList::GetSharedModule?

Thought about this too, but ModuleList::GetSharedModule is a static method, I wanted to avoid passing in Platform as an extra parameter which might cause more refactoring and focused on making this work for our use case

JDevlieghere

Please follow the LLVM coding style and really just the style used by the surrounding code:

Comments should be English prose, using proper capitalization and punctuation.
No braces for single-line if statements.

clayborg

I would like to see this functionality in ModuleList::GetSharedModule(...), even if we must pass a target in there. We pass down the module_search_paths_ptr which come from the target's settings anyway, so it would be worth seeing all of the callers to this function and see if they all can pass in a target. If we have a target, we can access all target settings without manually passing module_search_paths_ptr in and the target has a platform inside of it. Any solution we come up with should work for both executables and shared libraries in a consistent way.

jimingham · 2025-09-23T21:31:25Z

I would like to see this functionality in ModuleList::GetSharedModule(...), even if we must pass a target in there. We pass down the module_search_paths_ptr which come from the target's settings anyway, so it would be worth seeing all of the callers to this function and see if they all can pass in a target. If we have a target, we can access all target settings without manually passing module_search_paths_ptr in and the target has a platform inside of it. Any solution we come up with should work for both executables and shared libraries in a consistent way.

This seems fine to me as well - I can't imagine anywhere that we're asking for a Module that we don't have a Target on hand. And GetSharedModule does have the job of creating modules it doesn't already have cached, so that seems like a reasonable place to put this.

But that doesn't resolve the ordering question. I still think if you come into GetSharedModule with a ModuleSpec with a UUID in it, we should first look for that UUID amongst the Modules already in the SharedModuleList. Only if that fails should it go into any of the "create" mechanisms. We're okay with switching out the symbol file in a Module we already have, but I don't think we'd handle two Modules with the same UUID but different contents well, so we shouldn't allow that to happen.

Also, though fetching the Executable module once you have a ModuleSpec for it should be a straightforward call to GetSharedModule, I don't think we want to start trying to put all the code that you have to do to figure out what ModuleSpec is meant by the arguments passed to target create into GetSharedModule. That is a very different job than for all the modules reported by the dynamic loader, since that tells us about extant modules.

This reverts commit c6534a1.

GeorgeHuyubo · 2025-09-30T03:35:44Z

@jimingham @jeffreytan81 @clayborg @JDevlieghere Updated with several changes. Ready for another round of review.

lldb/include/lldb/Core/ModuleSpec.h

lldb/include/lldb/Core/ModuleList.h

lldb/source/Core/ModuleList.cpp

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

lldb/source/Target/Platform.cpp

GeorgeHuyubo · 2025-10-03T22:29:32Z

@JDevlieghere Addressed your comments

GeorgeHuyubo · 2025-10-14T22:19:48Z

lldb/include/lldb/Core/ModuleSpec.h

  ArchSpec m_arch;
  UUID m_uuid;
  ConstString m_object_name;
+  std::weak_ptr<Target>


Put comment above with ///

GeorgeHuyubo · 2025-10-14T22:23:06Z

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

-  // See if the file is present in any of the module_search_paths_ptr
+  TargetSP target_sp = module_spec.GetTargetSP();
+  FileSpecList module_search_paths;
+  if (target_sp) {


apply elsewhere

GeorgeHuyubo · 2025-10-14T22:28:24Z

lldb/source/Target/Target.cpp

              arch_spec.GetTriple().getTriple().c_str());
    ModuleSpec module_spec(executable_sp->GetFileSpec(), other);
-    FileSpecList search_paths = GetExecutableSearchPaths();
+    module_spec.SetTarget(this->shared_from_this());


Remove this->

GeorgeHuyubo · 2025-10-14T22:28:36Z

lldb/source/Target/Target.cpp


  // Apply any remappings specified in target.object-map:
  ModuleSpec module_spec(orig_module_spec);
+  module_spec.SetTarget(this->shared_from_this());


GeorgeHuyubo · 2025-10-14T22:28:44Z

lldb/source/Target/Target.cpp

          transformed_spec.GetFileSpec().SetDirectory(transformed_dir);
          transformed_spec.GetFileSpec().SetFilename(
                module_spec.GetFileSpec().GetFilename());
+          transformed_spec.SetTarget(this->shared_from_this());


github-actions · 2025-10-14T22:52:39Z

✅ With the latest revision this PR passed the C/C++ code formatter.

GeorgeHuyubo · 2025-10-15T17:03:09Z

@jimingham @JDevlieghere @clayborg Fixed code style issue, and added test. Can I have another round of review? Thanks

clayborg

A few things to fix mentioned in inline comments. We should add the ability to set the target in SBModuleSpec in the public API. So we need to add:

SBTarget SBModuleSpec::GetTarget();
void SBModuleSpec::SetTarget(SBTarget target);

clayborg · 2025-10-17T21:06:15Z

lldb/include/lldb/Core/ModuleSpec.h

  ConstString m_object_name;
+  /// This is set to take advantage of the target's search path and platform's
+  /// locate module callback
+  std::weak_ptr<Target> m_target;


change the name to m_target_wp to indicate a weak pointer. We name shared pointers with a _sp suffix and weak pointers with a _wp

clayborg · 2025-10-17T21:08:12Z

lldb/source/Core/ModuleList.cpp

+  // Get module search paths from the target if available.
+  lldb::TargetSP target_sp = module_spec.GetTargetSP();
+  FileSpecList module_search_paths;
+  FileSpecList *module_search_paths_ptr = nullptr;


We don't need this pointer, just use module_search_paths everywhere in this function.

clayborg · 2025-10-17T21:08:43Z

lldb/source/Core/ModuleList.cpp

+  lldb::TargetSP target_sp = module_spec.GetTargetSP();
+  FileSpecList module_search_paths;
+  FileSpecList *module_search_paths_ptr = nullptr;
+  if (target_sp && target_sp->IsValid()) {


No need to check target_sp->IsValid() here

GeorgeHuyubo requested review from clayborg and jeffreytan81 September 22, 2025 20:58

GeorgeHuyubo force-pushed the main branch 2 times, most recently from 90823c1 to aed6a69 Compare September 22, 2025 21:17

llvm deleted a comment from github-actions bot Sep 22, 2025

Call locate module callback for main executable

c6534a1

GeorgeHuyubo force-pushed the main branch from aed6a69 to c6534a1 Compare September 22, 2025 21:27

llvm deleted a comment from github-actions bot Sep 22, 2025

GeorgeHuyubo marked this pull request as ready for review September 22, 2025 21:54

GeorgeHuyubo requested a review from JDevlieghere as a code owner September 22, 2025 21:54

llvmbot added the lldb label Sep 22, 2025

JDevlieghere requested changes Sep 23, 2025

View reviewed changes

clayborg requested changes Sep 23, 2025

View reviewed changes

GeorgeHuyubo force-pushed the main branch from b6bf128 to 64e1fa8 Compare September 27, 2025 00:29

llvm deleted a comment from github-actions bot Sep 27, 2025

GeorgeHuyubo force-pushed the main branch 3 times, most recently from 65fe283 to dffcc73 Compare September 29, 2025 20:30

llvm deleted a comment from github-actions bot Sep 29, 2025

George Hu added 2 commits September 29, 2025 14:05

[lldb] Refactor add target into ModuleSpec

2dc6595

Revert "Call locate module callback for main executable"

1c8dcac

This reverts commit c6534a1.

GeorgeHuyubo force-pushed the main branch 2 times, most recently from 685bb1f to 9c69309 Compare September 30, 2025 03:11

[lldb] Enable locate module callback in GetSharedModule

cfe7d17

GeorgeHuyubo force-pushed the main branch from 9c69309 to cfe7d17 Compare September 30, 2025 03:17

llvm deleted a comment from github-actions bot Sep 30, 2025

GeorgeHuyubo requested review from JDevlieghere and clayborg September 30, 2025 03:34

JDevlieghere requested changes Oct 1, 2025

View reviewed changes

George Hu and others added 2 commits October 2, 2025 16:54

Address comment

23f1b98

Merge branch 'main' into main

d8bba3a

GeorgeHuyubo requested a review from JDevlieghere October 3, 2025 17:03

Merge branch 'main' into main

dff74fa

GeorgeHuyubo requested a review from jimingham October 10, 2025 17:11

GeorgeHuyubo commented Oct 14, 2025

View reviewed changes

Code style fix

f8a3cbb

GeorgeHuyubo force-pushed the main branch from 3966e6d to f8a3cbb Compare October 14, 2025 23:25

Add test

8d6ef4d

clayborg requested changes Oct 17, 2025

View reviewed changes

[lldb] Enable locate module callback for main executable #160199

Are you sure you want to change the base?

[lldb] Enable locate module callback for main executable #160199

Uh oh!

Conversation

GeorgeHuyubo commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 22, 2025

Uh oh!

jeffreytan81 commented Sep 22, 2025

Uh oh!

jimingham commented Sep 22, 2025

Uh oh!

jimingham commented Sep 22, 2025

Uh oh!

GeorgeHuyubo commented Sep 23, 2025

Uh oh!

GeorgeHuyubo commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

clayborg left a comment

Choose a reason for hiding this comment

Uh oh!

jimingham commented Sep 23, 2025

Uh oh!

GeorgeHuyubo commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GeorgeHuyubo commented Oct 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GeorgeHuyubo commented Oct 15, 2025

Uh oh!

clayborg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GeorgeHuyubo commented Sep 22, 2025 •

edited

Loading

GeorgeHuyubo commented Sep 23, 2025 •

edited

Loading

GeorgeHuyubo commented Sep 30, 2025 •

edited

Loading

github-actions bot commented Oct 14, 2025 •

edited

Loading