Skip to content

[BOLT][BTI] Refactor BTI helpers #173000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bgergely0 wants to merge 1 commit into
base: main
Choose a base branch
from
+75 −77
Open

[BOLT][BTI] Refactor BTI helpers #173000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions bolt/include/bolt/Core/MCPlusBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ enum class IndirectBranchType : char {
/// PIC jump table.
};

/// Enum used for readability when describing different BTI instruction
/// variants. The variant is encoded as an immediate of the instruction in LLVM.
enum BTIKind {
C, /// Accepting calls, and jumps using x16/x17.
J, /// Accepting jumps.
JC /// Accepting both.
};

class MCPlusBuilder {
public:
using AllocatorIdTy = uint16_t;
Expand Down Expand Up @@ -1871,8 +1879,7 @@ class MCPlusBuilder {

/// Check if an Instruction is a BTI landing pad with the required properties.
/// Takes both explicit and implicit BTIs into account.
virtual bool isBTILandingPad(MCInst &Inst, bool CallTarget,
bool JumpTarget) const {
virtual bool isBTILandingPad(MCInst &Inst, BTIKind BTI) const {
llvm_unreachable("not implemented");
return false;
}
Expand All @@ -1884,13 +1891,7 @@ class MCPlusBuilder {
}

/// Create a BTI landing pad instruction.
virtual void createBTI(MCInst &Inst, bool CallTarget, bool JumpTarget) const {
llvm_unreachable("not implemented");
}

/// Update operand of BTI instruction.
virtual void updateBTIVariant(MCInst &Inst, bool CallTarget,
bool JumpTarget) const {
virtual void createBTI(MCInst &Inst, BTIKind BTI) const {
llvm_unreachable("not implemented");
}

Expand Down
47 changes: 21 additions & 26 deletions bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2775,15 +2775,18 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return Insts;
}

void createBTI(MCInst &Inst, bool CallTarget,
bool JumpTarget) const override {
void createBTI(MCInst &Inst, BTIKind BTI) const override {
Inst.setOpcode(AArch64::HINT);
Inst.clear();
bool CallTarget = BTI == BTIKind::C || BTI == BTIKind::JC;
bool JumpTarget = BTI == BTIKind::J || BTI == BTIKind::JC;
unsigned HintNum = getBTIHintNum(CallTarget, JumpTarget);
Inst.addOperand(MCOperand::createImm(HintNum));
}

bool isBTILandingPad(MCInst &Inst, bool CallTarget,
bool JumpTarget) const override {
bool isBTILandingPad(MCInst &Inst, BTIKind BTI) const override {
bool CallTarget = BTI == BTIKind::C || BTI == BTIKind::JC;
bool JumpTarget = BTI == BTIKind::J || BTI == BTIKind::JC;
unsigned HintNum = getBTIHintNum(CallTarget, JumpTarget);
bool IsExplicitBTI =
Inst.getOpcode() == AArch64::HINT && Inst.getNumOperands() == 1 &&
Expand All @@ -2800,22 +2803,14 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
Inst.getOpcode() == AArch64::PACIBSP;
}

void updateBTIVariant(MCInst &Inst, bool CallTarget,
bool JumpTarget) const override {
assert(Inst.getOpcode() == AArch64::HINT && "Not a BTI instruction.");
unsigned HintNum = getBTIHintNum(CallTarget, JumpTarget);
Inst.clear();
Inst.addOperand(MCOperand::createImm(HintNum));
}

bool isCallCoveredByBTI(MCInst &Call, MCInst &Pad) const override {
assert((isIndirectCall(Call) || isIndirectBranch(Call)) &&
"Not an indirect call or branch.");

// A BLR can be accepted by a BTI c.
if (isIndirectCall(Call))
return isBTILandingPad(Pad, true, false) ||
isBTILandingPad(Pad, true, true);
return isBTILandingPad(Pad, BTIKind::C) ||
isBTILandingPad(Pad, BTIKind::JC);

// A BR can be accepted by a BTI j or BTI c (and BTI jc) IF the operand is
// x16 or x17. If the operand is not x16 or x17, it can be accepted by a BTI
Expand All @@ -2827,11 +2822,11 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
"Indirect branch does not have a register operand.");
MCPhysReg Reg = Call.getOperand(0).getReg();
if (Reg == AArch64::X16 || Reg == AArch64::X17)
return isBTILandingPad(Pad, true, false) ||
isBTILandingPad(Pad, false, true) ||
isBTILandingPad(Pad, true, true);
return isBTILandingPad(Pad, false, true) ||
isBTILandingPad(Pad, true, true);
return isBTILandingPad(Pad, BTIKind::C) ||
isBTILandingPad(Pad, BTIKind::J) ||
isBTILandingPad(Pad, BTIKind::JC);
return isBTILandingPad(Pad, BTIKind::J) ||
isBTILandingPad(Pad, BTIKind::JC);
}
return false;
}
Expand All @@ -2846,11 +2841,11 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
if (isIndirectCall(Call)) {
// if we have a BTI j at the start, extend it to a BTI jc,
// otherwise insert a new BTI c.
if (!Empty && isBTILandingPad(*II, false, true)) {
updateBTIVariant(*II, true, true);
if (!Empty && isBTILandingPad(*II, BTIKind::J)) {
createBTI(*II, BTIKind::JC);
} else {
MCInst BTIInst;
createBTI(BTIInst, true, false);
createBTI(BTIInst, BTIKind::C);
BB.insertInstruction(II, BTIInst);
}
}
Expand All @@ -2867,16 +2862,16 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
if (Reg == AArch64::X16 || Reg == AArch64::X17) {
// Add a new BTI c
MCInst BTIInst;
createBTI(BTIInst, true, false);
createBTI(BTIInst, BTIKind::C);
BB.insertInstruction(II, BTIInst);
} else {
// If BB starts with a BTI c, extend it to BTI jc,
// otherwise insert a new BTI j.
if (!Empty && isBTILandingPad(*II, true, false)) {
updateBTIVariant(*II, true, true);
if (!Empty && isBTILandingPad(*II, BTIKind::C)) {
createBTI(*II, BTIKind::JC);
} else {
MCInst BTIInst;
createBTI(BTIInst, false, true);
createBTI(BTIInst, BTIKind::J);
BB.insertInstruction(II, BTIInst);
}
}
Expand Down
86 changes: 44 additions & 42 deletions bolt/unittests/Core/MCPlusBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,51 +150,39 @@ TEST_P(MCPlusBuilderTester, AArch64_BTI) {
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();

MCInst BTIjc;
BC->MIB->createBTI(BTIjc, true, true);
BC->MIB->createBTI(BTIjc, BTIKind::JC);
BB->addInstruction(BTIjc);
auto II = BB->begin();
ASSERT_EQ(II->getOpcode(), AArch64::HINT);
ASSERT_EQ(II->getOperand(0).getImm(), 38);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true));
BC->MIB->updateBTIVariant(*II, true, false);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::JC));

MCInst BTIj;
BC->MIB->createBTI(BTIj, false, true);
BC->MIB->createBTI(BTIj, BTIKind::J);
II = BB->addInstruction(BTIj);
ASSERT_EQ(II->getOpcode(), AArch64::HINT);
ASSERT_EQ(II->getOperand(0).getImm(), 36);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, false, true));
BC->MIB->updateBTIVariant(*II, true, true);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true));
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::J));

