[llvm] [ARMAsmBackend] Add checks for relocation addends in assembler (PR #109969)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 25 05:29:24 PDT 2024
https://github.com/jcohen-apple created https://github.com/llvm/llvm-project/pull/109969
This PR adds checks that any addends attached to branch instructions are valid, and can be properly encoded in the branch instruction. Before this fix, the assembler would silently truncate or round invalid addend values, creating incorrect branch instructions.
>From 218bd0c1db88337a076edef6e0749d47c20d94d6 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 | 5 ++--
.../MC/ARM/macho-relocs-with-addend-invalid.s | 28 +++++++++++++++++++
2 files changed, 31 insertions(+), 2 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..df35b0b05d66c0 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -1121,12 +1121,12 @@ 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!");
assert(NumBytes <= FullSizeBytes && "Invalid fixup size!");
- }
+ }
// For each byte of the fragment that the fixup touches, mask in the bits from
// the fixup value. The Value has been "split up" into the appropriate
@@ -1135,6 +1135,7 @@ void ARMAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
unsigned Idx =
Endian == llvm::endianness::little ? i : (FullSizeBytes - 1 - i);
Data[Offset + Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
+
}
}
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..0c373fb67a1643
--- /dev/null
+++ b/llvm/test/MC/ARM/macho-relocs-with-addend-invalid.s
@@ -0,0 +1,28 @@
+// RUN: llvm-mc -triple armv7-apple-darwin -filetype=obj %s | llvm-objdump -d --macho - | FileCheck %s
+
+_foo:
+ // First issue: relocation addend range not checked, silently truncated
+ // CHECK: bl 0xffffff00
+ // CHECK: blx 0xffffff00
+ // CHECK: b 0xffffff00
+ // CHECK: ble 0xffffff00
+ // CHECK: beq 0xffffff00
+
+ bl _foo+0xfffffff00
+ blx _foo+0xfffffff00
+ b _foo+0xfffffff00
+ ble _foo+0xfffffff00
+ beq _foo+0xfffffff00
+
+ // Second bug: relocation addend alignment not checked, silently rounded
+ // CHECK: bl 0x100
+ // CHECK: blx 0x100
+ // CHECK: b 0x100
+ // CHECK: ble 0x100
+ // CHECK: beq 0x100
+
+ bl _foo+0x101
+ blx _foo+0x101
+ b _foo+0x101
+ ble _foo+0x101
+ beq _foo+0x101
>From 458238b7f5dde04e7c334ad08a7fd2d977612318 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 | 16 +++++++++--
.../MC/ARM/macho-relocs-with-addend-invalid.s | 28 +++++++++----------
llvm/test/MC/ARM/macho-relocs-with-addend.s | 4 +--
3 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index df35b0b05d66c0..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) {
@@ -1126,7 +1137,7 @@ void ARMAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
FullSizeBytes = getFixupKindContainerSizeBytes(Kind);
assert((Offset + FullSizeBytes) <= Data.size() && "Invalid fixup size!");
assert(NumBytes <= FullSizeBytes && "Invalid fixup size!");
- }
+ }
// For each byte of the fragment that the fixup touches, mask in the bits from
// the fixup value. The Value has been "split up" into the appropriate
@@ -1135,7 +1146,6 @@ void ARMAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
unsigned Idx =
Endian == llvm::endianness::little ? i : (FullSizeBytes - 1 - i);
Data[Offset + Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
-
}
}
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 0c373fb67a1643..11082ab0aebdbd 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,28 @@
-// RUN: llvm-mc -triple armv7-apple-darwin -filetype=obj %s | llvm-objdump -d --macho - | FileCheck %s
+// RUN: not llvm-mc -triple armv7-apple-darwin -filetype=obj %s 2>&1 | FileCheck %s
_foo:
- // First issue: relocation addend range not checked, silently truncated
- // CHECK: bl 0xffffff00
- // CHECK: blx 0xffffff00
- // CHECK: b 0xffffff00
- // CHECK: ble 0xffffff00
- // CHECK: beq 0xffffff00
+ // 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
- // Second bug: relocation addend alignment not checked, silently rounded
- // CHECK: bl 0x100
- // CHECK: blx 0x100
- // CHECK: b 0x100
- // CHECK: ble 0x100
- // CHECK: beq 0x100
-
+ // 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
More information about the llvm-commits
mailing list