Skip to content

Commit f9d31ca

Browse files
committed
InstructionDeleter ctor moves callbacks to fix iterator invalidation
The InstructionDeleter needs to move the callbacks during construction to prevent the client code from using the old callbacks. Fixes iterator invalidation bugs.
1 parent c86d112 commit f9d31ca

File tree

4 files changed

+16
-14
lines changed

4 files changed

+16
-14
lines changed

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1250,7 +1250,7 @@ static bool tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
12501250
}
12511251
(void)deletedInstructions.insert(deadInst);
12521252
});
1253-
InstructionDeleter deleter(callbacks);
1253+
InstructionDeleter deleter(std::move(callbacks));
12541254

12551255
unsigned startIndex = 0;
12561256
while (startIndex < worklist.size()) {

lib/SILOptimizer/Transforms/AccessEnforcementOpts.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,7 @@ static bool extendOwnership(BeginAccessInst *parentInst,
10281028
static bool
10291029
mergeAccesses(SILFunction *F, PostDominanceInfo *postDomTree,
10301030
const AccessConflictAndMergeAnalysis::MergeablePairs &mergePairs,
1031-
InstModCallbacks &callbacks, DeadEndBlocks &deBlocks) {
1031+
DeadEndBlocks &deBlocks) {
10321032

10331033
if (mergePairs.empty()) {
10341034
LLVM_DEBUG(llvm::dbgs() << "Skipping SCC Analysis...\n");
@@ -1064,7 +1064,7 @@ mergeAccesses(SILFunction *F, PostDominanceInfo *postDomTree,
10641064
// begin_access instruction. We store (begin_access %2 -> begin_access %1)
10651065
// to re-map a merged begin_access to it's replaced instruction.
10661066
llvm::DenseMap<BeginAccessInst *, BeginAccessInst *> oldToNewMap;
1067-
InstructionDeleter deleter(callbacks);
1067+
InstructionDeleter deleter;
10681068

10691069
while (!workPairs.empty()) {
10701070
auto curr = workPairs.pop_back_val();
@@ -1162,8 +1162,7 @@ struct AccessEnforcementOpts : public SILFunctionTransform {
11621162
getAnalysis<PostDominanceAnalysis>();
11631163
PostDominanceInfo *postDomTree = postDomAnalysis->get(F);
11641164
DeadEndBlocks *deBlocks = deBlocksAnalysis->get(F);
1165-
InstModCallbacks callbacks;
1166-
if (mergeAccesses(F, postDomTree, result.mergePairs, callbacks, *deBlocks))
1165+
if (mergeAccesses(F, postDomTree, result.mergePairs, *deBlocks))
11671166
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
11681167
}
11691168
};

lib/SILOptimizer/Utils/ConstantFolding.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,6 +1654,7 @@ ConstantFolder::processWorkList() {
16541654
InvalidateInstructions = true;
16551655
instToDelete->eraseFromParent();
16561656
});
1657+
InstructionDeleter deleter(std::move(callbacks));
16571658

16581659
// An out parameter array that we use to return new simplified results from
16591660
// constantFoldInstruction.
@@ -1677,7 +1678,7 @@ ConstantFolder::processWorkList() {
16771678
// Schedule users for constant folding.
16781679
WorkList.insert(AssertConfInt);
16791680
// Delete the call.
1680-
eliminateDeadInstruction(BI, callbacks);
1681+
eliminateDeadInstruction(BI, deleter.getCallbacks());
16811682
continue;
16821683
}
16831684

@@ -1686,7 +1687,7 @@ ConstantFolder::processWorkList() {
16861687
if (isApplyOfBuiltin(*BI, BuiltinValueKind::CondUnreachable)) {
16871688
assert(BI->use_empty() && "use of conditionallyUnreachable?!");
16881689
recursivelyDeleteTriviallyDeadInstructions(BI, /*force*/ true,
1689-
callbacks);
1690+
deleter.getCallbacks());
16901691
InvalidateInstructions = true;
16911692
continue;
16921693
}
@@ -1698,7 +1699,7 @@ ConstantFolder::processWorkList() {
16981699
SILBuilderWithScope B(I);
16991700
auto tru = B.createIntegerLiteral(apply->getLoc(), apply->getType(), 1);
17001701
apply->replaceAllUsesWith(tru);
1701-
eliminateDeadInstruction(I, callbacks);
1702+
eliminateDeadInstruction(I, deleter.getCallbacks());
17021703
WorkList.insert(tru);
17031704
InvalidateInstructions = true;
17041705
}
@@ -1747,7 +1748,7 @@ ConstantFolder::processWorkList() {
17471748
if (constantFoldGlobalStringTablePointerBuiltin(cast<BuiltinInst>(I),
17481749
EnableDiagnostics)) {
17491750
// Here, the builtin instruction got folded, so clean it up.
1750-
eliminateDeadInstruction(I, callbacks);
1751+
eliminateDeadInstruction(I, deleter.getCallbacks());
17511752
}
17521753
continue;
17531754
}
@@ -1763,7 +1764,7 @@ ConstantFolder::processWorkList() {
17631764
sli->getValue());
17641765
WorkList.insert(cfi);
17651766
recursivelyDeleteTriviallyDeadInstructions(I, /*force*/ true,
1766-
callbacks);
1767+
deleter.getCallbacks());
17671768
}
17681769
}
17691770
continue;
@@ -1773,7 +1774,7 @@ ConstantFolder::processWorkList() {
17731774
if (constantFoldIsConcrete(cast<BuiltinInst>(I))) {
17741775
// Here, the builtin instruction got folded, so clean it up.
17751776
recursivelyDeleteTriviallyDeadInstructions(I, /*force*/ true,
1776-
callbacks);
1777+
deleter.getCallbacks());
17771778
}
17781779
continue;
17791780
}
@@ -1796,7 +1797,7 @@ ConstantFolder::processWorkList() {
17961797
SILType::getBuiltinIntegerType(1, builder.getASTContext()), val);
17971798
BI->replaceAllUsesWith(inst);
17981799

1799-
eliminateDeadInstruction(I, callbacks);
1800+
eliminateDeadInstruction(I, deleter.getCallbacks());
18001801
continue;
18011802
}
18021803
}
@@ -1817,7 +1818,6 @@ ConstantFolder::processWorkList() {
18171818
}
18181819

18191820
// Go through all users of the constant and try to fold them.
1820-
InstructionDeleter deleter(callbacks);
18211821
for (auto Result : I->getResults()) {
18221822
for (auto *Use : Result->getUses()) {
18231823
SILInstruction *User = Use->getUser();

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,10 @@ extendOverBorrowScopeAndConsume(SILValue ownedValue) {
555555
// Populate the reborrowedPhis vector.
556556
analyzeExtendedScope();
557557

558-
InstructionDeleter deleter(callbacks);
558+
// Warning: don't use the original callbacks in this function after creating a
559+
// deleter.
560+
InstModCallbacks tempCallbacks = callbacks;
561+
InstructionDeleter deleter(std::move(tempCallbacks));
559562

560563
// Generate and map the phis with undef operands first, in case of recursion.
561564
auto undef = SILUndef::get(ownedValue->getType(), *ownedValue->getFunction());

0 commit comments

Comments
 (0)