[lld] [lld][ARM] Fix assertion when mixing ARM and Thumb objects (PR #101985)
Oliver Stannard via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 01:27:32 PDT 2024
https://github.com/ostannard updated https://github.com/llvm/llvm-project/pull/101985
>From 9c92a4a1610dbd58080bb9d9725bf3bc7cc1c1d5 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Mon, 5 Aug 2024 14:41:02 +0100
Subject: [PATCH 1/4] [lld][ARM] Fix assertion when mixing ARM and Thumb
objects
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 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 avaiable.
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 #99008
---
lld/ELF/Config.h | 2 ++
lld/ELF/Driver.cpp | 11 +++++++++
lld/ELF/InputFiles.cpp | 6 ++---
lld/test/ELF/arm-mixed-plts.s | 44 +++++++++++++++++++++++++++++++++++
4 files changed, 59 insertions(+), 4 deletions(-)
create mode 100644 lld/test/ELF/arm-mixed-plts.s
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 183dc88a93e2f..d3ea095940cd8 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -218,6 +218,8 @@ struct Config {
bool allowMultipleDefinition;
bool fatLTOObjects;
bool androidPackDynRelocs = false;
+ bool armHasArmISA = false;
+ bool armHasThumb2ISA = false;
bool armThumbPLTs = false;
bool armHasBlx = false;
bool armHasMovtMovw = false;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index a8c52e8c2b8e1..1928648a7c115 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -82,6 +82,7 @@ ConfigWrapper elf::config;
Ctx elf::ctx;
static void setConfigs(opt::InputArgList &args);
+static void setConfigsPostParse();
static void readConfigs(opt::InputArgList &args);
void elf::errorOrWarn(const Twine &msg) {
@@ -1886,6 +1887,14 @@ static void setConfigs(opt::InputArgList &args) {
}
}
+// Initialise config options which depend on information previously parsed from
+// input files, such as Arm build attributes.
+static void setConfigsPostParse() {
+ // Currently, Thumb PLTs require Thumb2, and are only used for targets which
+ // do not have the ARM ISA.
+ config->armThumbPLTs = config->armHasThumb2ISA && !config->armHasArmISA;
+}
+
static bool isFormatBinary(StringRef s) {
if (s == "binary")
return true;
@@ -2897,6 +2906,8 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
// No more lazy bitcode can be extracted at this point. Do post parse work
// like checking duplicate symbols.
+ setConfigsPostParse();
+
parallelForEach(ctx.objectFiles, [](ELFFileBase *file) {
initSectionsAndLocalSyms(file, /*ignoreComdats=*/false);
});
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 3bc39848d6035..b87a3ab429219 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -200,10 +200,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..5699371449532
--- /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: 000101d0 <.plt>:
+# CHECK-NEXT: 101d0: e52de004 str lr, [sp, #-0x4]!
+# CHECK-NEXT: 101d4: e28fe600 add lr, pc, #0, #12
+# CHECK-NEXT: 101d8: e28eea20 add lr, lr, #32, #20
+# CHECK-NEXT: 101dc: e5bef084 ldr pc, [lr, #0x84]!
+# CHECK-NEXT: 101e0: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: 101e4: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: 101e8: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: 101ec: d4 d4 d4 d4 .word 0xd4d4d4d4
+# CHECK-NEXT: 101f0: e28fc600 add r12, pc, #0, #12
+# CHECK-NEXT: 101f4: e28cca20 add r12, r12, #32, #20
+# CHECK-NEXT: 101f8: e5bcf06c ldr pc, [r12, #0x6c]!
+# CHECK-NEXT: 101fc: 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
>From 539ee79f19df914cfec71206a86b1b85fcf8a4e6 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Mon, 5 Aug 2024 18:06:23 +0100
Subject: [PATCH 2/4] Avoid creating setConfigsPostParse
---
lld/ELF/Arch/ARM.cpp | 20 +++++++++++++-------
lld/ELF/Config.h | 1 -
lld/ELF/Driver.cpp | 11 -----------
3 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/lld/ELF/Arch/ARM.cpp b/lld/ELF/Arch/ARM.cpp
index 3e0efe540e1bf..dc127650db77d 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,7 @@ 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 +681,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 d3ea095940cd8..ca7373b686f06 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -220,7 +220,6 @@ struct Config {
bool androidPackDynRelocs = false;
bool armHasArmISA = false;
bool armHasThumb2ISA = false;
- bool armThumbPLTs = false;
bool armHasBlx = false;
bool armHasMovtMovw = false;
bool armJ1J2BranchEncoding = false;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 1928648a7c115..a8c52e8c2b8e1 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -82,7 +82,6 @@ ConfigWrapper elf::config;
Ctx elf::ctx;
static void setConfigs(opt::InputArgList &args);
-static void setConfigsPostParse();
static void readConfigs(opt::InputArgList &args);
void elf::errorOrWarn(const Twine &msg) {
@@ -1887,14 +1886,6 @@ static void setConfigs(opt::InputArgList &args) {
}
}
-// Initialise config options which depend on information previously parsed from
-// input files, such as Arm build attributes.
-static void setConfigsPostParse() {
- // Currently, Thumb PLTs require Thumb2, and are only used for targets which
- // do not have the ARM ISA.
- config->armThumbPLTs = config->armHasThumb2ISA && !config->armHasArmISA;
-}
-
static bool isFormatBinary(StringRef s) {
if (s == "binary")
return true;
@@ -2906,8 +2897,6 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
// No more lazy bitcode can be extracted at this point. Do post parse work
// like checking duplicate symbols.
- setConfigsPostParse();
-
parallelForEach(ctx.objectFiles, [](ELFFileBase *file) {
initSectionsAndLocalSyms(file, /*ignoreComdats=*/false);
});
>From b7682d20fb5979be3ef98e29b0a3225836b61f9d Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 6 Aug 2024 09:21:14 +0100
Subject: [PATCH 3/4] clang-format
---
lld/ELF/Arch/ARM.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lld/ELF/Arch/ARM.cpp b/lld/ELF/Arch/ARM.cpp
index dc127650db77d..07a7535c4a231 100644
--- a/lld/ELF/Arch/ARM.cpp
+++ b/lld/ELF/Arch/ARM.cpp
@@ -413,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 && !useThumbPLTs()) || (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: {
>From dac2baf8ebce7ca8120944663a58f44a61ac8fae Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 6 Aug 2024 09:22:14 +0100
Subject: [PATCH 4/4] Test formatting
---
lld/test/ELF/arm-mixed-plts.s | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/lld/test/ELF/arm-mixed-plts.s b/lld/test/ELF/arm-mixed-plts.s
index 5699371449532..801de70f4f101 100644
--- a/lld/test/ELF/arm-mixed-plts.s
+++ b/lld/test/ELF/arm-mixed-plts.s
@@ -6,22 +6,22 @@
# 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 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: 000101d0 <.plt>:
-# CHECK-NEXT: 101d0: e52de004 str lr, [sp, #-0x4]!
-# CHECK-NEXT: 101d4: e28fe600 add lr, pc, #0, #12
-# CHECK-NEXT: 101d8: e28eea20 add lr, lr, #32, #20
-# CHECK-NEXT: 101dc: e5bef084 ldr pc, [lr, #0x84]!
-# CHECK-NEXT: 101e0: d4 d4 d4 d4 .word 0xd4d4d4d4
-# CHECK-NEXT: 101e4: d4 d4 d4 d4 .word 0xd4d4d4d4
-# CHECK-NEXT: 101e8: d4 d4 d4 d4 .word 0xd4d4d4d4
-# CHECK-NEXT: 101ec: d4 d4 d4 d4 .word 0xd4d4d4d4
-# CHECK-NEXT: 101f0: e28fc600 add r12, pc, #0, #12
-# CHECK-NEXT: 101f4: e28cca20 add r12, r12, #32, #20
-# CHECK-NEXT: 101f8: e5bcf06c ldr pc, [r12, #0x6c]!
-# CHECK-NEXT: 101fc: d4 d4 d4 d4 .word 0xd4d4d4d4
+# 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
More information about the llvm-commits
mailing list