Skip to content

Commit

Permalink
[BOLT] Fix handling of RememberState CFI
Browse files Browse the repository at this point in the history
Summary:
When RememberState CFI happens to be the last CFI in a basic block, we
used to set the state of the next basic block to a CFI prior to
executing RememberState instruction. This contradicts comments in
annotateCFIState() function and also differs form behaviour of
getCFIStateAtInstr(). As a result we were getting code like the
following:

  .LBB0121166 (21 instructions, align : 1)
    CFI State : 0
    ....
      0000001a:   !CFI    $1      ; OpOffset Reg6 -16
      0000001a:   !CFI    $2      ; OpRememberState
    ....
    Successors: .Ltmp4167600, .Ltmp4167601
    CFI State: 3

  .Ltmp4167601 (13 instructions, align : 1)
    CFI State : 2
    ....

Notice that the state at the entry of the 2nd basic block is less than
the state at the exit of the previous basic block.

In practice we have never seen basic blocks where RememberState was the
last CFI instruction in the basic block, and hence we've never run into
this issue before.

The fix is a synchronization of handling of last RememberState
instruction by annotateCFIState() and getCFIStateAtInstr().
In the example above, the CFI state at the entry to the second BB will
be 3 after this diff.

(cherry picked from FBD6314916)
  • Loading branch information
maksfb authored and memfrob committed Oct 4, 2022
1 parent 5ebdfed commit 30cc9b6
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions bolt/BinaryFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2344,6 +2344,7 @@ void BinaryFunction::annotateCFIState() {
switch (CFI->getOperation()) {
case MCCFIInstruction::OpRememberState:
StateStack.push(EffectiveState);
EffectiveState = State;
break;
case MCCFIInstruction::OpRestoreState:
assert(!StateStack.empty() && "corrupt CFI stack");
Expand Down

0 comments on commit 30cc9b6

Please sign in to comment.