[ARM] [Windows] Error out on branch relocations that require a symbol offset #101906
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… offset This adds the same kind of verification for ARM, as was added for AArch64 in 1e7f592. 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 8dd065d 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.
mstorsjo
force-pushed
the
arm-error-branch-offset
branch
from
2292108 to
e513005
Compare
|
@llvm/pr-subscribers-backend-arm Author: Martin Storsjö (mstorsjo) ChangesThis adds the same kind of verification for ARM, as was added for AArch64 in 1e7f592. 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 8dd065d 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. Full diff: https://github.com/llvm/llvm-project/pull/101906.diff 2 Files Affected:
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
|
|
@llvm/pr-subscribers-mc Author: Martin Storsjö (mstorsjo) ChangesThis adds the same kind of verification for ARM, as was added for AArch64 in 1e7f592. 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 8dd065d 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. Full diff: https://github.com/llvm/llvm-project/pull/101906.diff 2 Files Affected:
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
|
cjacek
approved these changes
Aug 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the same kind of verification for ARM, as was added for AArch64 in 1e7f592. 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 8dd065d 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.