[llvm] R11 not preserved with PAC-M and AAPCS frame chain defect fix (PR #81249)
James Westwood via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 9 06:18:20 PST 2024
https://github.com/jwestwood921 updated https://github.com/llvm/llvm-project/pull/81249
>From bae9fcda4b064f7e2137b9e0a1045e691c44558e Mon Sep 17 00:00:00 2001
From: James Westwood <james.westwood at arm.com>
Date: Fri, 9 Feb 2024 12:09:03 +0000
Subject: [PATCH 1/3] R11 not preserved with PAC-M and AAPCS frame chain defect
fix
When PAC-RET and AAPCS were enabled, R12 was pushed to the stack
between R11 (the frame pointer) and the link register, which violated
the principle that the frame pointer and link register must always be
adjacent on the stack. This change separates R12 into a different push
instruction to R11 and the link register when R11 is the frame pointer,
meaning the frame pointer and link register are adjacent on the stack.
---
llvm/lib/Target/ARM/ARMBaseRegisterInfo.h | 13 ++-
llvm/lib/Target/ARM/ARMSubtarget.cpp | 21 +++++
llvm/lib/Target/ARM/ARMSubtarget.h | 2 +-
.../CodeGen/Thumb2/pacbti-m-frame-chain.ll | 82 +++++++++++++++++++
4 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 llvm/test/CodeGen/Thumb2/pacbti-m-frame-chain.ll
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
index 926d702b4092a..95c108ed9d437 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
@@ -78,9 +78,12 @@ static inline bool isSplitFPArea1Register(unsigned Reg,
switch (Reg) {
case R0: case R1: case R2: case R3:
case R4: case R5: case R6: case R7:
- case R8: case R9: case R10: case R12:
- case SP: case PC:
+ case R8: case R9: case SP: case PC:
return true;
+ case R10: case R12:
+ return !SplitFramePushPop;
+ case LR:
+ return SplitFramePushPop;
default:
return false;
}
@@ -91,8 +94,12 @@ static inline bool isSplitFPArea2Register(unsigned Reg,
using namespace ARM;
switch (Reg) {
- case R11: case LR:
+ case R10: case R12:
+ return SplitFramePushPop;
+ case R11:
return true;
+ case LR:
+ return !SplitFramePushPop;
default:
return false;
}
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 922fa93226f29..3a28a2cc04c6c 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -496,6 +496,27 @@ bool ARMSubtarget::ignoreCSRForAllocationOrder(const MachineFunction &MF,
bool ARMSubtarget::splitFramePointerPush(const MachineFunction &MF) const {
const Function &F = MF.getFunction();
+ const std::vector<CalleeSavedInfo> CSI = MF.getFrameInfo().getCalleeSavedInfo();
+
+ if (CSI.size() > 1 && MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress()) {
+ bool r11InCSI = false;
+ bool lrInCSI = false;
+ unsigned long r11Idx = 0;
+ unsigned long lrIdx = 0;
+ for (unsigned long i = 0; i < CSI.size(); i++) {
+ if (CSI[i].getReg() == ARM::LR) {
+ lrIdx = i;
+ lrInCSI = true;
+ }
+ else if (CSI[i].getReg() == ARM::R11) {
+ r11Idx = i;
+ r11InCSI = true;
+ }
+ }
+ if (lrIdx +1 != r11Idx && r11InCSI && lrInCSI)
+ return true;
+ }
+
if (!MF.getTarget().getMCAsmInfo()->usesWindowsCFI() ||
!F.needsUnwindTableEntry())
return false;
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.h b/llvm/lib/Target/ARM/ARMSubtarget.h
index 91f3978b041a3..19dac4ffcb3b2 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.h
+++ b/llvm/lib/Target/ARM/ARMSubtarget.h
@@ -445,7 +445,7 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
/// to lr. This is always required on Thumb1-only targets, as the push and
/// pop instructions can't access the high registers.
bool splitFramePushPop(const MachineFunction &MF) const {
- if (MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress())
+ if (MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress() && !createAAPCSFrameChain())
return true;
return (getFramePointerReg() == ARM::R7 &&
MF.getTarget().Options.DisableFramePointerElim(MF)) ||
diff --git a/llvm/test/CodeGen/Thumb2/pacbti-m-frame-chain.ll b/llvm/test/CodeGen/Thumb2/pacbti-m-frame-chain.ll
new file mode 100644
index 0000000000000..638bb86777dfc
--- /dev/null
+++ b/llvm/test/CodeGen/Thumb2/pacbti-m-frame-chain.ll
@@ -0,0 +1,82 @@
+; RUN: llc -filetype asm -o - %s --frame-pointer=all -mattr=+aapcs-frame-chain -mattr=+aapcs-frame-chain-leaf -force-dwarf-frame-section | FileCheck %s
+target triple = "thumbv8m.main-none-none-eabi"
+
+; int f() {
+; return 0;
+; }
+;
+; int x(int, char *);
+; int y(int n) {
+; char a[n];
+; return 1 + x(n, a);
+; }
+
+define hidden i32 @f() local_unnamed_addr {
+entry:
+ ret i32 0;
+}
+
+define hidden i32 @x(i32 noundef %n) local_unnamed_addr {
+entry:
+ %vla = alloca i8, i32 %n, align 1
+ %call = call i32 @y(i32 noundef %n, ptr noundef nonnull %vla)
+ %add = add nsw i32 %call, 1
+ ret i32 %add
+}
+
+declare dso_local i32 @y(i32 noundef, ptr noundef) local_unnamed_addr
+
+; CHECK-LABEL: f:
+; CHECK: pac r12, lr, sp
+; CHECK-NEXT: .save {ra_auth_code}
+; CHECK-NEXT: str r12, [sp, #-4]!
+; CHECK-NEXT: .cfi_def_cfa_offset 4
+; CHECK-NEXT: .cfi_offset lr, -4
+; CHECK-NEXT: .cfi_offset r12, -8
+; CHECK-NEXT: .cfi_offset r11, -12
+; CHECK-NEXT: .save {r11, lr}
+; CHECK-NEXT: push.w {r11, lr}
+; CHECK-NEXT: .setfp r11, sp
+; CHECK-NEXT: mov r11, sp
+; CHECK-NEXT: .cfi_def_cfa r11, 12
+; CHECK-NEXT: .pad #8
+; CHECK-NEXT: sub sp, #8
+; CHECK-NEXT: movs r0, #0
+; CHECK-NEXT: pop.w {r11, lr}
+; CHECK-NEXT: ldr r12, [sp], #4
+; CHECK-NEXT: aut r12, lr, sp
+; CHECK-NEXT: bx lr
+
+; CHECK-LABEL: x:
+; CHECK: pac r12, lr, sp
+; CHECK-NEXT: .save {r4, r7, ra_auth_code}
+; CHECK-NEXT: push.w {r4, r7, r12}
+; CHECK-NEXT: .cfi_def_cfa_offset 12
+; CHECK-NEXT: .cfi_offset lr, -4
+; CHECK-NEXT: .cfi_offset r12, -8
+; CHECK-NEXT: .cfi_offset r11, -12
+; CHECK-NEXT: .cfi_offset r7, -16
+; CHECK-NEXT: .cfi_offset r4, -20
+; CHECK-NEXT: .save {r11, lr}
+; CHECK-NEXT: push.w {r11, lr}
+; CHECK-NEXT: .setfp r11, sp
+; CHECK-NEXT: mov r11, sp
+; CHECK-NEXT: .cfi_def_cfa_register r11
+; CHECK-NEXT: .pad #12
+; CHECK-NEXT: sub sp, #12
+; CHECK-NEXT: adds r1, r0, #7
+; CHECK-NEXT: bic r1, r1, #7
+; CHECK-NEXT: sub.w r1, sp, r1
+; CHECK-NEXT: mov sp, r1
+; CHECK-NEXT: bl y
+; CHECK-NEXT: sub.w r4, r11, #8
+; CHECK-NEXT: adds r0, #1
+; CHECK-NEXT: mov sp, r4
+; CHECK-NEXT: pop.w {r11, lr}
+; CHECK-NEXT: pop.w {r4, r7, r12}
+; CHECK-NEXT: aut r12, lr, sp
+; CHECK-NEXT: bx lr
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 8, !"sign-return-address", i32 1}
>From 0fa42a9cf8c486f0cbd82043db206511a6195654 Mon Sep 17 00:00:00 2001
From: James Westwood <james.westwood at arm.com>
Date: Fri, 9 Feb 2024 12:09:03 +0000
Subject: [PATCH 2/3] R11 not preserved with PAC-M and AAPCS frame chain defect
fix
When PAC-RET and AAPCS were enabled, R12 was pushed to the stack
between R11 (the frame pointer) and the link register, which violated
the principle that the frame pointer and link register must always be
adjacent on the stack. This change separates R12 into a different push
instruction to R11 and the link register when R11 is the frame pointer,
meaning the frame pointer and link register are adjacent on the stack.
---
llvm/lib/Target/ARM/ARMBaseRegisterInfo.h | 25 ++++++++++++++---------
llvm/lib/Target/ARM/ARMSubtarget.cpp | 11 +++++-----
llvm/lib/Target/ARM/ARMSubtarget.h | 3 ++-
3 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
index 95c108ed9d437..bf71b04e44f07 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
@@ -78,9 +78,13 @@ static inline bool isSplitFPArea1Register(unsigned Reg,
switch (Reg) {
case R0: case R1: case R2: case R3:
case R4: case R5: case R6: case R7:
- case R8: case R9: case SP: case PC:
+ case R8:
+ case R9:
+ case SP:
+ case PC:
return true;
- case R10: case R12:
+ case R10:
+ case R12:
return !SplitFramePushPop;
case LR:
return SplitFramePushPop;
@@ -94,14 +98,15 @@ static inline bool isSplitFPArea2Register(unsigned Reg,
using namespace ARM;
switch (Reg) {
- case R10: case R12:
- return SplitFramePushPop;
- case R11:
- return true;
- case LR:
- return !SplitFramePushPop;
- default:
- return false;
+ case R10:
+ case R12:
+ return SplitFramePushPop;
+ case R11:
+ return true;
+ case LR:
+ return !SplitFramePushPop;
+ default:
+ return false;
}
}
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 3a28a2cc04c6c..39cb39a3659aa 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -496,9 +496,11 @@ bool ARMSubtarget::ignoreCSRForAllocationOrder(const MachineFunction &MF,
bool ARMSubtarget::splitFramePointerPush(const MachineFunction &MF) const {
const Function &F = MF.getFunction();
- const std::vector<CalleeSavedInfo> CSI = MF.getFrameInfo().getCalleeSavedInfo();
+ const std::vector<CalleeSavedInfo> CSI =
+ MF.getFrameInfo().getCalleeSavedInfo();
- if (CSI.size() > 1 && MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress()) {
+ if (CSI.size() > 1 &&
+ MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress()) {
bool r11InCSI = false;
bool lrInCSI = false;
unsigned long r11Idx = 0;
@@ -507,13 +509,12 @@ bool ARMSubtarget::splitFramePointerPush(const MachineFunction &MF) const {
if (CSI[i].getReg() == ARM::LR) {
lrIdx = i;
lrInCSI = true;
- }
- else if (CSI[i].getReg() == ARM::R11) {
+ } else if (CSI[i].getReg() == ARM::R11) {
r11Idx = i;
r11InCSI = true;
}
}
- if (lrIdx +1 != r11Idx && r11InCSI && lrInCSI)
+ if (lrIdx + 1 != r11Idx && r11InCSI && lrInCSI)
return true;
}
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.h b/llvm/lib/Target/ARM/ARMSubtarget.h
index 19dac4ffcb3b2..09a32a45dfb19 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.h
+++ b/llvm/lib/Target/ARM/ARMSubtarget.h
@@ -445,7 +445,8 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
/// to lr. This is always required on Thumb1-only targets, as the push and
/// pop instructions can't access the high registers.
bool splitFramePushPop(const MachineFunction &MF) const {
- if (MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress() && !createAAPCSFrameChain())
+ if (MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress() &&
+ !createAAPCSFrameChain())
return true;
return (getFramePointerReg() == ARM::R7 &&
MF.getTarget().Options.DisableFramePointerElim(MF)) ||
>From c897f3d311fb8bc142f519379749bf29210fd642 Mon Sep 17 00:00:00 2001
From: James Westwood <james.westwood at arm.com>
Date: Fri, 9 Feb 2024 12:09:03 +0000
Subject: [PATCH 3/3] R11 not preserved with PAC-M and AAPCS frame chain defect
fix
When PAC-RET and AAPCS were enabled, R12 was pushed to the stack
between R11 (the frame pointer) and the link register, which violated
the principle that the frame pointer and link register must always be
adjacent on the stack. This change separates R12 into a different push
instruction to R11 and the link register when R11 is the frame pointer,
meaning the frame pointer and link register are adjacent on the stack.
---
llvm/lib/Target/ARM/ARMBaseRegisterInfo.h | 25 ++++++++++++++---------
llvm/lib/Target/ARM/ARMSubtarget.cpp | 11 +++++-----
llvm/lib/Target/ARM/ARMSubtarget.h | 3 ++-
3 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
index 95c108ed9d437..71f23edaad1d4 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
@@ -78,9 +78,13 @@ static inline bool isSplitFPArea1Register(unsigned Reg,
switch (Reg) {
case R0: case R1: case R2: case R3:
case R4: case R5: case R6: case R7:
- case R8: case R9: case SP: case PC:
+ case R8:
+ case R9:
+ case SP:
+ case PC:
return true;
- case R10: case R12:
+ case R10:
+ case R12:
return !SplitFramePushPop;
case LR:
return SplitFramePushPop;
@@ -94,14 +98,15 @@ static inline bool isSplitFPArea2Register(unsigned Reg,
using namespace ARM;
switch (Reg) {
- case R10: case R12:
- return SplitFramePushPop;
- case R11:
- return true;
- case LR:
- return !SplitFramePushPop;
- default:
- return false;
+ case R10:
+ case R12:
+ return SplitFramePushPop;
+ case R11:
+ return true;
+ case LR:
+ return !SplitFramePushPop;
+ default:
+ return false;
}
}
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 3a28a2cc04c6c..39cb39a3659aa 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -496,9 +496,11 @@ bool ARMSubtarget::ignoreCSRForAllocationOrder(const MachineFunction &MF,
bool ARMSubtarget::splitFramePointerPush(const MachineFunction &MF) const {
const Function &F = MF.getFunction();
- const std::vector<CalleeSavedInfo> CSI = MF.getFrameInfo().getCalleeSavedInfo();
+ const std::vector<CalleeSavedInfo> CSI =
+ MF.getFrameInfo().getCalleeSavedInfo();
- if (CSI.size() > 1 && MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress()) {
+ if (CSI.size() > 1 &&
+ MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress()) {
bool r11InCSI = false;
bool lrInCSI = false;
unsigned long r11Idx = 0;
@@ -507,13 +509,12 @@ bool ARMSubtarget::splitFramePointerPush(const MachineFunction &MF) const {
if (CSI[i].getReg() == ARM::LR) {
lrIdx = i;
lrInCSI = true;
- }
- else if (CSI[i].getReg() == ARM::R11) {
+ } else if (CSI[i].getReg() == ARM::R11) {
r11Idx = i;
r11InCSI = true;
}
}
- if (lrIdx +1 != r11Idx && r11InCSI && lrInCSI)
+ if (lrIdx + 1 != r11Idx && r11InCSI && lrInCSI)
return true;
}
diff --git a/llvm/lib/Target/ARM/ARMSubtarget.h b/llvm/lib/Target/ARM/ARMSubtarget.h
index 19dac4ffcb3b2..09a32a45dfb19 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.h
+++ b/llvm/lib/Target/ARM/ARMSubtarget.h
@@ -445,7 +445,8 @@ class ARMSubtarget : public ARMGenSubtargetInfo {
/// to lr. This is always required on Thumb1-only targets, as the push and
/// pop instructions can't access the high registers.
bool splitFramePushPop(const MachineFunction &MF) const {
- if (MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress() && !createAAPCSFrameChain())
+ if (MF.getInfo<ARMFunctionInfo>()->shouldSignReturnAddress() &&
+ !createAAPCSFrameChain())
return true;
return (getFramePointerReg() == ARM::R7 &&
MF.getTarget().Options.DisableFramePointerElim(MF)) ||
More information about the llvm-commits
mailing list