[llvm] [ARMAsmBackend] Add checks for relocation addends in assembler (PR #109969)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 29 02:01:18 PDT 2024
https://github.com/jcohen-apple updated https://github.com/llvm/llvm-project/pull/109969
>From 70db41e8fdfb7d0e2c853b2b213ada2e82ce9c67 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 1/2] [ARMAsmBackend] Add tests for invalid relocation values
The 32 bit ARM backend does not have range or alignment checks on constant addends that can be added to relocations.
---
.../Target/ARM/MCTargetDesc/ARMAsmBackend.cpp | 15 ++++++++--
.../MC/ARM/macho-relocs-with-addend-invalid.s | 28 +++++++++++++++++++
llvm/test/MC/ARM/macho-relocs-with-addend.s | 4 +--
3 files changed, 43 insertions(+), 4 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..55f55a49e2eaa9 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -579,13 +579,24 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
case ARM::fixup_arm_uncondbl:
case ARM::fixup_arm_condbl:
case ARM::fixup_arm_blx:
+ // Check that the fixup value is legal.
+ Value = Value - 8;
+ if (!isInt<25>(Value)) {
+ Ctx.reportError(Fixup.getLoc(), "Relocation out of range");
+ return 0;
+ }
+ if (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) {
@@ -1121,7 +1132,7 @@ void ARMAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
assert(Offset + NumBytes <= Data.size() && "Invalid fixup offset!");
// Used to point to big endian bytes.
- unsigned FullSizeBytes;
+ unsigned FullSizeBytes = 0;
if (Endian == llvm::endianness::big) {
FullSizeBytes = getFixupKindContainerSizeBytes(Kind);
assert((Offset + FullSizeBytes) <= Data.size() && "Invalid fixup size!");
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..11082ab0aebdbd
--- /dev/null
+++ b/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
@@ -0,0 +1,28 @@
+// RUN: not llvm-mc -triple armv7-apple-darwin -filetype=obj %s 2>&1 | FileCheck %s
+
+_foo:
+ // Check that the relocation size is valid.
+
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ bl _foo+0xfffffff00
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ blx _foo+0xfffffff00
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ b _foo+0xfffffff00
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ ble _foo+0xfffffff00
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ beq _foo+0xfffffff00
+
+ // Check that the relocation alignment is valid.
+
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+ bl _foo+0x101
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+ blx _foo+0x101
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+ b _foo+0x101
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+ ble _foo+0x101
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
+ beq _foo+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
>From aee552fc167c9b0ce61c892e7c377b464ec5b543 Mon Sep 17 00:00:00 2001
From: Jonathan Cohen <jcohen22 at apple.com>
Date: Wed, 25 Sep 2024 13:37:22 +0300
Subject: [PATCH 2/2] [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 | 12 +++--
.../MC/ARM/macho-relocs-with-addend-invalid.s | 53 ++++++++++++++-----
2 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index 55f55a49e2eaa9..d80a6d3830382d 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -579,13 +579,15 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
case ARM::fixup_arm_uncondbl:
case ARM::fixup_arm_condbl:
case ARM::fixup_arm_blx:
- // Check that the fixup value is legal.
- Value = Value - 8;
- if (!isInt<25>(Value)) {
+ // Check that the relocation value is legal.
+ if (!isInt<26>(Value)) {
Ctx.reportError(Fixup.getLoc(), "Relocation out of range");
return 0;
}
- if (Value % 4 != 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;
}
@@ -1132,7 +1134,7 @@ void ARMAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
assert(Offset + NumBytes <= Data.size() && "Invalid fixup offset!");
// Used to point to big endian bytes.
- unsigned FullSizeBytes = 0;
+ unsigned FullSizeBytes;
if (Endian == llvm::endianness::big) {
FullSizeBytes = getFixupKindContainerSizeBytes(Kind);
assert((Offset + FullSizeBytes) <= Data.size() && "Invalid fixup size!");
diff --git a/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s b/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
index 11082ab0aebdbd..d987d5404cd2a7 100644
--- a/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
+++ b/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
@@ -1,28 +1,55 @@
// RUN: not llvm-mc -triple armv7-apple-darwin -filetype=obj %s 2>&1 | FileCheck %s
-_foo:
- // Check that the relocation size is valid.
+// Check that the relocation size is valid.
+// Check outside of range of the largest accepted positive number
+_foo1:
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ b _foo1+33554432
+// Check Same as above, for smallest negative value
+_foo2:
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
- bl _foo+0xfffffff00
+ b _foo2-33554436
+
+// Edge case - subtracting positive number
+_foo3:
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
- blx _foo+0xfffffff00
+ b _foo3-0x2000010
+
+// Edge case - adding negative number
+_foo4:
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
- b _foo+0xfffffff00
+ b _foo4+0x2000008
+
+_foo5:
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
- ble _foo+0xfffffff00
+ bl _foo5+33554432
+
+_foo6:
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
- beq _foo+0xfffffff00
+ blx _foo6+33554432
- // Check that the relocation alignment is valid.
+// blx instruction is aligned to 16-bits.
+_foo7:
+ // CHECK-NOT:[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ blx _foo6+33554430
+_foo8:
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ ble _foo8+33554432
+
+_foo9:
+ // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation out of range
+ beq _foo9+33554432
+
+ // Check that the relocation alignment is valid.
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
- bl _foo+0x101
+ bl _foo1+0x101
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
- blx _foo+0x101
+ blx _foo1+0x101
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
- b _foo+0x101
+ b _foo1+0x101
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
- ble _foo+0x101
+ ble _foo1+0x101
// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Relocation not aligned
- beq _foo+0x101
+ beq _foo1+0x101
More information about the llvm-commits
mailing list