Skip to content

Conversation

@ostannard
Copy link
Collaborator

The R_ARM_SBREL32 relocation is used in debug info for ARM RWPI (read-write position independent) code. Compiler-generated DWARF info will use an expression to add the relocated value to the actual value of the static base (held in r9) at run-time, so it should be relocated as if the static base is at address 0.

The R_ARM_SBREL32 relocation is used in debug info for ARM RWPI
(read-write position independent) code. Compiler-generated DWARF info
will use an expression to add the relocated value to the actual value of
the static base (held in r9) at run-time, so it should be relocated as
if the static base is at address 0.
@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Oliver Stannard (ostannard)

Changes

The R_ARM_SBREL32 relocation is used in debug info for ARM RWPI (read-write position independent) code. Compiler-generated DWARF info will use an expression to add the relocated value to the actual value of the static base (held in r9) at run-time, so it should be relocated as if the static base is at address 0.


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

2 Files Affected:

  • (modified) lld/ELF/InputSection.cpp (+1-1)
  • (added) lld/test/ELF/arm-rwpi-debug-relocs.s (+57)
diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 1221f56dfe68a6..595545e194f815 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -1092,7 +1092,7 @@ void InputSection::relocateNonAlloc(Ctx &ctx, uint8_t *buf,
     // R_ABS/R_DTPREL and some other relocations can be used from non-SHF_ALLOC
     // sections.
     if (LLVM_LIKELY(expr == R_ABS) || expr == R_DTPREL || expr == R_GOTPLTREL ||
-        expr == R_RISCV_ADD) {
+        expr == R_RISCV_ADD || expr == R_ARM_SBREL) {
       target.relocateNoSym(bufLoc, type,
                            SignExtend64<bits>(sym.getVA(ctx, addend)));
       continue;
diff --git a/lld/test/ELF/arm-rwpi-debug-relocs.s b/lld/test/ELF/arm-rwpi-debug-relocs.s
new file mode 100644
index 00000000000000..47c2e0739377b9
--- /dev/null
+++ b/lld/test/ELF/arm-rwpi-debug-relocs.s
@@ -0,0 +1,57 @@
+// REQUIRES: arm
+// RUN: rm -rf %t && split-file %s %t && cd %t
+
+// RUN: llvm-mc -filetype=obj -triple=armv7a asm.s -o obj.o
+// RUN: ld.lld -T lds.ld obj.o -o exe.elf -e main 2>&1 | FileCheck %s --implicit-check-not=warning: --allow-empty
+// RUN: llvm-objdump -D exe.elf | FileCheck --check-prefix=DISASM %s
+
+// DISASM:      Disassembly of section data1:
+// DISASM:      00001000 <rw>:
+// DISASM-NEXT:     1000: 0000002a
+
+// DISASM:      Disassembly of section data2:
+// DISASM:      00002000 <rw2>:
+// DISASM-NEXT:     2000: 000004d2
+
+// DISASM:      Disassembly of section .debug_something:
+// DISASM:      00000000 <.debug_something>:
+// DISASM-NEXT:        0: 00001000
+// DISASM-NEXT:      ...
+// DISASM-NEXT:      104: 00002000
+
+// Test that R_ARM_SBREL32 relocations in debug info are relocated as if the
+// static base register (r9) is zero. Real DWARF info will use an expression to
+// add this to the real value of the static base at runtime.
+
+//--- lds.ld
+SECTIONS {
+  data1 0x1000 : { *(data1) }
+  data2 0x2000 : { *(data2) }
+}
+
+//--- asm.s
+  .text
+	.type	main,%function
+	.globl	main
+main:
+  bx lr
+  .size main, .-main
+
+	.section data1, "aw", %progbits
+	.type	rw,%object
+	.globl	rw
+rw:
+	.long	42
+	.size	rw, 4
+
+	.section data2, "aw", %progbits
+	.type	rw2,%object
+	.globl	rw2
+rw2:
+	.long	1234
+	.size	rw2, 4
+
+	.section	.debug_something, "", %progbits
+	.long	rw(sbrel)
+  .space 0x100
+	.long	rw2(sbrel)

Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I can confirm that this is how debug is supposed to be represented when RWPI is used. Please give MaskRay some time to comment.

I've left some small comments on lld conventions for test format.


//--- asm.s
.text
.type _start,%function
Copy link
Member

@MaskRay MaskRay Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file mixes tabs and spaces for indentation. Perhaps just use 2 spaces for consistency

// DISASM-NEXT: ...
// DISASM-NEXT: 104: 00002000

/// Test that R_ARM_SBREL32 relocations in debug info are relocated as if the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved to the top as a file comment

// sections.
if (LLVM_LIKELY(expr == R_ABS) || expr == R_DTPREL || expr == R_GOTPLTREL ||
expr == R_RISCV_ADD) {
expr == R_RISCV_ADD || expr == R_ARM_SBREL) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(In the future, we should move relocateNonAlloc to be target-specific into Arch/ )

Tabs->spaces
Move comment to top of file
@ostannard ostannard merged commit 6512e48 into llvm:main Nov 25, 2024
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 25, 2024

LLVM Buildbot has detected a new failure on builder llvm-x86_64-debian-dylib running on gribozavr4 while building lld at step 8 "test-build-unified-tree-check-lld".

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

Here is the relevant piece of the build log for the reference
Step 8 (test-build-unified-tree-check-lld) failure: 1200 seconds without output running [b'ninja', b'check-lld'], attempting to kill
...
PASS: lld :: wasm/why-extract.s (3004 of 3014)
PASS: lld :: MachO/stabs.s (3005 of 3014)
PASS: lld :: wasm/lto/thinlto.ll (3006 of 3014)
PASS: lld :: MachO/arm64-thunks.s (3007 of 3014)
PASS: lld :: wasm/lto/thinlto-index-only.ll (3008 of 3014)
PASS: lld :: wasm/libsearch.s (3009 of 3014)
PASS: lld :: MachO/order-file.s (3010 of 3014)
PASS: lld :: MachO/weak-definition-direct-fetch.s (3011 of 3014)
PASS: lld :: ELF/relocatable-many-sections.s (3012 of 3014)
PASS: lld :: MinGW/driver.test (3013 of 3014)
command timed out: 1200 seconds without output running [b'ninja', b'check-lld'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1212.975200

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.

5 participants