[llvm][AArch64] Avoid iterating off the beginning of a BB in a backward scan - #217752
Merged
Conversation
…rd scan ... using a reversed range, rather than raw iterator comparisons, which got us in trouble previously in a block that was entirely made up of FrameDestroy instructions. rdar://185425744
jroelofs
enabled auto-merge (squash)
August 20, 2026 20:30
|
@llvm/pr-subscribers-backend-aarch64 Author: Jon Roelofs (jroelofs) Changes... using a reversed range, rather than raw iterator comparisons, which got us in trouble previously in a block that was entirely made up of FrameDestroy instructions. rdar://185425744 Full diff: https://github.com/llvm/llvm-project/pull/217752.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
index f7eb3a9982faf..374f42a02cc08 100644
--- a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
+++ b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
@@ -327,15 +327,17 @@ void AArch64PointerAuthImpl::authenticateLR(
int64_t Offset = -ArgumentStackToRestore;
SmallVector<MachineInstr *, 2> SPMods;
if (ArgumentStackToRestore > 0) {
- for (auto I = MBBI; I->getFlag(MachineInstr::FrameDestroy); --I) {
- if ((I->getOpcode() == AArch64::ADDXri ||
- I->getOpcode() == AArch64::SUBXri) &&
- I->getOperand(0).getReg() == AArch64::SP &&
- I->getOperand(1).getReg() == AArch64::SP) {
- SPMods.push_back(&*I);
- int64_t Imm = I->getOperand(2).getImm()
- << AArch64_AM::getShiftValue(I->getOperand(3).getImm());
- Offset += I->getOpcode() == AArch64::ADDXri ? Imm : -Imm;
+ for (MachineInstr &MI : reverse(make_range(MBB.begin(), MBBI))) {
+ if (!MI.getFlag(MachineInstr::FrameDestroy))
+ break;
+ if ((MI.getOpcode() == AArch64::ADDXri ||
+ MI.getOpcode() == AArch64::SUBXri) &&
+ MI.getOperand(0).getReg() == AArch64::SP &&
+ MI.getOperand(1).getReg() == AArch64::SP) {
+ SPMods.push_back(&MI);
+ int64_t Imm = MI.getOperand(2).getImm()
+ << AArch64_AM::getShiftValue(MI.getOperand(3).getImm());
+ Offset += MI.getOpcode() == AArch64::ADDXri ? Imm : -Imm;
}
}
}
diff --git a/llvm/test/CodeGen/AArch64/swifttail-ptrauth.ll b/llvm/test/CodeGen/AArch64/swifttail-ptrauth.ll
index 50d7b2dddc68e..32627a62e7972 100644
--- a/llvm/test/CodeGen/AArch64/swifttail-ptrauth.ll
+++ b/llvm/test/CodeGen/AArch64/swifttail-ptrauth.ll
@@ -204,3 +204,111 @@ define swifttailcc void @caller_to8_from0() "branch-protection-pauth-lr" "sign-r
tail call swifttailcc void @callee_stack8([8 x i64] poison, i64 42)
ret void
}
+
+declare void @sink(ptr)
+
+define swifttailcc void @crash_tc(i1 %c, [8 x i64] %pad, i64 %x) "branch-protection-pauth-lr" "sign-return-address"="all" "frame-pointer"="all" uwtable(async) {
+; CHECK-LABEL: crash_tc:
+; CHECK-NEXT: .cfi_startproc
+; CHECK-NEXT: // %bb.0:
+
+; COMPAT-NEXT: hint #39
+; COMPAT-NEXT: .Ltmp3:
+; COMPAT-NEXT: hint #25
+; COMPAT-NEXT: .cfi_set_ra_state 2, .Ltmp3
+
+; V83A-NEXT: hint #39
+; V83A-NEXT: .Ltmp3:
+; V83A-NEXT: paciasp
+; V83A-NEXT: .cfi_set_ra_state 2, .Ltmp3
+
+; V9A-NEXT: .Ltmp3:
+; V9A-NEXT: paciasppc
+; V9A-NEXT: .cfi_set_ra_state 2, .Ltmp3
+
+; PAUTH-NEXT: paciasp
+; PAUTH-NEXT: .cfi_negate_ra_state
+
+; CHECK-NEXT: stp x29, x30, [sp, #-16]!
+; CHECK-NEXT: .cfi_def_cfa_offset 16
+; CHECK-NEXT: mov x29, sp
+; CHECK-NEXT: .cfi_def_cfa w29, 16
+; CHECK-NEXT: .cfi_offset w30, -8
+; CHECK-NEXT: .cfi_offset w29, -16
+; CHECK-NEXT: .cfi_remember_state
+; CHECK-NEXT: tbz w0, #0, .LBB3_2
+; CHECK-NEXT: // %bb.1:
+; CHECK-NEXT: sub x0, sp, #16
+; CHECK-NEXT: mov sp, x0
+; CHECK-NEXT: bl sink
+; CHECK-NEXT: mov sp, x29
+; CHECK-NEXT: .cfi_def_cfa wsp, 16
+; CHECK-NEXT: ldp x29, x30, [sp], #16
+; CHECK-NEXT: .cfi_def_cfa_offset 0
+; CHECK-NEXT: .cfi_def_cfa_offset -80
+; CHECK-NEXT: .cfi_restore w30
+; CHECK-NEXT: .cfi_restore w29
+
+; COMPAT-NEXT: adrp x16, .Ltmp3
+; COMPAT-NEXT: add x16, x16, :lo12:.Ltmp3
+; COMPAT-NEXT: hint #39
+; COMPAT-NEXT: hint #29
+; COMPAT-NEXT: .cfi_set_ra_state 0, 0
+
+; V83A-NEXT: adrp x16, .Ltmp3
+; V83A-NEXT: add x16, x16, :lo12:.Ltmp3
+; V83A-NEXT: hint #39
+; V83A-NEXT: autiasp
+; V83A-NEXT: .cfi_set_ra_state 0, 0
+
+; V9A-NEXT: autiasppc .Ltmp3
+; V9A-NEXT: .cfi_set_ra_state 0, 0
+
+; PAUTH-NEXT: autiasp
+; PAUTH-NEXT: .cfi_negate_ra_state
+
+; CHECK-NEXT: add sp, sp, #80
+; CHECK-NEXT: ret
+; CHECK-NEXT: .LBB3_2:
+; CHECK-NEXT: .cfi_restore_state
+; CHECK-NEXT: mov sp, x29
+; CHECK-NEXT: .cfi_def_cfa wsp, 16
+; CHECK-NEXT: ldp x29, x30, [sp], #16
+; CHECK-NEXT: .cfi_def_cfa_offset 0
+; CHECK-NEXT: .cfi_def_cfa_offset -80
+; CHECK-NEXT: .cfi_restore w30
+; CHECK-NEXT: .cfi_restore w29
+
+; COMPAT-NEXT: adrp x16, .Ltmp3
+; COMPAT-NEXT: add x16, x16, :lo12:.Ltmp3
+; COMPAT-NEXT: hint #39
+; COMPAT-NEXT: hint #29
+; COMPAT-NEXT: .cfi_set_ra_state 0, 0
+
+; V83A-NEXT: adrp x16, .Ltmp3
+; V83A-NEXT: add x16, x16, :lo12:.Ltmp3
+; V83A-NEXT: hint #39
+; V83A-NEXT: autiasp
+; V83A-NEXT: .cfi_set_ra_state 0, 0
+
+; V9A-NEXT: autiasppc .Ltmp3
+; V9A-NEXT: .cfi_set_ra_state 0, 0
+
+; PAUTH-NEXT: autiasp
+; PAUTH-NEXT: .cfi_negate_ra_state
+
+; CHECK-NEXT: add sp, sp, #80
+; CHECK-NEXT: b callee_stack0
+
+entry:
+ br i1 %c, label %work, label %exit
+
+work:
+ %p = alloca i64, align 8
+ call void @sink(ptr %p)
+ ret void
+
+exit:
+ tail call swifttailcc void @callee_stack0()
+ ret void
+}
|
vhscampos
reviewed
Aug 21, 2026
| int64_t Imm = I->getOperand(2).getImm() | ||
| << AArch64_AM::getShiftValue(I->getOperand(3).getImm()); | ||
| Offset += I->getOpcode() == AArch64::ADDXri ? Imm : -Imm; | ||
| for (MachineInstr &MI : reverse(make_range(MBB.begin(), MBBI))) { |
Member
There was a problem hiding this comment.
This new for loop seems to skip MBBI. Is this intentional?
Contributor
Author
There was a problem hiding this comment.
I'll put up a PR with a fix shortly.
Contributor
Author
There was a problem hiding this comment.
Given MBBI always points at PAUTH_EPILOGUE, there's no functional difference, but it's worth making this less un-obvious.
jroelofs
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Aug 21, 2026
…rd scan (llvm#217752) ... using a reversed range, rather than raw iterator comparisons, which got us in trouble previously in a block that was entirely made up of FrameDestroy instructions. rdar://185425744 (cherry picked from commit ab0b549)
jroelofs
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Aug 21, 2026
…rd scan (llvm#217752) ... using a reversed range, rather than raw iterator comparisons, which got us in trouble previously in a block that was entirely made up of FrameDestroy instructions. rdar://185425744 (cherry picked from commit ab0b549)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
... using a reversed range, rather than raw iterator comparisons, which got us in trouble previously in a block that was entirely made up of FrameDestroy instructions.
rdar://185425744