[lldb] Improve ELF trampoline function detection

[da-viper]

The mold linker adds a function symbol to the symbol table (.symtab) for every shared library exported function.
So if we try to step into a library, lldb will fail because the symbol at the address is not a trampline function.

Example:

// lib.c
int lib_add(int a, int b);

when we link the library to an executable, mold will create the normal trampoline functions in the .plt section but add an extra symbol in symbol table lib_add$plt.

I cannot think of a way to add another symbol in the trampoline's file address without linking with mold.
Is there a way to write a test for this ?

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

Changes

The mold linker adds a function symbol to the symbol table (.symtab) for every shared library exported function.
So if we try to step into a library, lldb will fail because the symbol at the address is not a trampline function.

Example:

// lib.c
int lib_add(int a, int b);

when we link the library to an executable, mold will create the normal trampoline functions in the .plt section but add an extra symbol in symbol table lib_add$plt.

I cannot think of a way to add another symbol in the trampoline's file address without linking with mold.
Is there a way to write a test for this ?

---

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

3 Files Affected:

• (modified) lldb/include/lldb/Core/Module.h (+4)
• (modified) lldb/source/Core/Module.cpp (+17)
• (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+25-2)

diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 643b9a5c3bf54..87d828fb41eed 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -265,6 +265,10 @@ class Module : public std::enable_shared_from_this<Module>,
                                   lldb::SymbolType symbol_type,
                                   SymbolContextList &sc_list);
 
+  void FindSymbolsContainingFileAddress(const Address &addr,
+                                        lldb::SymbolType symbol_type,
+                                        SymbolContextList &sc_list);
+
   void FindSymbolsMatchingRegExAndType(
       const RegularExpression &regex, lldb::SymbolType symbol_type,
       SymbolContextList &sc_list,
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 486af1a053344..436c4ce027d84 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1316,6 +1316,23 @@ void Module::FindSymbolsWithNameAndType(ConstString name,
   }
 }
 
+void Module::FindSymbolsContainingFileAddress(const Address &addr,
+                                              lldb::SymbolType symbol_type,
+                                              SymbolContextList &sc_list) {
+  if (Symtab *symtab = GetSymtab()) {
+    std::vector<uint32_t> symbol_indexes;
+    symtab->ForEachSymbolContainingFileAddress(
+        addr.GetFileAddress(), [&, symbol_type](Symbol *match_sym) {
+          if (const lldb::SymbolType curr_type = match_sym->GetType();
+              curr_type == lldb::eSymbolTypeAny || curr_type == symbol_type) {
+            symbol_indexes.push_back(symtab->GetIndexForSymbol(match_sym));
+          }
+          return true;
+        });
+    SymbolIndicesToSymbolContextList(symtab, symbol_indexes, sc_list);
+  }
+}
+
 void Module::FindSymbolsMatchingRegExAndType(
     const RegularExpression &regex, SymbolType symbol_type,
     SymbolContextList &sc_list, Mangled::NamePreference mangling_preference) {
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 6705ac139f0fb..8802b4b7cd27b 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -540,11 +540,34 @@ DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread &thread,
 
   StackFrame *frame = thread.GetStackFrameAtIndex(0).get();
   const SymbolContext &context = frame->GetSymbolContext(eSymbolContextSymbol);
-  const Symbol *sym = context.symbol;
 
-  if (sym == nullptr || !sym->IsTrampoline())
+  Symbol *sym = context.symbol;
+  if (sym == nullptr)
     return thread_plan_sp;
 
+  if (!sym->IsTrampoline()) {
+    if (sym->GetType() != lldb::eSymbolTypeCode)
+      return thread_plan_sp;
+
+    SymbolContextList addr_ctx_list;
+    context.module_sp->FindSymbolsContainingFileAddress(
+        context.GetFunctionOrSymbolAddress(), eSymbolTypeTrampoline,
+        addr_ctx_list);
+    if (addr_ctx_list.IsEmpty())
+      return thread_plan_sp;
+
+    for (const auto &sym_ctx : addr_ctx_list.SymbolContexts()) {
+      if (sym_ctx.symbol != nullptr) {
+        sym = sym_ctx.symbol;
+        break;
+      }
+    }
+
+    // may not find a match
+    if (sym == nullptr)
+      return thread_plan_sp;
+  }
+
   ConstString sym_name = sym->GetMangled().GetName(Mangled::ePreferMangled);
   if (!sym_name)
     return thread_plan_sp;

[DavidSpickett]

Invert the condition and early return instead.

[DavidSpickett]

The mold linker adds a function symbol to the symbol table (.symtab) for every shared library exported function.

This sounds reasonable but is it in fact correct to do so? Doesn't sound accidental so I guess it is allowable.

I cannot think of a way to add another symbol in the trampoline's file address without linking with mold.
Is there a way to write a test for this ?

Poorly thought out ideas:

• Load the symbol from a json file https://lldb.llvm.org/use/symbolfilejson.html, this was done for a non-address bit test at some point (so that it could run on any hardware). Though this is adding symbols into lldb's own structures, not the library.
• obj2yaml the smallest library you can get it down to, then link it with a main program just enough to be able to load into lldb. Assuming the YAML can capture that detail.
• Terrible extension of that - compile, obj2yaml, patch the yaml, yaml2obj, link normally
• Use llvm-objcopy (https://llvm.org/docs/CommandGuide/llvm-objcopy.html#cmdoption-llvm-objcopy-add-symbol). No idea if that'll add the type you want though.

[da-viper]

This sounds reasonable but is it in fact correct to do so? Doesn't sound accidental so I guess it is allowable.
not sure but it was added here

rui314/mold@6ea03e6

[jimingham]

You are in effect adding the rule: "If ANY symbol at the address where we've stopped is a Trampoline symbol, we'll follow the trampoline symbol to its target." The old rule being "If the FIRST symbol we come across is a Trampoline symbol..."

That seems like a fine rule, but it seems sufficiently general that we should be implementing it above the layer of a particular dynamic loader plugin.

OTOH, if this is really a weird thing to do and only mold on ELF will ever do that, I guess you could argue that we shouldn't penalize all the other systems with an extra search that will only ever bear fruit for the ELF dynamic loader plugin.

I'm on the fence about that.

Unfortunately, since this is execution control, you're going to have to actually try to step across one of these to make sure it works, so you really will have to have a process you can run.

As for the test, what you are trying to do at base is to produce a plt entry that has another symbol at the same address. In theory, you don't need mold for that if you can cook up some fancy linker script or asm trickery to make that happen.

[bulbazord]

Can you keep the const? :)