-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[BOLT][AArch64] Fix crash for conditional tail calls #140669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When conditional tail call is located in old code while BOLT is operating in lite mode, the call will require optional pending relocation with a type that is currently not supported resulting in a build-time crash. Before a proper fix is implemented, ignore conditional tail calls for relocation purposes and mark their target functions to be patched, i.e. to be served as veneers/thunks.
@llvm/pr-subscribers-bolt Author: Maksim Panchenko (maksfb) ChangesWhen conditional tail call is located in old code while BOLT is operating in lite mode, the call will require optional pending relocation with a type that is currently not supported resulting in a build-time crash. Before a proper fix is implemented, ignore conditional tail calls for relocation purposes and mark their target functions to be patched, i.e. to be served as veneers/thunks. Full diff: https://github.com/llvm/llvm-project/pull/140669.diff 2 Files Affected:
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 851fa36a6b4b7..1f9f023a10811 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1783,10 +1783,22 @@ bool BinaryFunction::scanExternalRefs() {
// On AArch64, we use instruction patches for fixing references. We make an
// exception for branch instructions since they require optional
// relocations.
- if (BC.isAArch64() && !BranchTargetSymbol) {
- LLVM_DEBUG(BC.printInstruction(dbgs(), Instruction, AbsoluteInstrAddr));
- InstructionPatches.push_back({AbsoluteInstrAddr, Instruction});
- continue;
+ if (BC.isAArch64()) {
+ if (!BranchTargetSymbol) {
+ LLVM_DEBUG(BC.printInstruction(dbgs(), Instruction, AbsoluteInstrAddr));
+ InstructionPatches.push_back({AbsoluteInstrAddr, Instruction});
+ continue;
+ }
+
+ // Conditional tail calls require new relocation types that are currently
+ // not supported. https://github.com/llvm/llvm-project/issues/138264
+ if (BC.MIB->isConditionalBranch(Instruction)) {
+ if (BinaryFunction *TargetBF =
+ BC.getFunctionForSymbol(BranchTargetSymbol)) {
+ TargetBF->setNeedsPatch(true);
+ continue;
+ }
+ }
}
// Emit the instruction using temp emitter and generate relocations.
diff --git a/bolt/test/AArch64/lite-mode.s b/bolt/test/AArch64/lite-mode.s
index d1e35ef75de46..f2d06219f7a2d 100644
--- a/bolt/test/AArch64/lite-mode.s
+++ b/bolt/test/AArch64/lite-mode.s
@@ -129,6 +129,15 @@ cold_function:
# CHECK-INPUT-NEXT: b {{.*}} <_start>
# CHECK-NEXT: b {{.*}} <_start.org.0>
+## Quick test for conditional tail calls. A proper test is being added in:
+## https://github.com/llvm/llvm-project/pull/139565
+## For now check that llvm-bolt doesn't choke on CTCs.
+.ifndef COMPACT
+ b.eq _start
+ cbz x0, _start
+ tbz x0, 42, _start
+.endif
+
.cfi_endproc
.size cold_function, .-cold_function
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
When conditional tail call is located in old code while BOLT is operating in lite mode, the call will require optional pending relocation with a type that is currently not supported resulting in a build-time crash. Before a proper fix is implemented, ignore conditional tail calls for relocation purposes and mark their target functions to be patched, i.e. to be served as veneers/thunks.
When conditional tail call is located in old code while BOLT is operating in lite mode, the call will require optional pending relocation with a type that is currently not supported resulting in a build-time crash.
Before a proper fix is implemented, ignore conditional tail calls for relocation purposes and mark their target functions to be patched, i.e. to be served as veneers/thunks.