Skip to content

Commit a5b7c36

Browse files
authored
[AArch64][PAC] Fix creating check instructions for BBs without an epilog (#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.
1 parent f114edd commit a5b7c36

File tree

2 files changed

+47
-16
lines changed

2 files changed

+47
-16
lines changed

llvm/lib/Target/AArch64/AArch64PointerAuth.cpp

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,14 @@ void llvm::AArch64PAuth::checkAuthenticatedRegister(
241241
const AArch64InstrInfo *TII = Subtarget.getInstrInfo();
242242
DebugLoc DL = MBBI->getDebugLoc();
243243

244+
// All terminator instructions should be grouped at the end of the machine
245+
// basic block, with no non-terminator instructions between them. Depending on
246+
// the method requested, we will insert some regular instructions, maybe
247+
// followed by a conditional branch instruction, which is a terminator, before
248+
// MBBI. Thus, MBBI is expected to be the first terminator of its MBB.
249+
assert(MBBI->isTerminator() && MBBI == MBB.getFirstTerminator() &&
250+
"MBBI should be the first terminator in MBB");
251+
244252
// First, handle the methods not requiring creating extra MBBs.
245253
switch (Method) {
246254
default:
@@ -257,33 +265,24 @@ void llvm::AArch64PAuth::checkAuthenticatedRegister(
257265

258266
// Control flow has to be changed, so arrange new MBBs.
259267

260-
// At now, at least an AUT* instruction is expected before MBBI
261-
assert(MBBI != MBB.begin() &&
262-
"Cannot insert the check at the very beginning of MBB");
263-
// The block to insert check into.
264-
MachineBasicBlock *CheckBlock = &MBB;
265-
// The remaining part of the original MBB that is executed on success.
266-
MachineBasicBlock *SuccessBlock = MBB.splitAt(*std::prev(MBBI));
267-
268268
// The block that explicitly generates a break-point exception on failure.
269269
MachineBasicBlock *BreakBlock =
270270
MF.CreateMachineBasicBlock(MBB.getBasicBlock());
271271
MF.push_back(BreakBlock);
272-
MBB.splitSuccessor(SuccessBlock, BreakBlock);
272+
MBB.addSuccessor(BreakBlock);
273273

274-
assert(CheckBlock->getFallThrough() == SuccessBlock);
275274
BuildMI(BreakBlock, DL, TII->get(AArch64::BRK)).addImm(BrkImm);
276275

277276
switch (Method) {
278277
case AuthCheckMethod::None:
279278
case AuthCheckMethod::DummyLoad:
280279
llvm_unreachable("Should be handled above");
281280
case AuthCheckMethod::HighBitsNoTBI:
282-
BuildMI(CheckBlock, DL, TII->get(AArch64::EORXrs), TmpReg)
281+
BuildMI(MBB, MBBI, DL, TII->get(AArch64::EORXrs), TmpReg)
283282
.addReg(AuthenticatedReg)
284283
.addReg(AuthenticatedReg)
285284
.addImm(1);
286-
BuildMI(CheckBlock, DL, TII->get(AArch64::TBNZX))
285+
BuildMI(MBB, MBBI, DL, TII->get(AArch64::TBNZX))
287286
.addReg(TmpReg)
288287
.addImm(62)
289288
.addMBB(BreakBlock);
@@ -292,16 +291,16 @@ void llvm::AArch64PAuth::checkAuthenticatedRegister(
292291
assert(AuthenticatedReg == AArch64::LR &&
293292
"XPACHint mode is only compatible with checking the LR register");
294293
assert(UseIKey && "XPACHint mode is only compatible with I-keys");
295-
BuildMI(CheckBlock, DL, TII->get(AArch64::ORRXrs), TmpReg)
294+
BuildMI(MBB, MBBI, DL, TII->get(AArch64::ORRXrs), TmpReg)
296295
.addReg(AArch64::XZR)
297296
.addReg(AArch64::LR)
298297
.addImm(0);
299-
BuildMI(CheckBlock, DL, TII->get(AArch64::XPACLRI));
300-
BuildMI(CheckBlock, DL, TII->get(AArch64::SUBSXrs), AArch64::XZR)
298+
BuildMI(MBB, MBBI, DL, TII->get(AArch64::XPACLRI));
299+
BuildMI(MBB, MBBI, DL, TII->get(AArch64::SUBSXrs), AArch64::XZR)
301300
.addReg(TmpReg)
302301
.addReg(AArch64::LR)
303302
.addImm(0);
304-
BuildMI(CheckBlock, DL, TII->get(AArch64::Bcc))
303+
BuildMI(MBB, MBBI, DL, TII->get(AArch64::Bcc))
305304
.addImm(AArch64CC::NE)
306305
.addMBB(BreakBlock);
307306
return;

llvm/test/CodeGen/AArch64/sign-return-address-tailcall.ll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,36 @@ define i32 @tailcall_ib_key() "sign-return-address"="all" "sign-return-address-k
129129
ret i32 %call
130130
}
131131

132+
define i32 @tailcall_two_branches(i1 %0) "sign-return-address"="all" {
133+
; COMMON-LABEL: tailcall_two_branches:
134+
; COMMON: tbz w0, #0, .[[ELSE:LBB[_0-9]+]]
135+
; COMMON: str x30, [sp, #-16]!
136+
; COMMON: bl callee2
137+
; COMMON: ldr x30, [sp], #16
138+
; COMMON-NEXT: [[AUTIASP]]
139+
; COMMON-NEXT: .[[ELSE]]:
140+
141+
; LDR-NEXT: ldr w16, [x30]
142+
;
143+
; BITS-NOTBI-NEXT: eor x16, x30, x30, lsl #1
144+
; BITS-NOTBI-NEXT: tbnz x16, #62, .[[FAIL:LBB[_0-9]+]]
145+
;
146+
; XPAC-NEXT: mov x16, x30
147+
; XPAC-NEXT: [[XPACLRI]]
148+
; XPAC-NEXT: cmp x16, x30
149+
; XPAC-NEXT: b.ne .[[FAIL:LBB[_0-9]+]]
150+
;
151+
; COMMON-NEXT: b callee
152+
; BRK-NEXT: .[[FAIL]]:
153+
; BRK-NEXT: brk #0xc470
154+
br i1 %0, label %2, label %3
155+
2:
156+
call void @callee2()
157+
br label %3
158+
3:
159+
%call = tail call i32 @callee()
160+
ret i32 %call
161+
}
162+
132163
declare i32 @callee()
164+
declare void @callee2()

0 commit comments

Comments
 (0)