Fix lld crash using --fix-cortex-a53-843419 #170495
Open
+155
−25
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
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Member
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-lld Author: Tarcísio Fischer (tarcisiofischer) ChangesOriginal crash was observed in Chromium, in [1]. The empty buffer is actually from a Patch843419Section. When the patched code grows too much, it gets far away from the short jump, and the current implementation assumes a R_AARCH64_JUMP26 will be enough. This PR changes the implementation to: [1] https://issues.chromium.org/issues/440019454 Patch is 22.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170495.diff 9 Files Affected:
diff --git a/lld/ELF/AArch64ErrataFix.cpp b/lld/ELF/AArch64ErrataFix.cpp
index fe8869d237b4d..a9f6c43f86d61 100644
--- a/lld/ELF/AArch64ErrataFix.cpp
+++ b/lld/ELF/AArch64ErrataFix.cpp
@@ -388,6 +388,8 @@ class elf::Patch843419Section final : public SyntheticSection {
uint64_t patcheeOffset;
// A label for the start of the Patch that we can use as a relocation target.
Symbol *patchSym;
+ // A label for the return location.
+ Symbol *retSym;
};
Patch843419Section::Patch843419Section(Ctx &ctx, InputSection *p, uint64_t off)
@@ -399,6 +401,12 @@ Patch843419Section::Patch843419Section(Ctx &ctx, InputSection *p, uint64_t off)
ctx, ctx.saver.save("__CortexA53843419_" + utohexstr(getLDSTAddr())),
STT_FUNC, 0, getSize(), *this);
addSyntheticLocal(ctx, ctx.saver.save("$x"), STT_NOTYPE, 0, 0, *this);
+ retSym = addSyntheticLocal(
+ ctx, ctx.saver.save("__CortexA53843419_" + utohexstr(getLDSTAddr()) + "_ret"),
+ STT_FUNC, off + 4, 4, *p);
+
+ // Relocation must be created as soon as possible, so it'll be picked up.
+ addReloc({R_PC, R_AARCH64_JUMP26, 4, 0, retSym});
}
uint64_t Patch843419Section::getLDSTAddr() const {
@@ -410,13 +418,12 @@ void Patch843419Section::writeTo(uint8_t *buf) {
// patchee Section.
write32le(buf, read32le(patchee->content().begin() + patcheeOffset));
- // Apply any relocation transferred from the original patchee section.
- ctx.target->relocateAlloc(*this, buf);
+ // Note: The jump back was configured in this classe's constructor, and
+ // will be filled by the relocation. Adding the relocation here would be
+ // too late.
- // Return address is the next instruction after the one we have just copied.
- uint64_t s = getLDSTAddr() + 4;
- uint64_t p = patchSym->getVA(ctx) + 4;
- ctx.target->relocateNoSym(buf + 4, R_AARCH64_JUMP26, s - p);
+ // Apply relocations
+ ctx.target->relocateAlloc(*this, buf);
}
void AArch64Err843419Patcher::init() {
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 2a97df4785ecb..7d18ad8cfdadd 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -48,6 +48,11 @@ bool elf::isAArch64BTILandingPad(Ctx &ctx, Symbol &s, int64_t a) {
if (off >= isec->getSize())
return true;
const uint8_t *buf = isec->content().begin();
+ // Synthetic sections may have a size but empty data - Assume that they won't contain a landing pad
+ if (buf == nullptr && dyn_cast<SyntheticSection>(isec) != nullptr) {
+ return false;
+ }
+
const uint32_t instr = read32le(buf + off);
// All BTI instructions are HINT instructions which all have same encoding
// apart from bits [11:5]
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index d21376fd3ee47..de39fd0a7cab9 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1922,7 +1922,7 @@ ThunkSection *ThunkCreator::getISThunkSec(InputSection *isec) {
if (isec->outSecOff < first->outSecOff || last->outSecOff < isec->outSecOff)
continue;
- ts = addThunkSection(tos, isd, isec->outSecOff);
+ ts = addThunkSection(tos, isd, isec->outSecOff, /* isPrefix */ true);
thunkedSections[isec] = ts;
return ts;
}
@@ -1981,11 +1981,12 @@ void ThunkCreator::createInitialThunkSections(
ThunkSection *ThunkCreator::addThunkSection(OutputSection *os,
InputSectionDescription *isd,
- uint64_t off) {
+ uint64_t off,
+ bool isPrefix) {
auto *ts = make<ThunkSection>(ctx, os, off);
ts->partition = os->partition;
if ((ctx.arg.fixCortexA53Errata843419 || ctx.arg.fixCortexA8) &&
- !isd->sections.empty()) {
+ !isd->sections.empty() && !isPrefix) {
// The errata fixes are sensitive to addresses modulo 4 KiB. When we add
// thunks we disturb the base addresses of sections placed after the thunks
// this makes patches we have generated redundant, and may cause us to
diff --git a/lld/ELF/Relocations.h b/lld/ELF/Relocations.h
index 86ca298cd7a56..1ed39bc5f64ac 100644
--- a/lld/ELF/Relocations.h
+++ b/lld/ELF/Relocations.h
@@ -203,7 +203,7 @@ class ThunkCreator {
std::pair<Thunk *, bool> getSyntheticLandingPad(Defined &d, int64_t a);
ThunkSection *addThunkSection(OutputSection *os, InputSectionDescription *,
- uint64_t off);
+ uint64_t off, bool isPrefix = false);
bool normalizeExistingThunk(Relocation &rel, uint64_t src);
diff --git a/lld/test/ELF/aarch64-cortex-a53-843419-address.s b/lld/test/ELF/aarch64-cortex-a53-843419-address.s
index d994b818ab26a..946a639d90ec7 100644
--- a/lld/test/ELF/aarch64-cortex-a53-843419-address.s
+++ b/lld/test/ELF/aarch64-cortex-a53-843419-address.s
@@ -41,6 +41,7 @@
// CHECK-NEXT: ff8: d0000020 adrp x0, 0x6000
// CHECK-NEXT: ffc: f9400021 ldr x1, [x1]
// CHECK-NEXT: 1000: 14000ff9 b 0x4fe4
+// CHECK: <__CortexA53843419_1000_ret>:
// CHECK-NEXT: 1004: d65f03c0 ret
.section .text.01, "ax", %progbits
.balign 4096
@@ -63,6 +64,7 @@ $x.999:
// CHECK-NEXT: 1ffc: b0000020 adrp x0, 0x6000
// CHECK-NEXT: 2000: bd400021 ldr s1, [x1]
// CHECK-NEXT: 2004: 14000bfa b 0x4fec
+// CHECK: <__CortexA53843419_2004_ret>:
// CHECK-NEXT: 2008: d65f03c0 ret
.globl t3_ffc_ldrsimd
.type t3_ffc_ldrsimd, %function
@@ -100,6 +102,7 @@ t3_ff8_ldralldata:
// CHECK-NEXT: 3ff8: f0000000 adrp x0, 0x6000
// CHECK-NEXT: 3ffc: f9400021 ldr x1, [x1]
// CHECK-NEXT: 4000: 140003fd b 0x4ff4
+// CHECK: <__CortexA53843419_4000_ret>:
// CHECK-NEXT: 4004: d65f03c0 ret
.space 4096 - 12
.globl t3_ffc_ldr
@@ -132,6 +135,7 @@ t3_ff8_ldralldata:
// CHECK-NEXT: 4ffc: d0000000 adrp x0, 0x6000
// CHECK-NEXT: 5000: f9000021 str x1, [x1]
// CHECK-NEXT: 5004: 140003fb b 0x5ff0
+// CHECK: <__CortexA53843419_5004_ret>:
// CHECK-NEXT: 5008: d65f03c0 ret
.section .newisd, "ax", %progbits
@@ -157,6 +161,7 @@ t3_ffc_str:
// CHECK-NEXT: 5ff8: b0000000 adrp x0, 0x6000
// CHECK-NEXT: 5ffc: f9000021 str x1, [x1]
// CHECK-NEXT: 6000: 14000003 b 0x600c
+// CHECK: <__CortexA53843419_6000_ret>:
// CHECK-NEXT: 6004: d65f03c0 ret
.section .newos, "ax", %progbits
diff --git a/lld/test/ELF/aarch64-cortex-a53-843419-large.s b/lld/test/ELF/aarch64-cortex-a53-843419-large.s
index 71c81dbcfdf3d..1ed2864490aff 100644
--- a/lld/test/ELF/aarch64-cortex-a53-843419-large.s
+++ b/lld/test/ELF/aarch64-cortex-a53-843419-large.s
@@ -45,6 +45,7 @@ t3_ff8_ldr:
// CHECK3-NEXT: 211ff8: f00400e0 adrp x0, 0x8230000
// CHECK3-NEXT: 211ffc: f9400021 ldr x1, [x1]
// CHECK3-NEXT: 212000: 15800802 b 0x6214008
+// CHECK3: <__CortexA53843419_211000_ret>:
// CHECK3-NEXT: 212004: d65f03c0 ret
.section .text.04, "ax", %progbits
@@ -66,6 +67,7 @@ t3_ff8_str:
// CHECK4-NEXT: 4213ff8: b00200e0 adrp x0, 0x8230000
// CHECK4-NEXT: 4213ffc: f9400021 ldr x1, [x1]
// CHECK4-NEXT: 4214000: 14800004 b 0x6214010
+// CHECK4: <__CortexA53843419_4213000_ret>:
// CHECK4-NEXT: 4214004: d65f03c0 ret
.section .text.06, "ax", %progbits
@@ -105,6 +107,7 @@ t3_ffc_ldr:
// CHECK7-NEXT: 8211ffc: f00000e0 adrp x0, 0x8230000
// CHECK7-NEXT: 8212000: f9400021 ldr x1, [x1]
// CHECK7-NEXT: 8212004: 14000002 b 0x821200c
+// CHECK7: <__CortexA53843419_8212004_ret>:
// CHECK7-NEXT: 8212008: d65f03c0 ret
// CHECK7: <__CortexA53843419_8212004>:
// CHECK7-NEXT: 821200c: f9400000 ldr x0, [x0]
diff --git a/lld/test/ELF/aarch64-cortex-a53-843419-recognize.s b/lld/test/ELF/aarch64-cortex-a53-843419-recognize.s
index 44263777b0adf..9855f9b8c411a 100644
--- a/lld/test/ELF/aarch64-cortex-a53-843419-recognize.s
+++ b/lld/test/ELF/aarch64-cortex-a53-843419-recognize.s
@@ -33,6 +33,7 @@
// CHECK-NEXT: 211ff8: f0000260 adrp x0, 0x260000
// CHECK-NEXT: 211ffc: f9400021 ldr x1, [x1]
// CHECK-FIX: 212000: 1400c803 b 0x24400c
+// CHECK-FIX: <__CortexA53843419_212000_ret>:
// CHECK-NOFIX: 212000: f9400000 ldr x0, [x0]
// CHECK-NEXT: 212004: d65f03c0 ret
// CHECK-RELOCATABLE: <t3_ff8_ldr>:
@@ -57,6 +58,7 @@ t3_ff8_ldr:
// CHECK-NEXT: 213ff8: b0000260 adrp x0, 0x260000
// CHECK-NEXT: 213ffc: bd400021 ldr s1, [x1]
// CHECK-FIX: 214000: 1400c005 b 0x244014
+// CHECK-FIX: <__CortexA53843419_214000_ret>:
// CHECK-NOFIX: 214000: f9400402 ldr x2, [x0, #8]
// CHECK-NEXT: 214004: d65f03c0 ret
.section .text.02, "ax", %progbits
@@ -75,6 +77,7 @@ t3_ff8_ldrsimd:
// CHECK-NEXT: 215ffc: f0000240 adrp x0, 0x260000
// CHECK-NEXT: 216000: bc408421 ldr s1, [x1], #8
// CHECK-FIX: 216004: 1400b806 b 0x24401c
+// CHECK-FIX: <__CortexA53843419_216004_ret>:
// CHECK-NOFIX: 216004: f9400803 ldr x3, [x0, #16]
// CHECK-NEXT: 216008: d65f03c0 ret
.section .text.03, "ax", %progbits
@@ -93,6 +96,7 @@ t3_ffc_ldrpost:
// CHECK-NEXT: 217ff8: b0000240 adrp x0, 0x260000
// CHECK-NEXT: 217ffc: bc008c21 str s1, [x1, #8]!
// CHECK-FIX: 218000: 1400b009 b 0x244024
+// CHECK-FIX: <__CortexA53843419_218000_ret>:
// CHECK-NOFIX: 218000: f9400c02 ldr x2, [x0, #24]
// CHECK-NEXT: 218004: d65f03c0 ret
.section .text.04, "ax", %progbits
@@ -111,6 +115,7 @@ t3_ff8_strpre:
// CHECK-NEXT: 219ffc: f000023c adrp x28, 0x260000
// CHECK-NEXT: 21a000: f9000042 str x2, [x2]
// CHECK-FIX: 21a004: 1400a80a b 0x24402c
+// CHECK-FIX: <__CortexA53843419_21A004_ret>:
// CHECK-NOFIX: 21a004: f900139c str x28, [x28, #32]
// CHECK-NEXT: 21a008: d65f03c0 ret
.section .text.05, "ax", %progbits
@@ -129,6 +134,7 @@ t3_ffc_str:
// CHECK-NEXT: 21bffc: b000023c adrp x28, 0x260000
// CHECK-NEXT: 21c000: b9000044 str w4, [x2]
// CHECK-FIX: 21c004: 1400a00c b 0x244034
+// CHECK-FIX: <__CortexA53843419_21C004_ret>:
// CHECK-NOFIX: 21c004: f9001784 str x4, [x28, #40]
// CHECK-NEXT: 21c008: d65f03c0 ret
.section .text.06, "ax", %progbits
@@ -147,6 +153,7 @@ t3_ffc_strsimd:
// CHECK-NEXT: 21dff8: f000021d adrp x29, 0x260000
// CHECK-NEXT: 21dffc: 38400841 ldtrb w1, [x2]
// CHECK-FIX: 21e000: 1400980f b 0x24403c
+// CHECK-FIX: <__CortexA53843419_21E000_ret>:
// CHECK-NOFIX: 21e000: f94003bd ldr x29, [x29]
// CHECK-NEXT: 21e004: d65f03c0 ret
.section .text.07, "ax", %progbits
@@ -165,6 +172,7 @@ t3_ff8_ldrunpriv:
// CHECK-NEXT: 21fffc: b000021d adrp x29, 0x260000
// CHECK-NEXT: 220000: b8404042 ldur w2, [x2, #4]
// CHECK-FIX: 220004: 14009010 b 0x244044
+// CHECK-FIX: <__CortexA53843419_220004_ret>:
// CHECK-NOFIX: 220004: f94007bd ldr x29, [x29, #8]
// CHECK-NEXT: 220008: d65f03c0 ret
.balign 4096
@@ -182,6 +190,7 @@ t3_ffc_ldur:
// CHECK-NEXT: 221ffc: f00001f2 adrp x18, 0x260000
// CHECK-NEXT: 222000: 78004043 sturh w3, [x2, #4]
// CHECK-FIX: 222004: 14008812 b 0x24404c
+// CHECK-FIX: <__CortexA53843419_222004_ret>:
// CHECK-NOFIX: 222004: f9400a41 ldr x1, [x18, #16]
// CHECK-NEXT: 222008: d65f03c0 ret
.section .text.09, "ax", %progbits
@@ -200,6 +209,7 @@ t3_ffc_sturh:
// CHECK-NEXT: 223ff8: b00001f2 adrp x18, 0x260000
// CHECK-NEXT: 223ffc: 58ffffe3 ldr x3, 0x223ff8
// CHECK-FIX: 224000: 14008015 b 0x244054
+// CHECK-FIX: <__CortexA53843419_224000_ret>:
// CHECK-NOFIX: 224000: f9400e52 ldr x18, [x18, #24]
// CHECK-NEXT: 224004: d65f03c0 ret
.section .text.10, "ax", %progbits
@@ -218,6 +228,7 @@ t3_ff8_literal:
// CHECK-NEXT: 225ffc: f00001cf adrp x15, 0x260000
// CHECK-NEXT: 226000: f8616843 ldr x3, [x2, x1]
// CHECK-FIX: 226004: 14007816 b 0x24405c
+// CHECK-FIX: <__CortexA53843419_226004_ret>:
// CHECK-NOFIX: 226004: f94011ea ldr x10, [x15, #32]
// CHECK-NEXT: 226008: d65f03c0 ret
.section .text.11, "ax", %progbits
@@ -236,6 +247,7 @@ t3_ffc_register:
// CHECK-NEXT: 227ff8: b00001d0 adrp x16, 0x260000
// CHECK-NEXT: 227ffc: a9000861 stp x1, x2, [x3]
// CHECK-FIX: 228000: 14007019 b 0x244064
+// CHECK-FIX: <__CortexA53843419_228000_ret>:
// CHECK-NOFIX: 228000: f940160d ldr x13, [x16, #40]
// CHECK-NEXT: 228004: d65f03c0 ret
.section .text.12, "ax", %progbits
@@ -254,6 +266,7 @@ t3_ff8_stp:
// CHECK-NEXT: 229ffc: f00001a7 adrp x7, 0x260000
// CHECK-NEXT: 22a000: a8000861 stnp x1, x2, [x3]
// CHECK-FIX: 22a004: 1400681a b 0x24406c
+// CHECK-FIX: <__CortexA53843419_22A004_ret>:
// CHECK-NOFIX: 22a004: f9400ce9 ldr x9, [x7, #24]
// CHECK-NEXT: 22a008: d65f03c0 ret
.section .text.13, "ax", %progbits
@@ -272,6 +285,7 @@ t3_ffc_stnp:
// CHECK-NEXT: 22bffc: b00001b7 adrp x23, 0x260000
// CHECK-NEXT: 22c000: 0d820420 st1 { v0.b }[1], [x1], x2
// CHECK-FIX: 22c004: 1400601c b 0x244074
+// CHECK-FIX: <__CortexA53843419_22C004_ret>:
// CHECK-NOFIX: 22c004: f94012f6 ldr x22, [x23, #32]
// CHECK-NEXT: 22c008: d65f03c0 ret
.section .text.14, "ax", %progbits
@@ -290,6 +304,7 @@ t3_ffc_st1singlepost:
// CHECK-NEXT: 22dff8: f0000197 adrp x23, 0x260000
// CHECK-NEXT: 22dffc: 4c00a020 st1 { v0.16b, v1.16b }, [x1]
// CHECK-FIX: 22e000: 1400581f b 0x24407c
+// CHECK-FIX: <__CortexA53843419_22E000_ret>:
// CHECK-NOFIX: 22e000: f94016f8 ldr x24, [x23, #40]
// CHECK-NEXT: 22e004: d65f03c0 ret
.section .text.15, "ax", %progbits
@@ -309,6 +324,7 @@ t3_ff8_st1multiple:
// CHECK-NEXT: 22fffc: f9400021 ldr x1, [x1]
// CHECK-NEXT: 230000: 8b000042 add x2, x2, x0
// CHECK-FIX: 230004: 14005020 b 0x244084
+// CHECK-FIX: <__CortexA53843419_230004_ret>:
// CHECK-NOFIX: 230004: f9400002 ldr x2, [x0]
// CHECK-NEXT: 230008: d65f03c0 ret
.section .text.16, "ax", %progbits
@@ -329,6 +345,7 @@ t4_ff8_ldr:
// CHECK-NEXT: 232000: f9000042 str x2, [x2]
// CHECK-NEXT: 232004: cb020020 sub x0, x1, x2
// CHECK-FIX: 232008: 14004821 b 0x24408c
+// CHECK-FIX: <__CortexA53843419_232008_ret>:
// CHECK-NOFIX: 232008: f900079b str x27, [x28, #8]
// CHECK-NEXT: 23200c: d65f03c0 ret
.section .text.17, "ax", %progbits
@@ -349,6 +366,7 @@ t4_ffc_str:
// CHECK-NEXT: 233ffc: a9000861 stp x1, x2, [x3]
// CHECK-NEXT: 234000: 9b107e03 mul x3, x16, x16
// CHECK-FIX: 234004: 14004024 b 0x244094
+// CHECK-FIX: <__CortexA53843419_234004_ret>:
// CHECK-NOFIX: 234004: f9400a0e ldr x14, [x16, #16]
// CHECK-NEXT: 234008: d65f03c0 ret
.section .text.18, "ax", %progbits
@@ -369,6 +387,7 @@ t4_ff8_stp:
// CHECK-NEXT: 235ffc: a9810861 stp x1, x2, [x3, #16]!
// CHECK-NEXT: 236000: 9b107e03 mul x3, x16, x16
// CHECK-FIX: 236004: 14003826 b 0x24409c
+// CHECK-FIX: <__CortexA53843419_236004_ret>:
// CHECK-NOFIX: 236004: f940060e ldr x14, [x16, #8]
// CHECK-NEXT: 236008: d65f03c0 ret
.section .text.19, "ax", %progbits
@@ -389,6 +408,7 @@ t4_ff8_stppre:
// CHECK-NEXT: 237ffc: a8810861 stp x1, x2, [x3], #16
// CHECK-NEXT: 238000: 9b107e03 mul x3, x16, x16
// CHECK-FIX: 238004: 14003028 b 0x2440a4
+// CHECK-FIX: <__CortexA53843419_238004_ret>:
// CHECK-NOFIX: 238004: f940060e ldr x14, [x16, #8]
// CHECK-NEXT: 238008: d65f03c0 ret
.section .text.20, "ax", %progbits
@@ -409,6 +429,7 @@ t4_ff8_stppost:
// CHECK-NEXT: 23a000: ad000861 stp q1, q2, [x3]
// CHECK-NEXT: 23a004: 9b107e03 mul x3, x16, x16
// CHECK-FIX: 23a008: 14002829 b 0x2440ac
+// CHECK-FIX: <__CortexA53843419_23A008_ret>:
// CHECK-NOFIX: 23a008: f940060e ldr x14, [x16, #8]
// CHECK-NEXT: 23a00c: d65f03c0 ret
.section .text.21, "ax", %progbits
@@ -429,6 +450,7 @@ t4_ffc_stpsimd:
// CHECK-NEXT: 23c000: a8000861 stnp x1, x2, [x3]
// CHECK-NEXT: 23c004: d503201f nop
// CHECK-FIX: 23c008: 1400202b b 0x2440b4
+// CHECK-FIX: <__CortexA53843419_23C008_ret>:
// CHECK-NOFIX: 23c008: f94000ea ldr x10, [x7]
// CHECK-NEXT: 23c00c: d65f03c0 ret
.section .text.22, "ax", %progbits
@@ -449,6 +471,7 @@ t4_ffc_stnp:
// CHECK-NEXT: 23e000: 4d008020 st1 { v0.s }[2], [x1]
// CHECK-NEXT: 23e004: f94006f6 ldr x22, [x23, #8]
// CHECK-FIX: 23e008: 1400182d b 0x2440bc
+// CHECK-FIX: <__CortexA53843419_23E008_ret>:
// CHECK-NOFIX: 23e008: f93fff18 str x24, [x24, #32760]
// CHECK-NEXT: 23e00c: d65f03c0 ret
.section .text.23, "ax", %progbits
@@ -468,6 +491,7 @@ t4_ffc_st1:
// CHECK-NEXT: 23fff8: b0000100 adrp x0, 0x260000
// CHECK-NEXT: 23fffc: 4c827020 st1 { v0.16b }, [x1], x2
// CHECK-FIX: 240000: 14001031 b 0x2440c4
+// CHECK-FIX: <__CortexA53843419_240000_ret>:
// CHECK-NOFIX: 240000: f9400801 ldr x1, [x0, #16]
// CHECK-NEXT: 240004: f9400802 ldr x2, [x0, #16]
// CHECK-NEXT: 240008: d65f03c0 ret
diff --git a/lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s b/lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
new file mode 100644
index 0000000000000..4e311dc414b0f
--- /dev/null
+++ b/lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
@@ -0,0 +1,68 @@
+// REQUIRES: aarch64
+// RUN: llvm-mc -mattr=+bti -filetype=obj -triple=aarch64 %s -o %t.o
+// RUN: echo "SECTIONS { .text 0x10000 : { *(.text.01); . += 0x8000000; *(.text.far); } }" > %t.script
+// RUN: ld.lld -z force-bti --script %t.script -fix-cortex-a53-843419 -verbose %t.o -o %t2 \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-PRINT %s
+// RUN: llvm-objdump --no-print-imm-hex --no-show-raw-insn --tripl...
[truncated]
|
Author
|
@smithp35 FYI |
smithp35
reviewed
Dec 3, 2025
Collaborator
smithp35
left a comment
smithp35
left a comment
There was a problem hiding this comment.
Approach looks good to me. BTI Thunks didn't exist when the code to round up Thunk Size to 4 KiB was written, and I should have thought of the errata patches when adding them.
Most of my comments are comment and test suggestions. The one suggestion that isn't a simple change is whether we can avoid adding an additional symbol for the return value of the patch. The clang integrated assembler will convert a branch to local symbol to use section symbol + addend, we should be able to do the same.
lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
Outdated
Show resolved
Hide resolved
lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
Outdated
Show resolved
Hide resolved
lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
Outdated
Show resolved
Hide resolved
lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
Outdated
Show resolved
Hide resolved
lld/test/ELF/aarch64-cortex-a53-843419-thunk-relocation-crash.s
Outdated
Show resolved
Hide resolved
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,cpp -- lld/ELF/AArch64ErrataFix.cpp lld/ELF/Arch/AArch64.cpp lld/ELF/Relocations.cpp lld/ELF/Relocations.h --diff_from_common_commit
View the diff from clang-format here.diff --git a/lld/ELF/AArch64ErrataFix.cpp b/lld/ELF/AArch64ErrataFix.cpp
index a9f6c43f8..10bf4b461 100644
--- a/lld/ELF/AArch64ErrataFix.cpp
+++ b/lld/ELF/AArch64ErrataFix.cpp
@@ -402,7 +402,8 @@ Patch843419Section::Patch843419Section(Ctx &ctx, InputSection *p, uint64_t off)
STT_FUNC, 0, getSize(), *this);
addSyntheticLocal(ctx, ctx.saver.save("$x"), STT_NOTYPE, 0, 0, *this);
retSym = addSyntheticLocal(
- ctx, ctx.saver.save("__CortexA53843419_" + utohexstr(getLDSTAddr()) + "_ret"),
+ ctx,
+ ctx.saver.save("__CortexA53843419_" + utohexstr(getLDSTAddr()) + "_ret"),
STT_FUNC, off + 4, 4, *p);
// Relocation must be created as soon as possible, so it'll be picked up.
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 1a927bc32..bdfaf4b97 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -48,7 +48,8 @@ bool elf::isAArch64BTILandingPad(Ctx &ctx, Symbol &s, int64_t a) {
if (off >= isec->getSize())
return true;
const uint8_t *buf = isec->content().begin();
- // Synthetic sections may have a size but empty data - Assume that they won't contain a landing pad
+ // Synthetic sections may have a size but empty data - Assume that they won't
+ // contain a landing pad
if (buf == nullptr && dyn_cast<SyntheticSection>(isec) != nullptr) {
return false;
}
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index afd2af2a0..612464ec2 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1998,8 +1998,7 @@ void ThunkCreator::createInitialThunkSections(
ThunkSection *ThunkCreator::addThunkSection(OutputSection *os,
InputSectionDescription *isd,
- uint64_t off,
- bool isPrefix) {
+ uint64_t off, bool isPrefix) {
auto *ts = make<ThunkSection>(ctx, os, off);
ts->partition = os->partition;
if ((ctx.arg.fixCortexA53Errata843419 || ctx.arg.fixCortexA8) &&
|
smithp35
reviewed
Dec 5, 2025
Collaborator
smithp35
left a comment
smithp35
left a comment
There was a problem hiding this comment.
Thanks for the updates. It is looking close.
I think we would be best to avoid using mapping symbols as relocation targets as that is not permitted in the AArch64 ABI, and this can be exposed to other tools with -emit-relocs. I've made a suggestion to use the Section Symbol which could be used instead of mapping symbols. Should not need a lot more code.
MaskRay
approved these changes
Dec 10, 2025
Member
|
Suggest
Add that how the crash occurred. |
added 4 commits
December 10, 2025 17:18
Original crash was observed in Chromium, in [1]. The problem occurs in elf::isAArch64BTILandingPad because it didn't handle synthetic sections, which can have a nullptr as a buf, so it crashed while trying to read that buf. After fixing that, a second issue occurs: When the patched code grows too much, it gets far away from the short jump, and the current implementation assumes a R_AARCH64_JUMP26 will be enough. This PR changes the implementation to: (a) In isAArch64BTILandingPad, checks if a section is synthetic, and assumes that it'll NOT contain a landing pad, avoiding the buffer check; (b) Suppress the size rounding for thunks that preceeds section (Making the situation less likely to happen); (c) Reimplements the patch by using a R_AARCH64_ABS64 in case the patched code is still far away. [1] https://issues.chromium.org/issues/440019454
tarcisiofischer
force-pushed
the
fix-bti-crash
branch
from
9800992 to
9cec7f4
Compare
tarcisiofischer
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Original crash was observed in Chromium, in [1]. The problem occurs in
elf::isAArch64BTILandingPad because it didn't handle synthetic sections,
which can have a nullptr as a buf, so it crashed while trying to read
that buf.
After fixing that, a second issue occurs: When the patched code grows too
much, it gets far away from the short jump, and the current implementation
assumes a R_AARCH64_JUMP26 will be enough.
This PR changes the implementation to:
(a) In isAArch64BTILandingPad, checks if a section is synthetic, and
assumes that it'll NOT contain a landing pad, avoiding the buffer check;
(b) Suppress the size rounding for thunks that preceeds section
(Making the situation less likely to happen);
(c) Reimplements the patch by using a R_AARCH64_ABS64 in case the
patched code is still far away.
[1] https://issues.chromium.org/issues/440019454