[llvm] 81300f7 - [AArch64][PAC] Remove the duplication of LR sign/auth implementations
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 11 04:42:10 PDT 2023
Author: Anatoly Trosinenko
Date: 2023-08-11T14:39:18+03:00
New Revision: 81300f75f4da87caff36026b10d40c2391e246d0
URL: https://github.com/llvm/llvm-project/commit/81300f75f4da87caff36026b10d40c2391e246d0
DIFF: https://github.com/llvm/llvm-project/commit/81300f75f4da87caff36026b10d40c2391e246d0.diff
LOG: [AArch64][PAC] Remove the duplication of LR sign/auth implementations
In the machine outliner implementation for AArch64, `signOutlinedFunction()`
reimplements signing the LR value in prologue and authenticating it in
epilogue of the outlined function. This patch factors out `signLR()` and
`authenticateLR()` functions from AArch64FrameLowering code and reuses
them in `signOutlinedFunction()`.
The `mergeOutliningCandidateAttributes()` outliner callback is
introduced as well to further unify signing and authentication of the LR
value.
Reviewed By: tmatheson
Differential Revision: https://reviews.llvm.org/D157320
Added:
Modified:
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
llvm/lib/Target/AArch64/AArch64FrameLowering.h
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.h
llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
Removed:
################################################################################
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index c90fb820816656..8511e48c7462dd 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -1385,6 +1385,42 @@ static void emitDefineCFAWithFP(MachineFunction &MF, MachineBasicBlock &MBB,
.setMIFlags(MachineInstr::FrameSetup);
}
+void AArch64FrameLowering::signLR(MachineFunction &MF, MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI,
+ bool NeedsWinCFI, bool *HasWinCFI) {
+ const auto &MFnI = *MF.getInfo<AArch64FunctionInfo>();
+ const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
+ const TargetInstrInfo *TII = Subtarget.getInstrInfo();
+ bool EmitCFI = MFnI.needsDwarfUnwindInfo(MF);
+
+ // Debug location must be unknown, see emitPrologue().
+ DebugLoc DL;
+
+ if (MFnI.shouldSignWithBKey()) {
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::EMITBKEY))
+ .setMIFlag(MachineInstr::FrameSetup);
+ }
+
+ // No SEH opcode for this one; it doesn't materialize into an
+ // instruction on Windows.
+ BuildMI(
+ MBB, MBBI, DL,
+ TII->get(MFnI.shouldSignWithBKey() ? AArch64::PACIBSP : AArch64::PACIASP))
+ .setMIFlag(MachineInstr::FrameSetup);
+
+ if (EmitCFI) {
+ unsigned CFIIndex =
+ MF.addFrameInst(MCCFIInstruction::createNegateRAState(nullptr));
+ BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+ .addCFIIndex(CFIIndex)
+ .setMIFlags(MachineInstr::FrameSetup);
+ } else if (NeedsWinCFI) {
+ *HasWinCFI = true;
+ BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_PACSignLR))
+ .setMIFlag(MachineInstr::FrameSetup);
+ }
+}
+
void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
MachineBasicBlock &MBB) const {
MachineBasicBlock::iterator MBBI = MBB.begin();
@@ -1418,31 +1454,9 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
emitShadowCallStackPrologue(*TII, MF, MBB, MBBI, DL, NeedsWinCFI,
MFnI.needsDwarfUnwindInfo(MF));
- if (MFnI.shouldSignReturnAddress(MF)) {
- if (MFnI.shouldSignWithBKey()) {
- BuildMI(MBB, MBBI, DL, TII->get(AArch64::EMITBKEY))
- .setMIFlag(MachineInstr::FrameSetup);
- }
-
- // No SEH opcode for this one; it doesn't materialize into an
- // instruction on Windows.
- BuildMI(MBB, MBBI, DL,
- TII->get(MFnI.shouldSignWithBKey() ? AArch64::PACIBSP
- : AArch64::PACIASP))
- .setMIFlag(MachineInstr::FrameSetup);
+ if (MFnI.shouldSignReturnAddress(MF))
+ signLR(MF, MBB, MBBI, NeedsWinCFI, &HasWinCFI);
- if (EmitCFI) {
- unsigned CFIIndex =
- MF.addFrameInst(MCCFIInstruction::createNegateRAState(nullptr));
- BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
- .addCFIIndex(CFIIndex)
- .setMIFlags(MachineInstr::FrameSetup);
- } else if (NeedsWinCFI) {
- HasWinCFI = true;
- BuildMI(MBB, MBBI, DL, TII->get(AArch64::SEH_PACSignLR))
- .setMIFlag(MachineInstr::FrameSetup);
- }
- }
if (EmitCFI && MFnI.isMTETagged()) {
BuildMI(MBB, MBBI, DL, TII->get(AArch64::EMITMTETAGGED))
.setMIFlag(MachineInstr::FrameSetup);
@@ -1901,11 +1915,10 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
}
}
-static void InsertReturnAddressAuth(MachineFunction &MF, MachineBasicBlock &MBB,
- bool NeedsWinCFI, bool *HasWinCFI) {
+void AArch64FrameLowering::authenticateLR(MachineFunction &MF,
+ MachineBasicBlock &MBB,
+ bool NeedsWinCFI, bool *HasWinCFI) {
const auto &MFI = *MF.getInfo<AArch64FunctionInfo>();
- if (!MFI.shouldSignReturnAddress(MF))
- return;
const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
const TargetInstrInfo *TII = Subtarget.getInstrInfo();
bool EmitAsyncCFI = MFI.needsAsyncDwarfUnwindInfo(MF);
@@ -1920,10 +1933,11 @@ static void InsertReturnAddressAuth(MachineFunction &MF, MachineBasicBlock &MBB,
// From v8.3a onwards there are optimised authenticate LR and return
// instructions, namely RETA{A,B}, that can be used instead. In this case the
// DW_CFA_AARCH64_negate_ra_state can't be emitted.
- if (Subtarget.hasPAuth() &&
- !MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack) &&
- MBBI != MBB.end() && MBBI->getOpcode() == AArch64::RET_ReallyLR &&
- !NeedsWinCFI) {
+ bool TerminatorIsCombinable =
+ MBBI != MBB.end() && (MBBI->getOpcode() == AArch64::RET_ReallyLR ||
+ MBBI->getOpcode() == AArch64::RET);
+ if (Subtarget.hasPAuth() && TerminatorIsCombinable && !NeedsWinCFI &&
+ !MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack)) {
BuildMI(MBB, MBBI, DL,
TII->get(MFI.shouldSignWithBKey() ? AArch64::RETAB : AArch64::RETAA))
.copyImplicitOps(*MBBI);
@@ -1963,12 +1977,12 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF,
MachineBasicBlock &MBB) const {
MachineBasicBlock::iterator MBBI = MBB.getLastNonDebugInstr();
MachineFrameInfo &MFI = MF.getFrameInfo();
+ AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
const TargetInstrInfo *TII = Subtarget.getInstrInfo();
DebugLoc DL;
bool NeedsWinCFI = needsWinCFI(MF);
- bool EmitCFI =
- MF.getInfo<AArch64FunctionInfo>()->needsAsyncDwarfUnwindInfo(MF);
+ bool EmitCFI = AFI->needsAsyncDwarfUnwindInfo(MF);
bool HasWinCFI = false;
bool IsFunclet = false;
auto WinCFI = make_scope_exit([&]() { assert(HasWinCFI == MF.hasWinCFI()); });
@@ -1979,7 +1993,8 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF,
}
auto FinishingTouches = make_scope_exit([&]() {
- InsertReturnAddressAuth(MF, MBB, NeedsWinCFI, &HasWinCFI);
+ if (AFI->shouldSignReturnAddress(MF))
+ authenticateLR(MF, MBB, NeedsWinCFI, &HasWinCFI);
if (needsShadowCallStackPrologueEpilogue(MF))
emitShadowCallStackEpilogue(*TII, MF, MBB, MBB.getFirstTerminator(), DL);
if (EmitCFI)
@@ -1992,7 +2007,6 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF,
int64_t NumBytes = IsFunclet ? getWinEHFuncletFrameSize(MF)
: MFI.getStackSize();
- AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
// All calls are tail calls in GHC calling conv, and functions have no
// prologue/epilogue.
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.h b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
index 147b5c181be5e5..2d5529aa2fd8cb 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
@@ -30,6 +30,13 @@ class AArch64FrameLowering : public TargetFrameLowering {
eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
MachineBasicBlock::iterator I) const override;
+ static void signLR(MachineFunction &MF, MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI, bool NeedsWinCFI,
+ bool *HasWinCFI);
+
+ static void authenticateLR(MachineFunction &MF, MachineBasicBlock &MBB,
+ bool NeedsWinCFI, bool *HasWinCFI);
+
/// emitProlog/emitEpilog - These methods insert prolog and epilog code into
/// the function.
void emitPrologue(MachineFunction &MF, MachineBasicBlock &MBB) const override;
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index c6673ec600309c..32df3ec68ff2df 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "AArch64InstrInfo.h"
+#include "AArch64FrameLowering.h"
#include "AArch64MachineFunctionInfo.h"
#include "AArch64Subtarget.h"
#include "MCTargetDesc/AArch64AddressingModes.h"
@@ -7679,6 +7680,23 @@ AArch64InstrInfo::getOutliningCandidateInfo(
NumBytesToCreateFrame, FrameID);
}
+void AArch64InstrInfo::mergeOutliningCandidateAttributes(
+ Function &F, std::vector<outliner::Candidate> &Candidates) const {
+ // If a bunch of candidates reach this point they must agree on their return
+ // address signing. It is therefore enough to just consider the signing
+ // behaviour of one of them
+ const auto &CFn = Candidates.front().getMF()->getFunction();
+
+ // Since all candidates belong to the same module, just copy the
+ // function-level attributes of an arbitrary function.
+ if (CFn.hasFnAttribute("sign-return-address"))
+ F.addFnAttr(CFn.getFnAttribute("sign-return-address"));
+ if (CFn.hasFnAttribute("sign-return-address-key"))
+ F.addFnAttr(CFn.getFnAttribute("sign-return-address-key"));
+
+ AArch64GenInstrInfo::mergeOutliningCandidateAttributes(F, Candidates);
+}
+
bool AArch64InstrInfo::isFunctionSafeToOutlineFrom(
MachineFunction &MF, bool OutlineFromLinkOnceODRs) const {
const Function &F = MF.getFunction();
@@ -7991,65 +8009,15 @@ void AArch64InstrInfo::fixupPostOutline(MachineBasicBlock &MBB) const {
}
static void signOutlinedFunction(MachineFunction &MF, MachineBasicBlock &MBB,
- bool ShouldSignReturnAddr,
- bool ShouldSignReturnAddrWithBKey) {
- if (ShouldSignReturnAddr) {
- MachineBasicBlock::iterator MBBPAC = MBB.begin();
- MachineBasicBlock::iterator MBBAUT = MBB.getFirstTerminator();
- const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
- const TargetInstrInfo *TII = Subtarget.getInstrInfo();
- DebugLoc DL;
-
- if (MBBAUT != MBB.end())
- DL = MBBAUT->getDebugLoc();
-
- // At the very beginning of the basic block we insert the following
- // depending on the key type
- //
- // a_key: b_key:
- // PACIASP EMITBKEY
- // CFI_INSTRUCTION PACIBSP
- // CFI_INSTRUCTION
- if (ShouldSignReturnAddrWithBKey) {
- BuildMI(MBB, MBBPAC, DebugLoc(), TII->get(AArch64::EMITBKEY))
- .setMIFlag(MachineInstr::FrameSetup);
- }
-
- BuildMI(MBB, MBBPAC, DebugLoc(),
- TII->get(ShouldSignReturnAddrWithBKey ? AArch64::PACIBSP
- : AArch64::PACIASP))
- .setMIFlag(MachineInstr::FrameSetup);
+ bool ShouldSignReturnAddr) {
+ if (!ShouldSignReturnAddr)
+ return;
- if (MF.getInfo<AArch64FunctionInfo>()->needsDwarfUnwindInfo(MF)) {
- unsigned CFIIndex =
- MF.addFrameInst(MCCFIInstruction::createNegateRAState(nullptr));
- BuildMI(MBB, MBBPAC, DebugLoc(), TII->get(AArch64::CFI_INSTRUCTION))
- .addCFIIndex(CFIIndex)
- .setMIFlags(MachineInstr::FrameSetup);
- }
+ AArch64FrameLowering::signLR(MF, MBB, MBB.begin(),
+ /*NeedsWinCFI=*/false, /*HasWinCFI=*/nullptr);
- // If v8.3a features are available we can replace a RET instruction by
- // RETAA or RETAB and omit the AUT instructions. In this case the
- // DW_CFA_AARCH64_negate_ra_state can't be emitted.
- if (Subtarget.hasPAuth() && MBBAUT != MBB.end() &&
- MBBAUT->getOpcode() == AArch64::RET) {
- BuildMI(MBB, MBBAUT, DL,
- TII->get(ShouldSignReturnAddrWithBKey ? AArch64::RETAB
- : AArch64::RETAA))
- .copyImplicitOps(*MBBAUT);
- MBB.erase(MBBAUT);
- } else {
- BuildMI(MBB, MBBAUT, DL,
- TII->get(ShouldSignReturnAddrWithBKey ? AArch64::AUTIBSP
- : AArch64::AUTIASP))
- .setMIFlag(MachineInstr::FrameDestroy);
- unsigned CFIIndexAuth =
- MF.addFrameInst(MCCFIInstruction::createNegateRAState(nullptr));
- BuildMI(MBB, MBBAUT, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
- .addCFIIndex(CFIIndexAuth)
- .setMIFlags(MachineInstr::FrameDestroy);
- }
- }
+ AArch64FrameLowering::authenticateLR(MF, MBB, /*NeedsWinCFI=*/false,
+ /*HasWinCFI=*/nullptr);
}
void AArch64InstrInfo::buildOutlinedFrame(
@@ -8149,20 +8117,12 @@ void AArch64InstrInfo::buildOutlinedFrame(
Et = MBB.insert(Et, LDRXpost);
}
- // If a bunch of candidates reach this point they must agree on their return
- // address signing. It is therefore enough to just consider the signing
- // behaviour of one of them
- const auto &MFI = *OF.Candidates.front().getMF()->getInfo<AArch64FunctionInfo>();
- bool ShouldSignReturnAddr = MFI.shouldSignReturnAddress(!IsLeafFunction);
-
- // a_key is the default
- bool ShouldSignReturnAddrWithBKey = MFI.shouldSignWithBKey();
+ bool ShouldSignReturnAddr = FI->shouldSignReturnAddress(!IsLeafFunction);
// If this is a tail call outlined function, then there's already a return.
if (OF.FrameConstructionID == MachineOutlinerTailCall ||
OF.FrameConstructionID == MachineOutlinerThunk) {
- signOutlinedFunction(MF, MBB, ShouldSignReturnAddr,
- ShouldSignReturnAddrWithBKey);
+ signOutlinedFunction(MF, MBB, ShouldSignReturnAddr);
return;
}
@@ -8176,8 +8136,7 @@ void AArch64InstrInfo::buildOutlinedFrame(
.addReg(AArch64::LR);
MBB.insert(MBB.end(), ret);
- signOutlinedFunction(MF, MBB, ShouldSignReturnAddr,
- ShouldSignReturnAddrWithBKey);
+ signOutlinedFunction(MF, MBB, ShouldSignReturnAddr);
FI->setOutliningStyle("Function");
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index 20210a96d67ad2..4d964e79096ea2 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -295,6 +295,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
bool OutlineFromLinkOnceODRs) const override;
std::optional<outliner::OutlinedFunction> getOutliningCandidateInfo(
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const override;
+ void mergeOutliningCandidateAttributes(
+ Function &F, std::vector<outliner::Candidate> &Candidates) const override;
outliner::InstrType
getOutliningTypeImpl(MachineBasicBlock::iterator &MIT, unsigned Flags) const override;
SmallVector<
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
index 0bf0a98cafebc9..3199b091766c46 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
@@ -211,5 +211,4 @@ body: |
# CHECK-NEXT: $sp = frame-setup SUBXri $sp, 16, 0
# CHECK: $sp = frame-destroy ADDXri $sp, 16, 0
# CHECK-NEXT: frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
-# CHECK-NEXT: frame-destroy CFI_INSTRUCTION negate_ra_sign_state
# CHECK-NEXT: RET $lr
More information about the llvm-commits
mailing list