Skip to content

Commit e1bb059

Browse files
authored
[MachineBasicBlock] Fix use after free in SplitCriticalEdge (#68786)
Remove use after free when attempting to update SlotIndexes in MachineBasicBlock::SplitCriticalEdge. Use MachineFunction delegate mechanism to capture target specific manipulations of branch instructions and update SlotIndexes.
1 parent fad99d3 commit e1bb059

File tree

1 file changed

+27
-31
lines changed

1 file changed

+27
-31
lines changed

llvm/lib/CodeGen/MachineBasicBlock.cpp

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,30 @@ static bool jumpTableHasOtherUses(const MachineFunction &MF,
10971097
return false;
10981098
}
10991099

1100+
class SlotIndexUpdateDelegate : public MachineFunction::Delegate {
1101+
private:
1102+
MachineFunction &MF;
1103+
SlotIndexes *Indexes;
1104+
1105+
public:
1106+
SlotIndexUpdateDelegate(MachineFunction &MF, SlotIndexes *Indexes)
1107+
: MF(MF), Indexes(Indexes) {
1108+
MF.setDelegate(this);
1109+
}
1110+
1111+
~SlotIndexUpdateDelegate() { MF.resetDelegate(this); }
1112+
1113+
void MF_HandleInsertion(MachineInstr &MI) override {
1114+
if (Indexes)
1115+
Indexes->insertMachineInstrInMaps(MI);
1116+
}
1117+
1118+
void MF_HandleRemoval(MachineInstr &MI) override {
1119+
if (Indexes)
1120+
Indexes->removeMachineInstrFromMaps(MI);
1121+
}
1122+
};
1123+
11001124
MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
11011125
MachineBasicBlock *Succ, Pass &P,
11021126
std::vector<SparseBitVector<>> *LiveInSets) {
@@ -1170,51 +1194,23 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
11701194

11711195
ReplaceUsesOfBlockWith(Succ, NMBB);
11721196

1173-
// If updateTerminator() removes instructions, we need to remove them from
1174-
// SlotIndexes.
1175-
SmallVector<MachineInstr*, 4> Terminators;
1176-
if (Indexes) {
1177-
for (MachineInstr &MI :
1178-
llvm::make_range(getFirstInstrTerminator(), instr_end()))
1179-
Terminators.push_back(&MI);
1180-
}
1181-
11821197
// Since we replaced all uses of Succ with NMBB, that should also be treated
11831198
// as the fallthrough successor
11841199
if (Succ == PrevFallthrough)
11851200
PrevFallthrough = NMBB;
11861201

1187-
if (!ChangedIndirectJump)
1202+
if (!ChangedIndirectJump) {
1203+
SlotIndexUpdateDelegate SlotUpdater(*MF, Indexes);
11881204
updateTerminator(PrevFallthrough);
1189-
1190-
if (Indexes) {
1191-
SmallVector<MachineInstr*, 4> NewTerminators;
1192-
for (MachineInstr &MI :
1193-
llvm::make_range(getFirstInstrTerminator(), instr_end()))
1194-
NewTerminators.push_back(&MI);
1195-
1196-
for (MachineInstr *Terminator : Terminators) {
1197-
if (!is_contained(NewTerminators, Terminator))
1198-
Indexes->removeMachineInstrFromMaps(*Terminator);
1199-
}
12001205
}
12011206

12021207
// Insert unconditional "jump Succ" instruction in NMBB if necessary.
12031208
NMBB->addSuccessor(Succ);
12041209
if (!NMBB->isLayoutSuccessor(Succ)) {
1210+
SlotIndexUpdateDelegate SlotUpdater(*MF, Indexes);
12051211
SmallVector<MachineOperand, 4> Cond;
12061212
const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
12071213
TII->insertBranch(*NMBB, Succ, nullptr, Cond, DL);
1208-
1209-
if (Indexes) {
1210-
for (MachineInstr &MI : NMBB->instrs()) {
1211-
// Some instructions may have been moved to NMBB by updateTerminator(),
1212-
// so we first remove any instruction that already has an index.
1213-
if (Indexes->hasIndex(MI))
1214-
Indexes->removeMachineInstrFromMaps(MI);
1215-
Indexes->insertMachineInstrInMaps(MI);
1216-
}
1217-
}
12181214
}
12191215

12201216
// Fix PHI nodes in Succ so they refer to NMBB instead of this.

0 commit comments

Comments
 (0)