[llvm] [ARMAsmBackend] Add checks for relocation addends in assembler (PR #109969)

[Jonathan Cohen via llvm-commits]

https://github.com/jcohen-apple updated https://github.com/llvm/llvm-project/pull/109969

>From 7253809dbd124e20d7719c64e46ab22bb1ef05fd Mon Sep 17 00:00:00 2001
From: Jonathan Cohen <jcohen22 at apple.com>
Date: Tue, 24 Sep 2024 17:28:47 +0300
Subject: [PATCH] [ARMAsmBackend] Add range and alignment checks for armv7
 branches

Currently we will silently truncate and round the relocation values input to the assembler, and produce branch instructions with a bad offset. After this change, the assembler will fail if the relocation value cannot be encoded into the instruction.
---
 .../Target/ARM/MCTargetDesc/ARMAsmBackend.cpp | 16 ++++-
 .../MC/ARM/macho-relocs-with-addend-invalid.s | 67 +++++++++++++++++++
 llvm/test/MC/ARM/macho-relocs-with-addend.s   |  4 +-
 3 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s

diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index 9f6bc31f8aa030..1223210a76f6e3 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -579,13 +579,27 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
   case ARM::fixup_arm_uncondbl:
   case ARM::fixup_arm_condbl:
   case ARM::fixup_arm_blx:
+    // Check that the relocation value is legal.
+    Value -= 8;
+    if (!isInt<26>(Value)) {
+      Ctx.reportError(Fixup.getLoc(), "Relocation out of range");
+      return 0;
+    }
+    // Alignment differs for blx. Because we are switching to thumb ISA, we use
+    // 16-bit alignment. Otherwise, use 32-bit.
+    if ((Kind == ARM::fixup_arm_blx && Value % 2 != 0) ||
+        (Kind != ARM::fixup_arm_blx && Value % 4 != 0)) {
+      Ctx.reportError(Fixup.getLoc(), "Relocation not aligned");
+      return 0;
+    }
+
     // These values don't encode the low two bits since they're always zero.
     // Offset by 8 just as above.
     if (const MCSymbolRefExpr *SRE =
             dyn_cast<MCSymbolRefExpr>(Fixup.getValue()))
       if (SRE->getKind() == MCSymbolRefExpr::VK_TLSCALL)
         return 0;
-    return 0xffffff & ((Value - 8) >> 2);
+    return 0xffffff & (Value >> 2);
   case ARM::fixup_t2_uncondbranch: {
     if (STI->getTargetTriple().isOSBinFormatCOFF() && !IsResolved &&
         Value != 4) {
diff --git a/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s b/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
new file mode 100644
index 00000000000000..4ecb73814dfac6
--- /dev/null
+++ b/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
@@ -0,0 +1,67 @@
+// RUN: not llvm-mc -triple armv7-apple-darwin -filetype=obj %s 2>&1 | FileCheck %s
+
+// Check that the relocation size is valid.
+// Check lower bound of edge case.
+_foo1_valid:
+    // CHECK-NOT: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    b   _foo1_valid+0x2000004
+// Check outside of range of the largest accepted positive number
+_foo1:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    b   _foo1+0x2000008
+
+// Check Same as above, for smallest negative value
+_foo2_valid:
+    // CHECK-NOT: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    b   _foo2_valid-0x1FFFFF8
+_foo2:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    b   _foo2-0x1FFFFFC
+
+// Edge case - subtracting positive number
+_foo3:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    b   _foo3-0x2000010
+
+// Edge case - adding negative number
+_foo4:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    b   _foo4+0x2000008
+
+_foo5:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    bl  _foo5+0x2000008
+
+_foo6:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    blx _foo6+0x2000008
+
+// blx instruction is aligned to 16-bits.
+_foo7_blx:
+    // CHECK-NOT:[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    blx _foo7_blx+0x1FFFFFE
+
+// Other branch instructions require 32-bit alignment.
+_foo7:
+    // CHECK:[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    bl _foo7_blx+0x1FFFFFE
+
+_foo8:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    ble _foo8+0x2000008
+
+_foo9:
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+    beq _foo9+0x2000008
+
+    // Check that the relocation alignment is valid.
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    bl  _foo1+0x101
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    blx _foo1+0x101
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    b   _foo1+0x101
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    ble _foo1+0x101
+    // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+    beq _foo1+0x101
diff --git a/llvm/test/MC/ARM/macho-relocs-with-addend.s b/llvm/test/MC/ARM/macho-relocs-with-addend.s
index fee930eee1b3b5..f5421baf610863 100644
--- a/llvm/test/MC/ARM/macho-relocs-with-addend.s
+++ b/llvm/test/MC/ARM/macho-relocs-with-addend.s
@@ -15,9 +15,9 @@ _with_thumb:
         .globl _with_arm
         .arm
 _with_arm:
-        bl _dest+10
+        bl _dest+12
         blx _dest+20
-        bne _dest+30
+        bne _dest+32
         b _dest+40
 
         .data