Skip to content

Commit 8182c9b

Browse files
committed
[BOLT] Bugfix: CFIs can be placed before the first Instruction
- This caused a crash when trying to Annotate RAState-changing CFIs (RememberState, RestoreState, NegateRAState). - The fix introduces an InitialRAState for each BinaryFunction. - If we have a NegateRAState before the first Instr, we set that to True. - In MarkRAStates, we push the InitialRAState to the RAStateStack: as we may have omitted the RememberState at the function start, its RestoreState pair would try to pop an empty stack otherwise.
1 parent 9e19f0a commit 8182c9b

File tree

3 files changed

+23
-6
lines changed

3 files changed

+23
-6
lines changed

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ class BinaryFunction {
148148
PF_MEMEVENT = 4, /// Profile has mem events.
149149
};
150150

151+
void setInitialRAState(bool State) { InitialRAState = State; }
152+
bool getInitialRAState() { return InitialRAState; }
153+
151154
/// Struct for tracking exception handling ranges.
152155
struct CallSite {
153156
const MCSymbol *Start;
@@ -221,6 +224,8 @@ class BinaryFunction {
221224
/// Current state of the function.
222225
State CurrentState{State::Empty};
223226

227+
bool InitialRAState{false};
228+
224229
/// A list of symbols associated with the function entry point.
225230
///
226231
/// Multiple symbols would typically result from identical code-folding

bolt/lib/Core/Exceptions.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -569,20 +569,24 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
569569
Function.addCFIInstruction(
570570
Offset, MCCFIInstruction::createRememberState(nullptr));
571571

572-
if (Function.getBinaryContext().isAArch64())
572+
if (Function.getBinaryContext().isAArch64()) {
573573
// Support for pointer authentication:
574574
// We need to annotate instructions that modify the RA State, to work
575575
// out the state of each instruction in MarkRAStates Pass.
576-
Function.setInstModifiesRAState(DW_CFA_remember_state, Offset);
576+
if (Offset != 0)
577+
Function.setInstModifiesRAState(DW_CFA_remember_state, Offset);
578+
}
577579
break;
578580
case DW_CFA_restore_state:
579581
Function.addCFIInstruction(Offset,
580582
MCCFIInstruction::createRestoreState(nullptr));
581-
if (Function.getBinaryContext().isAArch64())
583+
if (Function.getBinaryContext().isAArch64()) {
582584
// Support for pointer authentication:
583585
// We need to annotate instructions that modify the RA State, to work
584586
// out the state of each instruction in MarkRAStates Pass.
585-
Function.setInstModifiesRAState(DW_CFA_restore_state, Offset);
587+
if (Offset != 0)
588+
Function.setInstModifiesRAState(DW_CFA_restore_state, Offset);
589+
}
586590
break;
587591
case DW_CFA_def_cfa:
588592
Function.addCFIInstruction(
@@ -649,7 +653,14 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
649653
// is added to the instruction, to mark that the instruction modifies
650654
// the RA State. The actual state for instructions are worked out in
651655
// MarkRAStates based on these annotations.
652-
Function.setInstModifiesRAState(DW_CFA_AARCH64_negate_ra_state, Offset);
656+
if (Offset != 0)
657+
Function.setInstModifiesRAState(DW_CFA_AARCH64_negate_ra_state,
658+
Offset);
659+
else
660+
// We cannot Annotate an instruction at Offset == 0.
661+
// Instead, we save the initial (Signed) state, and push it to
662+
// MarkRAStates' RAStateStack.
663+
Function.setInitialRAState(true);
653664
break;
654665
}
655666
if (opts::Verbosity >= 1)

bolt/lib/Passes/MarkRAStates.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
6262
}
6363
}
6464

65-
bool RAState = false;
65+
bool RAState = BF.getInitialRAState();
6666
std::stack<bool> RAStateStack;
67+
RAStateStack.push(RAState);
6768

6869
for (BinaryBasicBlock &BB : BF) {
6970
for (auto It = BB.begin(); It != BB.end(); ++It) {

0 commit comments

Comments
 (0)