[PATCH] D75042: [LLD][ELF][ARM] Implement Thumb pc-relative relocations for adr and ldr

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/ARM.cpp:596
+    // bottom bit to recover S + A - Pa.
+    if (rel.sym && rel.sym->isFunc())
+      val &= ~0x1;
----------------
I believe rel.sym is never nullptr, even if ELF32_R_SYM(r) is 0.

The field Relocation::sym is a pointer to:

    Symbol &sym = file->getRelocTargetSym(rel);



================
Comment at: lld/ELF/Arch/ARM.cpp:608
+    // bottom bit to recover S + A - Pa.
+    if (rel.sym && rel.sym->isFunc())
+      val &= ~0x1;
----------------
ditto


================
Comment at: lld/test/ELF/arm-thumb-adr-err.s:2
+// RUN: llvm-mc -g --triple=thumbv6m-none-eabi --arm-add-build-attributes -filetype=obj -o %t.o %s
+// RUN: not ld.lld -n %t.o -o /dev/null 2>&1 | FileCheck %s
+
----------------
Is --nmagic (-n) needed?


================
Comment at: lld/test/ELF/arm-thumb2-adr-err.s:2
+// RUN: llvm-mc -g --triple=thumbv7m-none-eabi --arm-add-build-attributes -filetype=obj -o %t.o %s
+// RUN: not ld.lld -n %t.o -o %t 2>&1 | FileCheck %s
+
----------------
`%t` -> `/dev/null`

Delete `-n`


================
Comment at: lld/test/ELF/arm-thumb2-ldrlit.s:10
+// RUN:               } " > %t.script
+// RUN: ld.lld -n --script %t.script %t.o -o %t
+// RUN: llvm-readobj --symbols %t | FileCheck %s --check-prefix=SYMS
----------------
Delete `-n` (--nmagic)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75042/new/

https://reviews.llvm.org/D75042