Skip to content

[BOLT][AArch64] Support for pointer authentication #117578

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

Closed
wants to merge 7 commits into from
Closed

[BOLT][AArch64] Support for pointer authentication #117578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
61efea9
[BOLT] Recognize paciasp and autiasp instructions
bgergely0 Nov 19, 2024
0353abb
[BOLT] Support for OpNegateRAState - first half
bgergely0 Nov 25, 2024
6217b66
[BOLT] Support for OpNegateRAState - second half
bgergely0 Nov 25, 2024
d5fae5e
[GadgetScanner/pacret] Handle noreturn functions.
kbeyls Sep 25, 2023
6b7957a
Fix to build with "handle noret"
bgergely0 Nov 7, 2024
e0c0957
Remove bolt-gadget-scanner specific test
bgergely0 Nov 26, 2024
15d008f
[BOLT] InsertNegateRAStatePass: change CFG error conditions
bgergely0 Nov 26, 2024
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
16 changes: 16 additions & 0 deletions bolt/include/bolt/Core/BinaryBasicBlock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ class JumpTable;

class BinaryBasicBlock {
public:
enum class RAStateEnum : char {
Unknown, /// Not discovered yet
Signed,
Unsigned,
};
/// Profile execution information for a given edge in CFG.
///
/// If MispredictedCount equals COUNT_INFERRED, then we have a profile
Expand Down Expand Up @@ -350,6 +355,17 @@ class BinaryBasicBlock {
BranchInfo.end());
}

RAStateEnum RAState{RAStateEnum::Unknown};
void setRASigned() { RAState = RAStateEnum::Signed; }
bool isRAStateUnknown() { return RAState == RAStateEnum::Unknown; }
bool isRAStateSigned() { return RAState == RAStateEnum::Signed; }
/// Unsigned should only overwrite Unknown state, and not Signed
void setRAUnsigned() {
if (RAState == RAStateEnum::Unknown) {
RAState = RAStateEnum::Unsigned;
}
}

/// Get instruction at given index.
MCInst &getInstructionAtIndex(unsigned Index) { return Instructions[Index]; }

Expand Down
30 changes: 30 additions & 0 deletions bolt/include/bolt/Core/MCPlusBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "bolt/Core/MCPlus.h"
#include "bolt/Core/Relocation.h"
#include "bolt/Utils/CommandLineOpts.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/StringMap.h"
Expand All @@ -27,6 +28,7 @@
#include "llvm/MC/MCInstrAnalysis.h"
#include "llvm/MC/MCInstrDesc.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
Expand Down Expand Up @@ -546,6 +548,27 @@ class MCPlusBuilder {
return Analysis->isCall(Inst) || isTailCall(Inst);
}

virtual std::optional<StringRef> getCalleeName(const MCInst &Inst) const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could move implementation to the cpp file (of getCalleeName and isNoReturnCall) if we want to avoid including more headers earlier in this file.

assert(isCall(Inst));
if (MCPlus::getNumPrimeOperands(Inst) != 1 || !Inst.getOperand(0).isExpr())
return {};

const MCSymbol *CalleeSymbol = getTargetSymbol(Inst);
assert(CalleeSymbol != nullptr);
return CalleeSymbol->getName();
}

virtual bool isNoReturnCall(const MCInst &Inst) const {
if (!isCall(Inst))
return false;
auto calleeName = getCalleeName(Inst);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: start with uppercase

if (calleeName)
for (std::string &Name : opts::AssumeNoReturnFunctions)
if (calleeName->equals(Name))
return true;
return false;
}

virtual bool isReturn(const MCInst &Inst) const {
return Analysis->isReturn(Inst);
}
Expand Down Expand Up @@ -648,6 +671,13 @@ class MCPlusBuilder {
llvm_unreachable("not implemented");
return false;
}
virtual bool isPAuth(MCInst &Inst) const {
llvm_unreachable("not implemented");
}

virtual bool isPSign(MCInst &Inst) const {
llvm_unreachable("not implemented");
}

