release/19.x: [lld][ARM] Fix assertion when mixing ARM and Thumb objects (#101985) #102292
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
|
@MaskRay What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: None (llvmbot) ChangesBackport a1c6467 Requested by: @ostannard Full diff: https://github.com/llvm/llvm-project/pull/102292.diff 4 Files Affected:
diff --git a/lld/ELF/Arch/ARM.cpp b/lld/ELF/Arch/ARM.cpp
index 3e0efe540e1bf..07a7535c4a231 100644
--- a/lld/ELF/Arch/ARM.cpp
+++ b/lld/ELF/Arch/ARM.cpp
@@ -228,10 +228,16 @@ static void writePltHeaderLong(uint8_t *buf) {
write32(buf + 16, gotPlt - l1 - 8);
}
+// True if we should use Thumb PLTs, which currently require Thumb2, and are
+// only used if the target does not have the ARM ISA.
+static bool useThumbPLTs() {
+ return config->armHasThumb2ISA && !config->armHasArmISA;
+}
+
// The default PLT header requires the .got.plt to be within 128 Mb of the
// .plt in the positive direction.
void ARM::writePltHeader(uint8_t *buf) const {
- if (config->armThumbPLTs) {
+ if (useThumbPLTs()) {
// The instruction sequence for thumb:
//
// 0: b500 push {lr}
@@ -289,7 +295,7 @@ void ARM::writePltHeader(uint8_t *buf) const {
}
void ARM::addPltHeaderSymbols(InputSection &isec) const {
- if (config->armThumbPLTs) {
+ if (useThumbPLTs()) {
addSyntheticLocal("$t", STT_NOTYPE, 0, 0, isec);
addSyntheticLocal("$d", STT_NOTYPE, 12, 0, isec);
} else {
@@ -315,7 +321,7 @@ static void writePltLong(uint8_t *buf, uint64_t gotPltEntryAddr,
void ARM::writePlt(uint8_t *buf, const Symbol &sym,
uint64_t pltEntryAddr) const {
- if (!config->armThumbPLTs) {
+ if (!useThumbPLTs()) {
uint64_t offset = sym.getGotPltVA() - pltEntryAddr - 8;
// The PLT entry is similar to the example given in Appendix A of ELF for
@@ -367,7 +373,7 @@ void ARM::writePlt(uint8_t *buf, const Symbol &sym,
}
void ARM::addPltSymbols(InputSection &isec, uint64_t off) const {
- if (config->armThumbPLTs) {
+ if (useThumbPLTs()) {
addSyntheticLocal("$t", STT_NOTYPE, off, 0, isec);
} else {
addSyntheticLocal("$a", STT_NOTYPE, off, 0, isec);
@@ -393,7 +399,7 @@ bool ARM::needsThunk(RelExpr expr, RelType type, const InputFile *file,
case R_ARM_JUMP24:
// Source is ARM, all PLT entries are ARM so no interworking required.
// Otherwise we need to interwork if STT_FUNC Symbol has bit 0 set (Thumb).
- assert(!config->armThumbPLTs &&
+ assert(!useThumbPLTs() &&
"If the source is ARM, we should not need Thumb PLTs");
if (s.isFunc() && expr == R_PC && (s.getVA() & 1))
return true;
@@ -407,7 +413,8 @@ bool ARM::needsThunk(RelExpr expr, RelType type, const InputFile *file,
case R_ARM_THM_JUMP24:
// Source is Thumb, when all PLT entries are ARM interworking is required.
// Otherwise we need to interwork if STT_FUNC Symbol has bit 0 clear (ARM).
- if ((expr == R_PLT_PC && !config->armThumbPLTs) || (s.isFunc() && (s.getVA() & 1) == 0))
+ if ((expr == R_PLT_PC && !useThumbPLTs()) ||
+ (s.isFunc() && (s.getVA() & 1) == 0))
return true;
[[fallthrough]];
case R_ARM_THM_CALL: {
@@ -675,7 +682,7 @@ void ARM::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
// PLT entries are always ARM state so we know we need to interwork.
assert(rel.sym); // R_ARM_THM_CALL is always reached via relocate().
bool bit0Thumb = val & 1;
- bool useThumb = bit0Thumb || config->armThumbPLTs;
+ bool useThumb = bit0Thumb || useThumbPLTs();
bool isBlx = (read16(loc + 2) & 0x1000) == 0;
// lld 10.0 and before always used bit0Thumb when deciding to write a BLX
// even when type not STT_FUNC.
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 0173be396163e..28726d48e4284 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -217,7 +217,8 @@ struct Config {
bool allowMultipleDefinition;
bool fatLTOObjects;
bool androidPackDynRelocs = false;
- bool armThumbPLTs = false;
+ bool armHasArmISA = false;
+ bool armHasThumb2ISA = false;
bool armHasBlx = false;
bool armHasMovtMovw = false;
bool armJ1J2BranchEncoding = false;
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index f1c0eb292361b..48f5a9609ecfb 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -203,10 +203,8 @@ static void updateSupportedARMFeatures(const ARMAttributeParser &attributes) {
attributes.getAttributeValue(ARMBuildAttrs::ARM_ISA_use);
std::optional<unsigned> thumb =
attributes.getAttributeValue(ARMBuildAttrs::THUMB_ISA_use);
- bool noArmISA = !armISA || *armISA == ARMBuildAttrs::Not_Allowed;
- bool hasThumb2 = thumb && *thumb >= ARMBuildAttrs::AllowThumb32;
- if (noArmISA && hasThumb2)
- config->armThumbPLTs = true;
+ config->armHasArmISA |= armISA && *armISA >= ARMBuildAttrs::Allowed;
+ config->armHasThumb2ISA |= thumb && *thumb >= ARMBuildAttrs::AllowThumb32;
}
InputFile::InputFile(Kind k, MemoryBufferRef m)
diff --git a/lld/test/ELF/arm-mixed-plts.s b/lld/test/ELF/arm-mixed-plts.s
new file mode 100644
index 0000000000000..801de70f4f101
--- /dev/null
+++ b/lld/test/ELF/arm-mixed-plts.s
@@ -0,0 +1,44 @@
+# REQUIRES: arm
+
+# RUN: rm -rf %t && split-file %s %t
+# RUN: llvm-mc -filetype=obj -arm-add-build-attributes -triple=armv7a-none-linux-gnueabi %t/a.s -o %t1.o
+# RUN: llvm-mc -filetype=obj -arm-add-build-attributes -triple=armv7a-none-linux-gnueabi %t/b.s -o %t2.o
+# RUN: ld.lld -shared %t1.o %t2.o -o %t.so
+# RUN: llvm-objdump -d %t.so | FileCheck %s
+
+## Check that, when the input is a mixture of objects which can and cannot use
+## the ARM ISA, we use the default ARM PLT sequences.
+
+# CHECK: <.plt>:
+# CHECK-NEXT: e52de004 str lr, [sp, #-0x4]!
+# CHECK-NEXT: e28fe600 add lr, pc, #0, #12
+# CHECK-NEXT: e28eea20 add lr, lr, #32, #20
+# CHECK-NEXT: e5bef084 ldr pc, [lr, #0x84]!
+# CHECK-NEXT: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: e28fc600 add r12, pc, #0, #12
+# CHECK-NEXT: e28cca20 add r12, r12, #32, #20
+# CHECK-NEXT: e5bcf06c ldr pc, [r12, #0x6c]!
+# CHECK-NEXT: d4 d4 d4 d4 .word 0xd4d4d4d4
+
+#--- a.s
+ .globl foo
+ .type foo, %function
+ .globl bar
+ .type bar, %function
+
+ .thumb
+foo:
+ bl bar
+ bx lr
+
+#--- b.s
+ .eabi_attribute Tag_ARM_ISA_use, 0
+
+ .arm
+ .globl bar
+ .type bar, %function
+bar:
+ bx lr
|
MaskRay
approved these changes
Aug 7, 2024
Previously, we selected the Thumb2 PLT sequences if any input object is marked as not supporting the ARM ISA, which then causes assertion failures when calls from ARM code in other objects are seen. I think the intention here was to only use Thumb PLTs when the target does not have the ARM ISA available, signalled by no objects being marked as having it available. To do that we need to track which ISAs we have seen as we parse the build attributes, and defer the decision about PLTs until all input objects have been parsed. This bug was triggered by real code in picolibc, which have some versions of string.h functions built with Thumb2-only build attributes, so that they are compatible with v7-A, v7-R and v7-M. Fixes llvm#99008. (cherry picked from commit a1c6467)
|
@ostannard (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
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.
Backport a1c6467
Requested by: @ostannard