Skip to content

Commit 39397cf

Browse files
committed
[BOLT][AArch64] Fix which PSign and PAuth variants are used (#120064)
- only the ones operating on LR should be marked with .cfi_cfi_negate_ra_state - added support for fused PtrAuth and Ret instructions, e.g. RETAA.
1 parent 2ca430a commit 39397cf

File tree

5 files changed

+52
-39
lines changed

5 files changed

+52
-39
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,21 @@ class MCPlusBuilder {
581581
return getNoRegister();
582582
}
583583

584+
virtual bool isPSignOnLR(const MCInst &Inst) const {
585+
llvm_unreachable("not implemented");
586+
return false;
587+
}
588+
589+
virtual bool isPAuthOnLR(const MCInst &Inst) const {
590+
llvm_unreachable("not implemented");
591+
return false;
592+
}
593+
594+
virtual bool isPAuthAndRet(const MCInst &Inst) const {
595+
llvm_unreachable("not implemented");
596+
return false;
597+
}
598+
584599
virtual bool isAuthenticationOfReg(const MCInst &Inst,
585600
MCPhysReg AuthenticatedReg) const {
586601
llvm_unreachable("not implemented");
@@ -794,13 +809,6 @@ class MCPlusBuilder {
794809
llvm_unreachable("not implemented");
795810
return false;
796811
}
797-
virtual bool isPAuth(MCInst &Inst) const {
798-
llvm_unreachable("not implemented");
799-
}
800-
801-
virtual bool isPSign(MCInst &Inst) const {
802-
llvm_unreachable("not implemented");
803-
}
804812

805813
virtual bool isCleanRegXOR(const MCInst &Inst) const {
806814
llvm_unreachable("not implemented");

bolt/include/bolt/Passes/InsertNegateRAStatePass.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ class InsertNegateRAState : public BinaryFunctionPass {
3030

3131
private:
3232
/// Loops over all instructions and adds OpNegateRAState CFI
33-
/// after any pointer signing or authenticating instructions.
33+
/// after any pointer signing or authenticating instructions,
34+
/// which operate on the LR, except fused ptrauth + ret instructions
35+
/// (such as RETAA).
3436
/// Returns true, if any OpNegateRAState CFIs were added.
3537
bool addNegateRAStateAfterPacOrAuth(BinaryFunction &BF);
3638
/// Because states are tracked as MCAnnotations on individual instructions,

bolt/lib/Passes/InsertNegateRAStatePass.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,16 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
4646
bool FirstIter = true;
4747
MCInst PrevInst;
4848
BinaryBasicBlock *PrevBB = nullptr;
49+
// We need to iterate on BBs in the Layout order
50+
// not in the order they are stored in the BF class.
4951
auto *Begin = BF.getLayout().block_begin();
5052
auto *End = BF.getLayout().block_end();
5153
for (auto *BB = Begin; BB != End; BB++) {
5254

5355
// Support for function splitting:
54-
// if two consecutive BBs are going to end up in different functions,
55-
// we have to negate the RA State, so the new function starts with a Signed
56-
// state.
56+
// if two consecutive BBs with Signed state are going to end up in different
57+
// functions, we have to add a OpNegateRAState to the beginning of the newly
58+
// split function, so it starts with a Signed state.
5759
if (PrevBB != nullptr &&
5860
PrevBB->getFragmentNum() != (*BB)->getFragmentNum() &&
5961
BC.MIB->isRASigned(*((*BB)->begin()))) {
@@ -68,6 +70,8 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
6870
continue;
6971

7072
if (!FirstIter) {
73+
// Consecutive instructions with different RAState means we need to add
74+
// a OpNegateRAState.
7175
if ((BC.MIB->isRASigned(PrevInst) && BC.MIB->isRAUnsigned(Inst)) ||
7276
(BC.MIB->isRAUnsigned(PrevInst) && BC.MIB->isRASigned(Inst))) {
7377

@@ -90,7 +94,8 @@ bool InsertNegateRAState::addNegateRAStateAfterPacOrAuth(BinaryFunction &BF) {
9094
for (BinaryBasicBlock &BB : BF) {
9195
for (auto Iter = BB.begin(); Iter != BB.end(); ++Iter) {
9296
MCInst &Inst = *Iter;
93-
if (BC.MIB->isPSign(Inst) || BC.MIB->isPAuth(Inst)) {
97+
if (BC.MIB->isPSignOnLR(Inst) ||
98+
(BC.MIB->isPAuthOnLR(Inst) && !BC.MIB->isPAuthAndRet(Inst))) {
9499
Iter = BF.addCFIInstruction(
95100
&BB, Iter + 1, MCCFIInstruction::createNegateRAState(nullptr));
96101
FoundAny = true;

bolt/lib/Passes/MarkRAStates.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
4646
for (BinaryBasicBlock &BB : BF) {
4747
for (auto It = BB.begin(); It != BB.end(); ++It) {
4848
MCInst &Inst = *It;
49-
if ((BC.MIB->isPSign(Inst) || BC.MIB->isPAuth(Inst)) &&
49+
if ((BC.MIB->isPSignOnLR(Inst) ||
50+
(BC.MIB->isPAuthOnLR(Inst) && !BC.MIB->isPAuthAndRet(Inst))) &&
5051
!BC.MIB->hasNegateRAState(Inst)) {
5152
// no .cfi_negate_ra_state attached to signing or authenticating instr
5253
// means, that this is a function with handwritten assembly, which might
@@ -71,7 +72,7 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
7172
if (BC.MIB->isCFI(Inst))
7273
continue;
7374

74-
if (BC.MIB->isPSign(Inst)) {
75+
if (BC.MIB->isPSignOnLR(Inst)) {
7576
if (RAState) {
7677
// RA signing instructions should only follow unsigned RA state.
7778
BC.outs() << "BOLT-INFO: inconsistent RAStates in function "
@@ -80,7 +81,7 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
8081
return;
8182
}
8283
BC.MIB->setRASigning(Inst);
83-
} else if (BC.MIB->isPAuth(Inst)) {
84+
} else if (BC.MIB->isPAuthOnLR(Inst)) {
8485
if (!RAState) {
8586
// RA authenticating instructions should only follow signed RA state.
8687
BC.outs() << "BOLT-INFO: inconsistent RAStates in function "

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,27 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
250250
}
251251
}
252252

253+
bool isPSignOnLR(const MCInst &Inst) const override {
254+
MCPhysReg SignReg = getSignedReg(Inst);
255+
return SignReg != getNoRegister() && SignReg == AArch64::LR;
256+
}
257+
258+
bool isPAuthOnLR(const MCInst &Inst) const override {
259+
ErrorOr<MCPhysReg> AutReg = getAuthenticatedReg(Inst);
260+
if (AutReg && *AutReg != getNoRegister() && *AutReg == AArch64::LR)
261+
return true;
262+
return false;
263+
}
264+
265+
bool isPAuthAndRet(const MCInst &Inst) const override {
266+
return Inst.getOpcode() == AArch64::RETAA ||
267+
Inst.getOpcode() == AArch64::RETAB ||
268+
Inst.getOpcode() == AArch64::RETAASPPCi ||
269+
Inst.getOpcode() == AArch64::RETABSPPCi ||
270+
Inst.getOpcode() == AArch64::RETAASPPCr ||
271+
Inst.getOpcode() == AArch64::RETABSPPCr;
272+
}
273+
253274
bool isAuthenticationOfReg(const MCInst &Inst, MCPhysReg Reg) const override {
254275
if (Reg == getNoRegister())
255276
return false;
@@ -904,30 +925,6 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
904925
}
905926
return false;
906927
}
907-
bool isPAuth(MCInst &Inst) const override {
908-
return Inst.getOpcode() == AArch64::AUTIA ||
909-
Inst.getOpcode() == AArch64::AUTIB ||
910-
Inst.getOpcode() == AArch64::AUTIA1716 ||
911-
Inst.getOpcode() == AArch64::AUTIB1716 ||
912-
Inst.getOpcode() == AArch64::AUTIASP ||
913-
Inst.getOpcode() == AArch64::AUTIBSP ||
914-
Inst.getOpcode() == AArch64::AUTIAZ ||
915-
Inst.getOpcode() == AArch64::AUTIBZ ||
916-
Inst.getOpcode() == AArch64::AUTIZA ||
917-
Inst.getOpcode() == AArch64::AUTIZB;
918-
}
919-
bool isPSign(MCInst &Inst) const override {
920-
return Inst.getOpcode() == AArch64::PACIA ||
921-
Inst.getOpcode() == AArch64::PACIB ||
922-
Inst.getOpcode() == AArch64::PACIA1716 ||
923-
Inst.getOpcode() == AArch64::PACIB1716 ||
924-
Inst.getOpcode() == AArch64::PACIASP ||
925-
Inst.getOpcode() == AArch64::PACIBSP ||
926-
Inst.getOpcode() == AArch64::PACIAZ ||
927-
Inst.getOpcode() == AArch64::PACIBZ ||
928-
Inst.getOpcode() == AArch64::PACIZA ||
929-
Inst.getOpcode() == AArch64::PACIZB;
930-
}
931928

932929
bool isRegToRegMove(const MCInst &Inst, MCPhysReg &From,
933930
MCPhysReg &To) const override {

0 commit comments

Comments
 (0)