virtual bool isCleanRegXOR(const MCInst &Inst) const {
llvm_unreachable("not implemented");
Expand Down
33 changes: 33 additions & 0 deletions bolt/include/bolt/Passes/InsertNegateRAStatePass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#ifndef BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
#define BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS

#include "bolt/Passes/BinaryPasses.h"
#include <stack>

namespace llvm {
namespace bolt {

class InsertNegateRAState : public BinaryFunctionPass {
public:
explicit InsertNegateRAState() : BinaryFunctionPass(false) {}

const char *getName() const override { return "insert-negate-ra-state-pass"; }

/// Pass entry point
Error runOnFunctions(BinaryContext &BC) override;
void runOnFunction(BinaryFunction &BF);
bool addNegateRAStateAfterPacOrAuth(BinaryFunction &BF);
bool BBhasAUTH(BinaryContext &BC, BinaryBasicBlock *BB);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use lower-camelcase for some relevant functions below?
like blockHasAuth or blockHasAUTH, whichever seems more suitable.

Consider making them private if they are not used outiside this pass.

Also it'll be great if you can provide some short description to some of these,
especially for the explore/process, and addNegateRAStateAfterPacOrAuth functions.

bool BBhasSIGN(BinaryContext &BC, BinaryBasicBlock *BB);
void explore_call_graph(BinaryContext &BC, BinaryBasicBlock *BB);
void process_signed_BB(BinaryContext &BC, BinaryBasicBlock *BB,
std::stack<BinaryBasicBlock *> *SignedStack,
std::stack<BinaryBasicBlock *> *UnsignedStack);
void process_unsigned_BB(BinaryContext &BC, BinaryBasicBlock *BB,
std::stack<BinaryBasicBlock *> *SignedStack,
std::stack<BinaryBasicBlock *> *UnsignedStack);
};

} // namespace bolt
} // namespace llvm
#endif
7 changes: 7 additions & 0 deletions bolt/include/bolt/Utils/CommandLineOpts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ extern llvm::cl::opt<bool> TimeOpts;
extern llvm::cl::opt<bool> UseOldText;
extern llvm::cl::opt<bool> UpdateDebugSections;

extern llvm::cl::list<std::string> AssumeNoReturnFunctions;
extern llvm::cl::opt<std::string> AssumeNoReturnFunctionsFile;

/// Reads names from FunctionNamesFile and adds them to FunctionNames.
void populateFunctionNames(const llvm::cl::opt<std::string> &FunctionNamesFile,
llvm::cl::list<std::string> &FunctionNames);

// The default verbosity level (0) is pretty terse, level 1 is fairly
// verbose and usually prints some informational message for every
// function processed. Level 2 is for the noisiest of messages and
Expand Down
6 changes: 5 additions & 1 deletion bolt/lib/Core/BinaryBasicBlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,11 @@ int32_t BinaryBasicBlock::getCFIStateAtInstr(const MCInst *Instr) const {
InstrSeen = (&Inst == Instr);
continue;
}
if (Function->getBinaryContext().MIB->isCFI(Inst)) {
// Fix: ignoring OpNegateRAState CFIs here, as they dont have a "State"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this handled later on? Can you reword your comment to something like

// Ignoring (all the cases here..). ABC will handle Negate-RA case?

// number associated with them.
if (Function->getBinaryContext().MIB->isCFI(Inst) &&
(Function->getCFIFor(Inst)->getOperation() !=
MCCFIInstruction::OpNegateRAState)) {
LastCFI = &Inst;
break;
}
Expand Down
11 changes: 8 additions & 3 deletions bolt/lib/Core/BinaryFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2155,7 +2155,8 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
addCFIPlaceholders(Offset, InsertBB);
}

const bool IsBlockEnd = MIB->isTerminator(Instr);
const bool IsBlockEnd =
MIB->isTerminator(Instr) || MIB->isNoReturnCall(Instr);
IsLastInstrNop = MIB->isNoop(Instr);
if (!IsLastInstrNop)
LastInstrOffset = Offset;
Expand Down Expand Up @@ -2242,8 +2243,11 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
//
// Conditional tail call is a special case since we don't add a taken
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adapting this earlier comment to mention this new special case?

// branch successor for it.
IsPrevFT = !MIB->isTerminator(*LastInstr) ||
MIB->getConditionalTailCall(*LastInstr);
if (MIB->isNoReturnCall(*LastInstr))
IsPrevFT = false;
else
IsPrevFT = !MIB->isTerminator(*LastInstr) ||
MIB->getConditionalTailCall(*LastInstr);
} else if (BB->succ_size() == 1) {
IsPrevFT = MIB->isConditionalBranch(*LastInstr);
} else {
Expand Down Expand Up @@ -2596,6 +2600,7 @@ struct CFISnapshot {
void advanceTo(int32_t State) {
for (int32_t I = CurState, E = State; I != E; ++I) {
const MCCFIInstruction &Instr = FDE[I];
assert(Instr.getOperation() != MCCFIInstruction::OpNegateRAState);
if (Instr.getOperation() != MCCFIInstruction::OpRestoreState) {
update(Instr, I);
continue;
Expand Down
6 changes: 4 additions & 2 deletions bolt/lib/Core/Exceptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,10 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
// DW_CFA_GNU_window_save and DW_CFA_GNU_NegateRAState just use the same
// id but mean different things. The latter is used in AArch64.
if (Function.getBinaryContext().isAArch64()) {
Function.addCFIInstruction(
Offset, MCCFIInstruction::createNegateRAState(nullptr));
// Fix: not adding OpNegateRAState since the location they are needed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can modify/expand the earlier comment (line 632) to explain this one, dropping the word 'Fix'.

// depends on the order of BasicBlocks, which changes during
// optimizations. They are generated in InsertNegateRAStatePass after
// optimizations instead.
break;
}
if (opts::Verbosity >= 1)
Expand Down
26 changes: 12 additions & 14 deletions bolt/lib/Passes/BinaryPasses.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1852,17 +1852,16 @@ Error InlineMemcpy::runOnFunctions(BinaryContext &BC) {
for (auto II = BB.begin(); II != BB.end(); ++II) {
MCInst &Inst = *II;

if (!BC.MIB->isCall(Inst) || MCPlus::getNumPrimeOperands(Inst) != 1 ||
!Inst.getOperand(0).isExpr())
if (!BC.MIB->isCall(Inst))
continue;

const MCSymbol *CalleeSymbol = BC.MIB->getTargetSymbol(Inst);
if (CalleeSymbol->getName() != "memcpy" &&
CalleeSymbol->getName() != "memcpy@PLT" &&
CalleeSymbol->getName() != "_memcpy8")
std::optional<StringRef> CalleeName = BC.MIB->getCalleeName(Inst);
if (!CalleeName)
continue;
if (*CalleeName != "memcpy" && *CalleeName != "memcpy@PLT" &&
*CalleeName != "_memcpy8")
continue;

const bool IsMemcpy8 = (CalleeSymbol->getName() == "_memcpy8");
const bool IsMemcpy8 = (*CalleeName == "_memcpy8");
const bool IsTailCall = BC.MIB->isTailCall(Inst);

const InstructionListType NewCode =
Expand Down Expand Up @@ -1951,13 +1950,12 @@ Error SpecializeMemcpy1::runOnFunctions(BinaryContext &BC) {
for (auto II = CurBB->begin(); II != CurBB->end(); ++II) {
MCInst &Inst = *II;

if (!BC.MIB->isCall(Inst) || MCPlus::getNumPrimeOperands(Inst) != 1 ||
!Inst.getOperand(0).isExpr())
if (!BC.MIB->isCall(Inst))
continue;

const MCSymbol *CalleeSymbol = BC.MIB->getTargetSymbol(Inst);
if (CalleeSymbol->getName() != "memcpy" &&
CalleeSymbol->getName() != "memcpy@PLT")
std::optional<StringRef> CalleeName = BC.MIB->getCalleeName(Inst);
if (!CalleeName)
continue;
if (*CalleeName != "memcpy" && *CalleeName != "memcpy@PLT")
continue;

if (BC.MIB->isTailCall(Inst))
Expand Down
1 change: 1 addition & 0 deletions bolt/lib/Passes/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ add_llvm_library(LLVMBOLTPasses
IdenticalCodeFolding.cpp
IndirectCallPromotion.cpp
Inliner.cpp
InsertNegateRAStatePass.cpp
Instrumentation.cpp
JTFootprintReduction.cpp
LongJmp.cpp
Expand Down
Loading
Loading