[lldb] Enable locate module callback for all module loading

[GeorgeHuyubo]

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]

@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]

This works functionality wise but I have two concerns:

1. 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?
2. 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]

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]

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]

@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]

2. 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]

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.

[github-actions]

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

[GeorgeHuyubo]

@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); 

[llvm-ci]

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu running on as-builder-9 while building lldb at step 16 "test-check-lldb-api".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/17037

Here is the relevant piece of the build log for the reference

Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure) ******************** TEST 'lldb-api :: functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py' FAILED ******************** Script: -- /usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --cmake-build-type Release --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild -p TestLocationsAfterRebuild.py -- Exit Code: 1 Command Output (stdout): -- lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision fce58897ce82de84c8d794609132eb547b2b4871) clang revision fce58897ce82de84c8d794609132eb547b2b4871 llvm revision fce58897ce82de84c8d794609132eb547b2b4871 Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-2198.lab.llvm.org:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['lldb-server', 'libc++', 'msvcstl', 'dsym', 'pdb', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- WARNING:root:Custom libc++ is not supported for remote runs: ignoring --libcxx arguments FAIL: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_remaining_location_spec (TestLocationsAfterRebuild.TestLocationsAfterRebuild.test_remaining_location_spec) ====================================================================== FAIL: test_remaining_location_spec (TestLocationsAfterRebuild.TestLocationsAfterRebuild.test_remaining_location_spec) If we rebuild a couple of times some of the old locations ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py", line 59, in test_remaining_location_spec self.runCmd(f"break disable {loc_string}") File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1006, in runCmd self.assertTrue(self.res.Succeeded(), msg + output) AssertionError: False is not true : Command 'break disable 1.3' did not return successfully Error output: error: '1.3' is not a currently valid breakpoint/location id. Config=aarch64-/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang ---------------------------------------------------------------------- Ran 1 test in 1.387s FAILED (failures=1) -- ******************** 

[llvm-ci]

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-win running on as-builder-10 while building lldb at step 17 "test-check-lldb-api".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/10629

Here is the relevant piece of the build log for the reference

Step 17 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure) ******************** TEST 'lldb-api :: functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py' FAILED ******************** Script: -- C:/Python312/python.exe C:/buildbot/as-builder-10/lldb-x-aarch64/llvm-project/lldb\test\API\dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/./lib --env LLVM_INCLUDE_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/include --env LLVM_TOOLS_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin --arch aarch64 --build-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex --lldb-module-cache-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex/module-cache-lldb\lldb-api --clang-module-cache-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex/module-cache-clang\lldb-api --executable C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/lldb.exe --compiler C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/clang.exe --dsymutil C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/dsymutil.exe --make C:/ninja/make.exe --llvm-tools-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin --lldb-obj-root C:/buildbot/as-builder-10/lldb-x-aarch64/build/tools/lldb --lldb-libs-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/./lib --cmake-build-type Release --platform-url connect://jetson-agx-0086.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot c:/buildbot/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\API\functionalities\breakpoint\breakpoint_locations\after_rebuild -p TestLocationsAfterRebuild.py -- Exit Code: 1 Command Output (stdout): -- lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision fce58897ce82de84c8d794609132eb547b2b4871) clang revision fce58897ce82de84c8d794609132eb547b2b4871 llvm revision fce58897ce82de84c8d794609132eb547b2b4871 Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-0086.lab.llvm.org:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['lldb-server', 'libc++', 'msvcstl', 'dsym', 'pdb', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- FAIL: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_remaining_location_spec (TestLocationsAfterRebuild.TestLocationsAfterRebuild.test_remaining_location_spec) ====================================================================== FAIL: test_remaining_location_spec (TestLocationsAfterRebuild.TestLocationsAfterRebuild.test_remaining_location_spec) If we rebuild a couple of times some of the old locations ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\API\functionalities\breakpoint\breakpoint_locations\after_rebuild\TestLocationsAfterRebuild.py", line 59, in test_remaining_location_spec self.runCmd(f"break disable {loc_string}") File "C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\packages\Python\lldbsuite\test\lldbtest.py", line 1006, in runCmd self.assertTrue(self.res.Succeeded(), msg + output) AssertionError: False is not true : Command 'break disable 1.3' did not return successfully Error output: ... 

[slydiman]

2 buildbots are broken (see above). Please fix the test ASAP

Traceback (most recent call last): File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py", line 59, in test_remaining_location_spec self.runCmd(f"break disable {loc_string}") File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1006, in runCmd self.assertTrue(self.res.Succeeded(), msg + output) AssertionError: False is not true : Command 'break disable 1.3' did not return successfully Error output: error: '1.3' is not a currently valid breakpoint/location id. 

[slydiman]

@GeorgeHuyubo
This patch broke the test TestLocationsAfterRebuild.py for remote Linux.
#167239 may make buildbots green for now. But you must fix it for remote Linux ASAP.