[llvm] 0182334 - [ARM] [Windows] Error out on branch relocations that require a symbol offset (#101906)

[via llvm-commits]

Author: Martin Storsjö
Date: 2024-08-06T23:38:43+03:00
New Revision: 0182334e7b9fc472b3f7c9e059c3743301d487d5

URL: https://github.com/llvm/llvm-project/commit/0182334e7b9fc472b3f7c9e059c3743301d487d5
DIFF: https://github.com/llvm/llvm-project/commit/0182334e7b9fc472b3f7c9e059c3743301d487d5.diff

LOG: [ARM] [Windows] Error out on branch relocations that require a symbol offset (#101906)

This adds the same kind of verification for ARM, as was added for
AArch64 in 1e7f592a890aad860605cf5220530b3744e107ba. This allows
catching issues at assembly time, instead of having the linker
misinterpret the relocations (as the linker ignores the symbol offset).
This verifies that the issue fixed by
8dd065d5bc81b0c8ab57f365bb169a5d92928f25 really is fixed, and points out
explicitly if the same issue appears elsewhere.

Note that the parameter Value in the adjustFixupValue function is offset
by 4 from the value that is stored as immediate in the instructions, so
we compare with 4, when we want to make sure that the written immediate
will be zero.

Added: 
    

Modified: 
    llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
    llvm/test/MC/ARM/Windows/branch-reloc-offset.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index 994b43f1abb49..4e4a19ddf5584 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -587,6 +587,14 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
         return 0;
     return 0xffffff & ((Value - 8) >> 2);
   case ARM::fixup_t2_uncondbranch: {
+    if (STI->getTargetTriple().isOSBinFormatCOFF() && !IsResolved &&
+        Value != 4) {
+      // MSVC link.exe and lld do not support this relocation type
+      // with a non-zero offset. ("Value" is offset by 4 at this point.)
+      Ctx.reportError(Fixup.getLoc(),
+                      "cannot perform a PC-relative fixup with a non-zero "
+                      "symbol offset");
+    }
     Value = Value - 4;
     if (!isInt<25>(Value)) {
       Ctx.reportError(Fixup.getLoc(), "Relocation out of range");
@@ -637,6 +645,14 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
       Ctx.reportError(Fixup.getLoc(), "Relocation out of range");
       return 0;
     }
+    if (STI->getTargetTriple().isOSBinFormatCOFF() && !IsResolved &&
+        Value != 4) {
+      // MSVC link.exe and lld do not support this relocation type
+      // with a non-zero offset. ("Value" is offset by 4 at this point.)
+      Ctx.reportError(Fixup.getLoc(),
+                      "cannot perform a PC-relative fixup with a non-zero "
+                      "symbol offset");
+    }
 
     // The value doesn't encode the low bit (always zero) and is offset by
     // four. The 32-bit immediate value is encoded as
@@ -666,6 +682,14 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
                          Endian == llvm::endianness::little);
   }
   case ARM::fixup_arm_thumb_blx: {
+    if (STI->getTargetTriple().isOSBinFormatCOFF() && !IsResolved &&
+        Value != 4) {
+      // MSVC link.exe and lld do not support this relocation type
+      // with a non-zero offset. ("Value" is offset by 4 at this point.)
+      Ctx.reportError(Fixup.getLoc(),
+                      "cannot perform a PC-relative fixup with a non-zero "
+                      "symbol offset");
+    }
     // The value doesn't encode the low two bits (always zero) and is offset by
     // four (see fixup_arm_thumb_cp). The 32-bit immediate value is encoded as
     //   imm32 = SignExtend(S:I1:I2:imm10H:imm10L:00)

diff  --git a/llvm/test/MC/ARM/Windows/branch-reloc-offset.s b/llvm/test/MC/ARM/Windows/branch-reloc-offset.s
index 2e70a723ccf78..e7d59cdcf6d40 100644
--- a/llvm/test/MC/ARM/Windows/branch-reloc-offset.s
+++ b/llvm/test/MC/ARM/Windows/branch-reloc-offset.s
@@ -1,4 +1,5 @@
 // RUN: llvm-mc -triple thumbv7-windows-gnu -filetype obj %s -o - | llvm-objdump -D -r - | FileCheck %s
+// RUN: not llvm-mc -triple thumbv7-windows-gnu -filetype obj --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
 
     .text
 main:
@@ -55,3 +56,40 @@ main:
 // CHECK:        e: bf00          nop
 // CHECK: 00000010 <.Lother_target>:
 // CHECK:       10: 4770          bx      lr
+
+.ifdef ERR
+    .section "other2", "xr"
+err:
+    nop
+
+// Test errors, if referencing a symbol with an offset
+
+    b .Lerr_target+4
+// ERR: [[#@LINE-1]]:5: error: cannot perform a PC-relative fixup with a non-zero symbol offset
+    bl .Lerr_target+4
+// ERR: [[#@LINE-1]]:5: error: cannot perform a PC-relative fixup with a non-zero symbol offset
+    blx .Lerr_target+4
+// ERR: [[#@LINE-1]]:5: error: cannot perform a PC-relative fixup with a non-zero symbol offset
+
+// Test errors, if referencing a private label which lacks .def/.scl/.type/.endef, in another
+// section, without an offset. Such symbols are omitted from the output symbol table, so the
+// relocation can't reference them. Such relocations usually are made towards the base of the
+// section plus an offset, but such an offset is not supported with this relocation.
+
+    b .Lerr_target2
+// ERR: [[#@LINE-1]]:5: error: cannot perform a PC-relative fixup with a non-zero symbol offset
+
+    .def .Lerr_target
+    .scl 3
+    .type 32
+    .endef
+.Lerr_target:
+    nop
+    nop
+    bx lr
+
+    .section "other3", "xr"
+    nop
+.Lerr_target2:
+    bx lr
+.endif