-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
[AArch64][PAC] Fix creating check instructions for BBs without an epilog #92508
[AArch64][PAC] Fix creating check instructions for BBs without an epilog #92508
Conversation
The return value is not used. This change simplifies an upcoming patch.
`AArch64PAuth::checkAuthenticatedRegister()` splits the basic block containing the tail call instruction to add check instructions, assuming at least one more instruction before the call. This assumption is incorrect in cases where some execution paths lead to the termination block without creating the stack frame. This patch rearranges the creation of the checks so that the prior splitting is not required.
@llvm/pr-subscribers-backend-aarch64 Author: Igor Kudrin (igorkudrin) Changes
Full diff: https://github.com/llvm/llvm-project/pull/92508.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
index 90bf089dbebf7..60d3d533d9c10 100644
--- a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
+++ b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
@@ -257,21 +257,12 @@ void llvm::AArch64PAuth::checkAuthenticatedRegister(
// Control flow has to be changed, so arrange new MBBs.
- // At now, at least an AUT* instruction is expected before MBBI
- assert(MBBI != MBB.begin() &&
- "Cannot insert the check at the very beginning of MBB");
- // The block to insert check into.
- MachineBasicBlock *CheckBlock = &MBB;
- // The remaining part of the original MBB that is executed on success.
- MachineBasicBlock *SuccessBlock = MBB.splitAt(*std::prev(MBBI));
-
// The block that explicitly generates a break-point exception on failure.
MachineBasicBlock *BreakBlock =
MF.CreateMachineBasicBlock(MBB.getBasicBlock());
MF.push_back(BreakBlock);
- MBB.splitSuccessor(SuccessBlock, BreakBlock);
+ MBB.addSuccessor(BreakBlock);
- assert(CheckBlock->getFallThrough() == SuccessBlock);
BuildMI(BreakBlock, DL, TII->get(AArch64::BRK)).addImm(BrkImm);
switch (Method) {
@@ -279,11 +270,11 @@ void llvm::AArch64PAuth::checkAuthenticatedRegister(
case AuthCheckMethod::DummyLoad:
llvm_unreachable("Should be handled above");
case AuthCheckMethod::HighBitsNoTBI:
- BuildMI(CheckBlock, DL, TII->get(AArch64::EORXrs), TmpReg)
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::EORXrs), TmpReg)
.addReg(AuthenticatedReg)
.addReg(AuthenticatedReg)
.addImm(1);
- BuildMI(CheckBlock, DL, TII->get(AArch64::TBNZX))
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::TBNZX))
.addReg(TmpReg)
.addImm(62)
.addMBB(BreakBlock);
@@ -292,16 +283,16 @@ void llvm::AArch64PAuth::checkAuthenticatedRegister(
assert(AuthenticatedReg == AArch64::LR &&
"XPACHint mode is only compatible with checking the LR register");
assert(UseIKey && "XPACHint mode is only compatible with I-keys");
- BuildMI(CheckBlock, DL, TII->get(AArch64::ORRXrs), TmpReg)
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::ORRXrs), TmpReg)
.addReg(AArch64::XZR)
.addReg(AArch64::LR)
.addImm(0);
- BuildMI(CheckBlock, DL, TII->get(AArch64::XPACLRI));
- BuildMI(CheckBlock, DL, TII->get(AArch64::SUBSXrs), AArch64::XZR)
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::XPACLRI));
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::SUBSXrs), AArch64::XZR)
.addReg(TmpReg)
.addReg(AArch64::LR)
.addImm(0);
- BuildMI(CheckBlock, DL, TII->get(AArch64::Bcc))
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::Bcc))
.addImm(AArch64CC::NE)
.addMBB(BreakBlock);
return;
diff --git a/llvm/test/CodeGen/AArch64/sign-return-address-tailcall.ll b/llvm/test/CodeGen/AArch64/sign-return-address-tailcall.ll
index cf033cb8208cc..0cc707298e458 100644
--- a/llvm/test/CodeGen/AArch64/sign-return-address-tailcall.ll
+++ b/llvm/test/CodeGen/AArch64/sign-return-address-tailcall.ll
@@ -129,4 +129,36 @@ define i32 @tailcall_ib_key() "sign-return-address"="all" "sign-return-address-k
ret i32 %call
}
+define i32 @tailcall_two_branches(i1 %0) "sign-return-address"="all" {
+; COMMON-LABEL: tailcall_two_branches:
+; COMMON: tbz w0, #0, .[[ELSE:LBB[_0-9]+]]
+; COMMON: str x30, [sp, #-16]!
+; COMMON: bl callee2
+; COMMON: ldr x30, [sp], #16
+; COMMON-NEXT: [[AUTIASP]]
+; COMMON-NEXT: .[[ELSE]]:
+
+; LDR-NEXT: ldr w16, [x30]
+;
+; BITS-NOTBI-NEXT: eor x16, x30, x30, lsl #1
+; BITS-NOTBI-NEXT: tbnz x16, #62, .[[FAIL:LBB[_0-9]+]]
+;
+; XPAC-NEXT: mov x16, x30
+; XPAC-NEXT: [[XPACLRI]]
+; XPAC-NEXT: cmp x16, x30
+; XPAC-NEXT: b.ne .[[FAIL:LBB[_0-9]+]]
+;
+; COMMON-NEXT: b callee
+; BRK-NEXT: .[[FAIL]]:
+; BRK-NEXT: brk #0xc470
+ br i1 %0, label %2, label %3
+2:
+ call void @callee2()
+ br label %3
+3:
+ %call = tail call i32 @callee()
+ ret i32 %call
+}
+
declare i32 @callee()
+declare void @callee2()
|
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.
Thank you for simplification, LGTM!
…log (llvm#92508) `AArch64PAuth::checkAuthenticatedRegister()` splits the basic block containing the tail call instruction to add check instructions, assuming at least one more instruction before the call. This assumption is incorrect in cases where some execution paths lead to the termination block without creating the stack frame. This patch rearranges the creation of the checks so that the prior splitting is not required.
AArch64PAuth::checkAuthenticatedRegister()
splits the basic block containing the tail call instruction to add check instructions, assuming at least one more instruction before the call. This assumption is incorrect in cases where some execution paths lead to the termination block without creating the stack frame. This patch rearranges the creation of the checks so that the prior splitting is not required.