MCInst BTIc;
BC->MIB->createBTI(BTIc, true, false);
BC->MIB->createBTI(BTIc, BTIKind::C);
II = BB->addInstruction(BTIc);
ASSERT_EQ(II->getOpcode(), AArch64::HINT);
ASSERT_EQ(II->getOperand(0).getImm(), 34);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
BC->MIB->updateBTIVariant(*II, false, true);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, false, true));

#ifndef NDEBUG
MCInst BTIinvalid;
ASSERT_DEATH(BC->MIB->createBTI(BTIinvalid, false, false),
"No target kinds!");
#endif
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));

MCInst Paciasp = MCInstBuilder(AArch64::PACIASP);
II = BB->addInstruction(Paciasp);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, true, true));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, false, true));
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, BTIKind::JC));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, BTIKind::J));
ASSERT_TRUE(BC->MIB->isImplicitBTIC(*II));

MCInst Pacibsp = MCInstBuilder(AArch64::PACIBSP);
II = BB->addInstruction(Pacibsp);
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, true, true));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, false, true));
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, BTIKind::JC));
ASSERT_FALSE(BC->MIB->isBTILandingPad(*II, BTIKind::J));
ASSERT_TRUE(BC->MIB->isImplicitBTIC(*II));
}

Expand All @@ -207,7 +195,7 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_empty) {
BC->MIB->insertBTI(*BB, CallInst);
// Check that BTI c is added to the empty block.
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
}
TEST_P(MCPlusBuilderTester, AArch64_insertBTI_0) {
if (GetParam() != Triple::aarch64)
Expand All @@ -218,9 +206,11 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_0) {
BB->addInstruction(Inst);
// BR x16 needs BTI c or BTI j. We prefer adding a BTI c.
MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X16);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_FALSE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
}

