[lldb] On POSIX, check for duplicate interpreter modules without loading them

[DavidSpickett]

Fixes #68987

Early on we load the interpreter (most commonly ld-linux) in LoadInterpreterModule. Then later when we get the first DYLD rendezvous we get a list of libraries that commonly includes ld-linux again.

Previously we would load this duplicate, see that it was a duplicate, and unload it.

Problem was that this unloaded the section information of the first copy of ld-linux. On platforms where you can place a breakpoint using only an address, this wasn't an issue.

On ARM you have ARM and Thumb modes. We must know which one the section we're breaking in is, otherwise we'll go there in the wrong mode and SIGILL. This happened on ARM when lldb tried to call mmap during expression evaluation.

To fix this, I am making the assumption that the base address we see in the module prior to loading can be compared with what we know the interpreter base address is. Then we don't have to load the module to know we can ignore it.

This fixes the lldb test suite on Ubuntu versions where https://bugs.launchpad.net/ubuntu/+source/gdb/+bug/1927192 has been fixed. Which was recently done on Jammy.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

Changes

Fixes #68987

Early on we load the interpreter (most commonly ld-linux) in LoadInterpreterModule. Then later when we get the first DYLD rendezvous we get a list of libraries that commonly includes ld-linux again.

Previously we would load this duplicate, see that it was a duplicate, and unload it.

Problem was that this unloaded the section information of the first copy of ld-linux. On platforms where you can place a breakpoint using only an address, this wasn't an issue.

On ARM you have ARM and Thumb modes. We must know which one the section we're breaking in is, otherwise we'll go there in the wrong mode and SIGILL. This happened on ARM when lldb tried to call mmap during expression evaluation.

To fix this, I am making the assumption that the base address we see in the module prior to loading can be compared with what we know the interpreter base address is. Then we don't have to load the module to know we can ignore it.

This fixes the lldb test suite on Ubuntu versions where https://bugs.launchpad.net/ubuntu/+source/gdb/+bug/1927192 has been fixed. Which was recently done on Jammy.

---

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

1 Files Affected:

• (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+8-9)

diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index c427b476089e458..3d65f496742099d 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -437,6 +437,14 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
       m_initial_modules_added = true;
     }
     for (; I != E; ++I) {
+      // Don't load a duplicate copy of ld.so if we have already loaded it
+      // earlier in LoadInterpreterModule. If we instead loaded then unloaded it
+      // later, the section information for ld.so would be removed. That
+      // information is required for placing breakpoints on Arm/Thumb systems.
+      if ((m_interpreter_module.lock() != nullptr) &&
+          (I->base_addr == m_interpreter_base))
+        continue;
+
       ModuleSP module_sp =
           LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
       if (!module_sp.get())
@@ -450,15 +458,6 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
         } else if (module_sp == interpreter_sp) {
           // Module already loaded.
           continue;
-        } else {
-          // If this is a duplicate instance of ld.so, unload it.  We may end
-          // up with it if we load it via a different path than before
-          // (symlink vs real path).
-          // TODO: remove this once we either fix library matching or avoid
-          // loading the interpreter when setting the rendezvous breakpoint.
-          UnloadSections(module_sp);
-          loaded_modules.Remove(module_sp);
-          continue;
         }
       }
 

[DavidSpickett]

Following the code down into ObjectFileELF::SetLoadAddress for ELF, I don't see the value we get from I->base_addr being any different from the one we get later as module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress. We set value_is_offset to true, if it were false, then it could change and this assumption would not hold.

I've tested this on a few versions of Ubuntu across ARM, AArch64 and AMD64.

As this is the POSIX DYLD not just Linux, @emaste in case there's any FreeBSD details I need to account for.

[clayborg]

LGTM. I have seen multiple VSDO libraries being loaded sometimes too, but it doesn't use this path in the code, so this fix should work for ld.so

[DavidSpickett]

I'd really like to get Arm checks going again, so I'm going to land this today and see how the bot does. Of course I'll revert if there's any sign of instability on the BSD side.