[lldb] fix step in AArch64 trampoline

[v-bulle]

Detects AArch64 trampolines in order to be able to step in a function through a trampoline on AArch64.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Vincent Belliard (v-bulle)

Changes

Detects AArch64 trampolines in order to be able to step in a function through a trampoline on AArch64.

---

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

4 Files Affected:

• (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+23-3)
• (modified) lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (+18-1)
• (added) lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc (+15)
• (added) lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test (+17)

diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..232030268e42c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,9 +506,29 @@ DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread &thread,
   Target &target = thread.GetProcess()->GetTarget();
   const ModuleList &images = target.GetImages();
 
-  images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
-  if (!target_symbols.GetSize())
-    return thread_plan_sp;
+  llvm::StringRef target_name = sym_name.GetStringRef();
+  // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+  // __AArch64AbsLongThunk_) added to the function name. If we detect a
+  // trampoline with the prefix, we need to remove the prefix to find the
+  // function symbol.
+  if (target_name.consume_front("__AArch64ADRPThunk_")) {
+    // An empty target name can happen when for trampolines generated for
+    // section-referencing relocations.
+    if (!target_name.empty()) {
+      images.FindSymbolsWithNameAndType(ConstString(target_name),
+                                        eSymbolTypeCode, target_symbols);
+    }
+  } else if (target_name.consume_front("__AArch64AbsLongThunk_")) {
+    // An empty target name can happen when for trampolines generated for
+    // section-referencing relocations.
+    if (!target_name.empty()) {
+      images.FindSymbolsWithNameAndType(ConstString(target_name),
+                                        eSymbolTypeCode, target_symbols);
+    }
+  } else {
+    images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode,
+                                      target_symbols);
+  }
 
   typedef std::vector<lldb::addr_t> AddressVector;
   AddressVector addrs;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
     bool symbol_size_valid =
         symbol.st_size != 0 || symbol.getType() != STT_FUNC;
 
+    bool is_trampoline = false;
+    if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+      // On AArch64, trampolines are registered as code.
+      // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+      // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+      // way we will be able to detect the trampoline when we step in a function
+      // and step through the trampoline.
+      if (symbol_type == eSymbolTypeCode) {
+        llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+        if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+            trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+          symbol_type = eSymbolTypeTrampoline;
+          is_trampoline = true;
+        }
+      }
+    }
+
     Symbol dc_symbol(
         i + start_id, // ID is the original symbol table index.
         mangled,
         symbol_type,                    // Type of this symbol
         is_global,                      // Is this globally visible?
         false,                          // Is this symbol debug info?
-        false,                          // Is this symbol a trampoline?
+        is_trampoline,                  // Is this symbol a trampoline?
         false,                          // Is this symbol artificial?
         AddressRange(symbol_section_sp, // Section in which this symbol is
                                         // defined or null.
diff --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00000000000000..02f3bef32a59a3
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
@@ -0,0 +1,15 @@
+extern "C" int __attribute__((naked)) __AArch64ADRPThunk_step_here() {
+    asm (
+      "adrp x16, step_here\n"
+      "add x16, x16, :lo12:step_here\n"
+      "br x16"
+    );
+}
+
+extern "C" __attribute__((used)) int step_here() {
+    return 47;
+}
+
+int main() {
+  return __AArch64ADRPThunk_step_here();
+}
diff --git a/lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test b/lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test
new file mode 100644
index 00000000000000..336a746fa3a418
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test
@@ -0,0 +1,17 @@
+# REQUIRES: native && target-aarch64
+
+# This test is specific to elf platforms.
+# UNSUPPORTED: system-windows, system-darwin
+
+# RUN: %clangxx_host %p/Inputs/aarch64_thunk.cc -g -o %t.out
+# RUN: %lldb %t.out -s %s | FileCheck %s
+
+b main
+# CHECK: Breakpoint 1: where = step_through-aarch64-thunk.test.tmp.out`main
+
+r
+# CHECK: stop reason = breakpoint 1.1
+
+s
+# CHECK: stop reason = step in
+# CHECK:     frame #0: {{.*}} step_through-aarch64-thunk.test.tmp.out`::step_here()

[DavidSpickett]

My first instinct with this sort of if arch is bla code is to put it into a plugin, usually the ABI. However this being ELF specific, and the ELF plugin not having access to a target to get the ABI from, that's not possible. The DYLD plugin could but then we still have 2 copies of the symbol prefixes. So same result really.

The ELF plugin function is already 300 lines of architecture details so this is not unique to trampolines at all.

If someone wanted to add trampolines for MachO, they would need to add them to the MachO plugin and the list in DYLD so the DYLD list is going to end up a superset of them anyway. So there will always be one larger list, with subsets of it in the different object type plugins.

The fact that DYLD might look for suffixes for ELF in a MachO file should not be a problem (at least for correctness) because a symbol with an ELF thunk suffix name would not be marked as a thunk symbol by the MachO object plugin in the first place.

So no need to change the structure of this PR, but do correct me if any of my assumptions are incorrect there.

[labath]

So no need to change the structure of this PR, but do correct me if any of my assumptions are incorrect there.

The DynamicLoaderPOSIXDYLD is only used on ELF systems anyway (we might as well rename it do DynamicLoaderELF), so this actually lines up fairly well.