From 5ab3f19b63c164244b3531178ea9f1bdc64595f7 Mon Sep 17 00:00:00 2001 From: Gergely Balint Date: Wed, 17 Sep 2025 12:24:04 +0000 Subject: [PATCH 1/5] [BOLT][BTI] Add MCPlusBuilder::addBTItoBBStart This function contains most of the logic for BTI: - it takes the BasicBlock and the instruction used to jump to it. - then it checks if the first non-pseudo instruction is a sufficient landing pad for the used call. - if not, it generates the correct BTI instruction. Also introduce the isBTIVariantCoveringCall helper to simplify the logic. --- bolt/include/bolt/Core/MCPlusBuilder.h | 13 +++ .../Target/AArch64/AArch64MCPlusBuilder.cpp | 75 +++++++++++++ bolt/unittests/Core/MCPlusBuilder.cpp | 105 ++++++++++++++++++ 3 files changed, 193 insertions(+) diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index a318ef0b6bd68..86b3d4d05ffac 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1894,6 +1894,19 @@ class MCPlusBuilder { llvm_unreachable("not implemented"); } + /// Checks if the indirect call / jump is accepted by the landing pad at the + /// start of the target BasicBlock. + virtual bool isBTIVariantCoveringCall(MCInst &Call, MCInst &Pad) const { + llvm_unreachable("not implemented"); + return false; + } + + /// Adds a BTI landing pad to the start of the BB, that matches the indirect + /// call/jump inst. + virtual void addBTItoBBStart(BinaryBasicBlock &BB, MCInst &Call) const { + llvm_unreachable("not implemented"); + } + /// Store \p Target absolute address to \p RegName virtual InstructionListType materializeAddress(const MCSymbol *Target, MCContext *Ctx, diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index af87d5c12b5ce..8a39f3e8ca25c 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2808,6 +2808,81 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createImm(HintNum)); } + bool isBTIVariantCoveringCall(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); + + // 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 + // j or BTI jc (and not BTI c). + if (isIndirectBranch(Call)) { + assert(Call.getNumOperands() == 1 && + "Indirect branch needs to have 1 operand."); + assert(Call.getOperand(0).isReg() && + "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 false; + } + + void addBTItoBBStart(BinaryBasicBlock &BB, MCInst &Call) const override { + auto II = BB.getFirstNonPseudo(); + if (II != BB.end()) { + if (isBTIVariantCoveringCall(Call, *II)) + return; + // A BLR can be accepted by a BTI c. + 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 (isBTILandingPad(*II, false, true)) { + updateBTIVariant(*II, true, true); + } else { + MCInst BTIInst; + createBTI(BTIInst, true, false); + BB.insertInstruction(II, BTIInst); + } + } + + // 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 j or BTI jc (and not BTI c). + if (isIndirectBranch(Call)) { + assert(Call.getNumOperands() == 1 && + "Indirect branch needs to have 1 operand."); + assert(Call.getOperand(0).isReg() && + "Indirect branch does not have a register operand."); + MCPhysReg Reg = Call.getOperand(0).getReg(); + if (Reg == AArch64::X16 || Reg == AArch64::X17) { + // Add a new BTI c + MCInst BTIInst; + createBTI(BTIInst, true, false); + BB.insertInstruction(II, BTIInst); + } else { + // If BB starts with a BTI c, extend it to BTI jc, + // otherwise insert a new BTI j. + if (isBTILandingPad(*II, true, false)) { + updateBTIVariant(*II, true, true); + } else { + MCInst BTIInst; + createBTI(BTIInst, false, true); + BB.insertInstruction(II, BTIInst); + } + } + } + } + } + InstructionListType materializeAddress(const MCSymbol *Target, MCContext *Ctx, MCPhysReg RegName, int64_t Addend = 0) const override { diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp index 7b6f1620a3f2c..f4d120d0e2dec 100644 --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -198,6 +198,111 @@ TEST_P(MCPlusBuilderTester, AArch64_BTI) { ASSERT_TRUE(BC->MIB->isImplicitBTIC(*II)); } +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_0) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst Inst = MCInstBuilder(AArch64::RET).addReg(AArch64::LR); + 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->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); +} + +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_1) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst BTIc; + BC->MIB->createBTI(BTIc, true, false); + 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->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); +} + +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_2) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst BTIc; + BC->MIB->createBTI(BTIc, true, false); + 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->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true)); +} + +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_3) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst Inst = MCInstBuilder(AArch64::RET).addReg(AArch64::LR); + BB->addInstruction(Inst); + // BR x5 needs BTI j + MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X5); + BC->MIB->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, false, true)); +} + +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_4) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst Inst = MCInstBuilder(AArch64::RET).addReg(AArch64::LR); + BB->addInstruction(Inst); + // BLR needs BTI c, regardless of the register used. + MCInst CallInst = MCInstBuilder(AArch64::BLR).addReg(AArch64::X5); + BC->MIB->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); +} + +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_5) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst BTIj; + BC->MIB->createBTI(BTIj, false, true); + 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->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true)); +} + +TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_6) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst Paciasp = + MCInstBuilder(AArch64::PACIASP).addReg(AArch64::LR).addReg(AArch64::SP); + BB->addInstruction(Paciasp); + // PACI(AB)SP are implicit BTI c, no change needed. + MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X17); + BC->MIB->addBTItoBBStart(*BB, CallInst); + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); + ASSERT_TRUE(BC->MIB->isPSignOnLR(*II)); +} + TEST_P(MCPlusBuilderTester, AArch64_CmpJNE) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); From a12f44e3642545cec1bdddf5b78a2421cacbe50d Mon Sep 17 00:00:00 2001 From: Gergely Balint Date: Wed, 26 Nov 2025 17:18:00 +0000 Subject: [PATCH 2/5] [BOLT][BTI] rename addBTItoBBStart to insertBTI --- bolt/include/bolt/Core/MCPlusBuilder.h | 6 ++-- .../Target/AArch64/AArch64MCPlusBuilder.cpp | 2 +- bolt/unittests/Core/MCPlusBuilder.cpp | 28 +++++++++---------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index 86b3d4d05ffac..4011fa2b557e6 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1901,9 +1901,9 @@ class MCPlusBuilder { return false; } - /// Adds a BTI landing pad to the start of the BB, that matches the indirect - /// call/jump inst. - virtual void addBTItoBBStart(BinaryBasicBlock &BB, MCInst &Call) const { + /// Inserts a BTI landing pad to the start of the BB, that matches the + /// indirect call inst used to call the BB. + virtual void insertBTI(BinaryBasicBlock &BB, MCInst &Call) const { llvm_unreachable("not implemented"); } diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index 8a39f3e8ca25c..dc96e91dfe2f4 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2836,7 +2836,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { return false; } - void addBTItoBBStart(BinaryBasicBlock &BB, MCInst &Call) const override { + void insertBTI(BinaryBasicBlock &BB, MCInst &Call) const override { auto II = BB.getFirstNonPseudo(); if (II != BB.end()) { if (isBTIVariantCoveringCall(Call, *II)) diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp index f4d120d0e2dec..83395e4e10493 100644 --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -198,7 +198,7 @@ TEST_P(MCPlusBuilderTester, AArch64_BTI) { ASSERT_TRUE(BC->MIB->isImplicitBTIC(*II)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_0) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_0) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -207,12 +207,12 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_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->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_1) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_1) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -222,12 +222,12 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_1) { 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->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_2) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_2) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -238,12 +238,12 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_2) { // BR x5 needs BTI j // we have BTI c -> extend it to BTI jc. MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X5); - BC->MIB->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_3) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_3) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -252,12 +252,12 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_3) { BB->addInstruction(Inst); // BR x5 needs BTI j MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X5); - BC->MIB->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, false, true)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_4) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_4) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -266,12 +266,12 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_4) { BB->addInstruction(Inst); // BLR needs BTI c, regardless of the register used. MCInst CallInst = MCInstBuilder(AArch64::BLR).addReg(AArch64::X5); - BC->MIB->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_5) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_5) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -282,12 +282,12 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_5) { // 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->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, true)); } -TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_6) { +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_6) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); @@ -297,7 +297,7 @@ TEST_P(MCPlusBuilderTester, AArch64_addBTItoBBStart_6) { BB->addInstruction(Paciasp); // PACI(AB)SP are implicit BTI c, no change needed. MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X17); - BC->MIB->addBTItoBBStart(*BB, CallInst); + BC->MIB->insertBTI(*BB, CallInst); auto II = BB->begin(); ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); ASSERT_TRUE(BC->MIB->isPSignOnLR(*II)); From 43eecd147ac8aa2107119f9a2c663a0a2c23254f Mon Sep 17 00:00:00 2001 From: Gergely Balint Date: Fri, 5 Dec 2025 12:42:34 +0000 Subject: [PATCH 3/5] [BOLT] Add assert wehn calling insertBTI on empty BBs BOLT may generate empty BBs, e.g. around function splitting, to hold temporary labels. If they are the target of a new indirect branch, the BTI should be inserted into the first "real" BasicBlock. --- bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 4 ++++ bolt/unittests/Core/MCPlusBuilder.cpp | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index dc96e91dfe2f4..ea02846c18572 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2838,6 +2838,10 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { void insertBTI(BinaryBasicBlock &BB, MCInst &Call) const override { auto II = BB.getFirstNonPseudo(); + assert(II != BB.end() && + "insertBTI should only be called on non-empty BasicBlocks"); + // Make sure there is no crash in non-assertion builds when calling on empty + // BBs. if (II != BB.end()) { if (isBTIVariantCoveringCall(Call, *II)) return; diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp index 83395e4e10493..dd3704742c694 100644 --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -198,6 +198,20 @@ TEST_P(MCPlusBuilderTester, AArch64_BTI) { ASSERT_TRUE(BC->MIB->isImplicitBTIC(*II)); } +TEST_P(MCPlusBuilderTester, AArch64_insertBTI_empty) { + if (GetParam() != Triple::aarch64) + GTEST_SKIP(); + BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); + std::unique_ptr BB = BF->createBasicBlock(); + MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X16); +#ifndef NDEBUG + ASSERT_DEATH(BC->MIB->insertBTI(*BB, CallInst), + "insertBTI should only be called on non-empty BasicBlocks"); +#else + BC->MIB->insertBTI(*BB, CallInst); + ASSERT(BB->size() == 0); +#endif +} TEST_P(MCPlusBuilderTester, AArch64_insertBTI_0) { if (GetParam() != Triple::aarch64) GTEST_SKIP(); From 6f63c8d005c09607abbf740822336d3e73bb0036 Mon Sep 17 00:00:00 2001 From: Gergely Balint Date: Tue, 9 Dec 2025 10:36:47 +0000 Subject: [PATCH 4/5] [BOLT] allow adding BTI to empty BBs --- .../Target/AArch64/AArch64MCPlusBuilder.cpp | 76 +++++++++---------- bolt/unittests/Core/MCPlusBuilder.cpp | 9 +-- 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index ea02846c18572..601599bded8ad 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2838,50 +2838,46 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { void insertBTI(BinaryBasicBlock &BB, MCInst &Call) const override { auto II = BB.getFirstNonPseudo(); - assert(II != BB.end() && - "insertBTI should only be called on non-empty BasicBlocks"); - // Make sure there is no crash in non-assertion builds when calling on empty - // BBs. - if (II != BB.end()) { - if (isBTIVariantCoveringCall(Call, *II)) - return; - // A BLR can be accepted by a BTI c. - 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 (isBTILandingPad(*II, false, true)) { - updateBTIVariant(*II, true, true); - } else { - MCInst BTIInst; - createBTI(BTIInst, true, false); - BB.insertInstruction(II, BTIInst); - } + // Only check the first instruction for non-empty BasicBlocks + bool Empty = (II == BB.end()); + if (!Empty && isBTIVariantCoveringCall(Call, *II)) + return; + // A BLR can be accepted by a BTI c. + 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); + } else { + MCInst BTIInst; + createBTI(BTIInst, true, false); + BB.insertInstruction(II, BTIInst); } + } - // 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 j or BTI jc (and not BTI c). - if (isIndirectBranch(Call)) { - assert(Call.getNumOperands() == 1 && - "Indirect branch needs to have 1 operand."); - assert(Call.getOperand(0).isReg() && - "Indirect branch does not have a register operand."); - MCPhysReg Reg = Call.getOperand(0).getReg(); - if (Reg == AArch64::X16 || Reg == AArch64::X17) { - // Add a new BTI c + // 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 j or BTI jc (and not BTI c). + if (isIndirectBranch(Call)) { + assert(Call.getNumOperands() == 1 && + "Indirect branch needs to have 1 operand."); + assert(Call.getOperand(0).isReg() && + "Indirect branch does not have a register operand."); + MCPhysReg Reg = Call.getOperand(0).getReg(); + if (Reg == AArch64::X16 || Reg == AArch64::X17) { + // Add a new BTI c + MCInst BTIInst; + createBTI(BTIInst, true, false); + 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); + } else { MCInst BTIInst; - createBTI(BTIInst, true, false); + createBTI(BTIInst, false, true); BB.insertInstruction(II, BTIInst); - } else { - // If BB starts with a BTI c, extend it to BTI jc, - // otherwise insert a new BTI j. - if (isBTILandingPad(*II, true, false)) { - updateBTIVariant(*II, true, true); - } else { - MCInst BTIInst; - createBTI(BTIInst, false, true); - BB.insertInstruction(II, BTIInst); - } } } } diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp index dd3704742c694..e8323e87fe148 100644 --- a/bolt/unittests/Core/MCPlusBuilder.cpp +++ b/bolt/unittests/Core/MCPlusBuilder.cpp @@ -204,13 +204,10 @@ TEST_P(MCPlusBuilderTester, AArch64_insertBTI_empty) { BinaryFunction *BF = BC->createInjectedBinaryFunction("BF", true); std::unique_ptr BB = BF->createBasicBlock(); MCInst CallInst = MCInstBuilder(AArch64::BR).addReg(AArch64::X16); -#ifndef NDEBUG - ASSERT_DEATH(BC->MIB->insertBTI(*BB, CallInst), - "insertBTI should only be called on non-empty BasicBlocks"); -#else BC->MIB->insertBTI(*BB, CallInst); - ASSERT(BB->size() == 0); -#endif + // Check that BTI c is added to the empty block. + auto II = BB->begin(); + ASSERT_TRUE(BC->MIB->isBTILandingPad(*II, true, false)); } TEST_P(MCPlusBuilderTester, AArch64_insertBTI_0) { if (GetParam() != Triple::aarch64) From 4ef7a5a2d4b5aa5f7116383c44f55f25c0683a5f Mon Sep 17 00:00:00 2001 From: Gergely Balint Date: Tue, 9 Dec 2025 11:54:40 +0000 Subject: [PATCH 5/5] [BOLT] Rename function to isCallCoveredByBTI - this way the order of the parameters matches the order in the name of the function --- bolt/include/bolt/Core/MCPlusBuilder.h | 2 +- bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index 4011fa2b557e6..6d0ba466347c1 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1896,7 +1896,7 @@ class MCPlusBuilder { /// Checks if the indirect call / jump is accepted by the landing pad at the /// start of the target BasicBlock. - virtual bool isBTIVariantCoveringCall(MCInst &Call, MCInst &Pad) const { + virtual bool isCallCoveredByBTI(MCInst &Call, MCInst &Pad) const { llvm_unreachable("not implemented"); return false; } diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index 601599bded8ad..eaaa42f93439d 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -2808,7 +2808,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createImm(HintNum)); } - bool isBTIVariantCoveringCall(MCInst &Call, MCInst &Pad) const override { + bool isCallCoveredByBTI(MCInst &Call, MCInst &Pad) const override { assert((isIndirectCall(Call) || isIndirectBranch(Call)) && "Not an indirect call or branch."); @@ -2840,7 +2840,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { auto II = BB.getFirstNonPseudo(); // Only check the first instruction for non-empty BasicBlocks bool Empty = (II == BB.end()); - if (!Empty && isBTIVariantCoveringCall(Call, *II)) + if (!Empty && isCallCoveredByBTI(Call, *II)) return; // A BLR can be accepted by a BTI c. if (isIndirectCall(Call)) {