Skip to content

Conversation

@smithp35
Copy link
Collaborator

Include the offset of a thunk in the ThunkSection when adding symbols.

At Thunk creation time the offset is set to 0 as we don't know where in the ThunkSection the Thunk will end up. The symbol values are updated by the setOffset() call in assignOffsets().

When we transform a thunk from a short to a long, we sometimes add a mapping symbol. At this point the offset of the thunk is non zero and we need to account for that when defining the symbol, as the setOffset() call subtracts the offset before adding the new one back in.

To test; added a second thunk that is converted to a long thunk to aarch64-thunk-bit-multipass. This second thunk is given a non zero offset from the start of the Thunk Section so we can observe the mapping symbol being put in the wrong place without accounting for the offset.

fixes: #142326

Include the offset of a thunk in the ThunkSection when adding
symbols.

At Thunk creation time the offset is set to 0 as we don't know
where in the ThunkSection the Thunk will end up. The symbol
values are updated by the setOffset() call in assignOffsets().

When we transform a thunk from a short to a long, we sometimes
add a mapping symbol. At this point the offset of the thunk is
non zero and we need to account for that when defining the
symbol, as the setOffset() call subtracts the offset before
adding the new one back in.

To test; added a second thunk that is converted to a long thunk
to aarch64-thunk-bit-multipass. This second thunk is given a non
zero offset from the start of the Thunk Section so we can observe
the mapping symbol being put in the wrong place without accounting
for the offset.

fixes: llvm#142326
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Peter Smith (smithp35)

Changes

Include the offset of a thunk in the ThunkSection when adding symbols.

At Thunk creation time the offset is set to 0 as we don't know where in the ThunkSection the Thunk will end up. The symbol values are updated by the setOffset() call in assignOffsets().

When we transform a thunk from a short to a long, we sometimes add a mapping symbol. At this point the offset of the thunk is non zero and we need to account for that when defining the symbol, as the setOffset() call subtracts the offset before adding the new one back in.

To test; added a second thunk that is converted to a long thunk to aarch64-thunk-bit-multipass. This second thunk is given a non zero offset from the start of the Thunk Section so we can observe the mapping symbol being put in the wrong place without accounting for the offset.

fixes: #142326


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

2 Files Affected:

  • (modified) lld/ELF/Thunks.cpp (+2-1)
  • (modified) lld/test/ELF/aarch64-thunk-bti-multipass.s (+21-1)
diff --git a/lld/ELF/Thunks.cpp b/lld/ELF/Thunks.cpp
index bad1b4b85735a..c26ba76bccb7e 100644
--- a/lld/ELF/Thunks.cpp
+++ b/lld/ELF/Thunks.cpp
@@ -601,7 +601,8 @@ class PPC64PDLongBranchThunk final : public PPC64LongBranchThunk {
 
 Defined *Thunk::addSymbol(StringRef name, uint8_t type, uint64_t value,
                           InputSectionBase &section) {
-  Defined *d = addSyntheticLocal(ctx, name, type, value, /*size=*/0, section);
+  Defined *d =
+      addSyntheticLocal(ctx, name, type, value + offset, /*size=*/0, section);
   syms.push_back(d);
   return d;
 }
diff --git a/lld/test/ELF/aarch64-thunk-bti-multipass.s b/lld/test/ELF/aarch64-thunk-bti-multipass.s
index f2ff914fb850d..f470d832843b8 100644
--- a/lld/test/ELF/aarch64-thunk-bti-multipass.s
+++ b/lld/test/ELF/aarch64-thunk-bti-multipass.s
@@ -29,6 +29,7 @@
 _start:
 /// Call that requires a thunk.
  bl fn1
+ bl fn2
 /// padding so that the thunk for fn1 is placed after this section is
 /// sufficiently close to the target to be within short range, but only
 /// just so that a small displacement will mean a long thunk is needed.
@@ -39,6 +40,7 @@ _start:
 
 // CHECK-LABEL: <_start>:
 // CHECK-NEXT: 10001000: bl  0x10002008 <__AArch64AbsLongThunk_fn1>
+// CHECK-NEXT:           bl  0x10002018 <__AArch64AbsLongThunk_fn2>
 
 // CHECK-LABEL: <__AArch64AbsLongThunk_fn1>:
 // CHECK-NEXT: 10002008: ldr     x16, 0x10002010 <__AArch64AbsLongThunk_fn1+0x8>
@@ -46,6 +48,12 @@ _start:
 // CHECK-NEXT:           00 30 00 18    .word   0x18003000
 // CHECK-NEXT:           00 00 00 00    .word   0x00000000
 
+// CHECK-LABEL: <__AArch64AbsLongThunk_fn2>:
+// CHECK-NEXT: 10002018: ldr     x16, 0x10002020 <__AArch64AbsLongThunk_fn2+0x8>
+// CHECK-NEXT:           br      x16
+// CHECK-NEXT:           04 40 00 18    .word   0x18004004
+// CHECK-NEXT:           00 00 00 00    .word   0x00000000
+
 .section .text.1, "ax", %progbits
 .balign 0x1000
 .global farcall
@@ -75,6 +83,12 @@ farcall:
 fn1:
  ret
 
+.section .text.3, "ax", %progbits
+.global fn2
+.type fn2, %function
+fn2:
+ ret
+
 .section .text.far, "ax", %progbits
 .type far, %function
 .global far
@@ -88,6 +102,12 @@ far:
 // CHECK-LABEL: <fn1>:
 // CHECK-NEXT: 18004000: ret
 
+// CHECK-LABEL: <__AArch64BTIThunk_fn2>:
+// CHECK-NEXT: 18004004:       bti     c
+
+// CHECK-LABEL: <fn2>:
+// CHECK-NEXT: 18004008:       ret
+
 // CHECK-LABEL: <__AArch64BTIThunk_far>:
 // CHECK-NEXT: 30000000: bti     c
 
@@ -104,6 +124,6 @@ SECTIONS {
   .rodata 0x10000000 : { *(.note.gnu.property) } :low
   .text_low : { *(.text.0) } :low
   .text 0x18001000 : { *(.text.1) } :mid
-  .text_aligned : { *(.text.2) } :mid
+  .text_aligned : { *(.text.2) *(.text.3) } :mid
   .text_high 0x30000000 : { *(.text.far) } :high
 }

@smithp35 smithp35 merged commit eb0f1dc into llvm:main Jun 20, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[LLD][AArch64] Occasionally LLD drops data marker for long abs thunks

3 participants