TEST_P(MCPlusBuilderTester, AArch64_insertBTI_1) {
Expand All @@ -229,13 +219,15 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_1) {
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
MCInst BTIc;
BC->MIB->createBTI(BTIc, true, false);
BC->MIB->createBTI(BTIc, BTIKind::C);
BB->addInstruction(BTIc);
// BR x16 needs BTI c or BTI j. We have a BTI c, no change is needed.
MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X16);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_TRUE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
}

TEST_P(MCPlusBuilderTester, AArch64_insertBTI_2) {
Expand All @@ -244,14 +236,16 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_2) {
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
MCInst BTIc;
BC->MIB->createBTI(BTIc, true, false);
BC->MIB->createBTI(BTIc, BTIKind::C);
BB->addInstruction(BTIc);
// BR x5 needs BTI j
// we have BTI c -> extend it to BTI jc.
MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X5);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true));
ASSERT_FALSE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::JC));
}

TEST_P(MCPlusBuilderTester, AArch64_insertBTI_3) {
Expand All @@ -263,9 +257,11 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_3) {
BB->addInstruction(Inst);
// BR x5 needs BTI j
MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X5);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, false, true));
ASSERT_FALSE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::J));
}

TEST_P(MCPlusBuilderTester, AArch64_insertBTI_4) {
Expand All @@ -277,9 +273,11 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_4) {
BB->addInstruction(Inst);
// BLR needs BTI c, regardless of the register used.
MCInst CallInst = MCInstBuilder(AArch64::BLR).addReg(AArch64::X5);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_FALSE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
}

TEST_P(MCPlusBuilderTester, AArch64_insertBTI_5) {
Expand All @@ -288,14 +286,16 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_5) {
BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true);
std::unique_ptr<BinaryBasicBlock> BB = BF->createBasicBlock();
MCInst BTIj;
BC->MIB->createBTI(BTIj, false, true);
BC->MIB->createBTI(BTIj, BTIKind::J);
BB->addInstruction(BTIj);
// BLR needs BTI c, regardless of the register used.
// We have a BTI j -> extend it to BTI jc.
MCInst CallInst = MCInstBuilder(AArch64::BLR).addReg(AArch64::X5);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true));
ASSERT_FALSE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::JC));
}

TEST_P(MCPlusBuilderTester, AArch64_insertBTI_6) {
Expand All @@ -308,9 +308,11 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_6) {
BB->addInstruction(Paciasp);
// PACI(AB)SP are implicit BTI c, no change needed.
MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X17);
BC->MIB->insertBTI(*BB, CallInst);
auto II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false));
ASSERT_TRUE(BC->MIB->isCallCoveredByBTI(CallInst, *II));
BC->MIB->insertBTI(*BB, CallInst);
II = BB->begin();
ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, BTIKind::C));
ASSERT_TRUE(BC->MIB->isPSignOnLR(*II));
}

Expand Down
Loading