[llvm] [BOLT] Fix iterator bugs (PR #190978)
Brian Cain via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 10 07:14:37 PDT 2026
https://github.com/androm3da updated https://github.com/llvm/llvm-project/pull/190978
>From 10f4710982ef8a2f9b14e24add2e652c67167717 Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Wed, 8 Apr 2026 06:59:53 -0700
Subject: [PATCH 1/4] [BOLT] Fix iterator invalidation in AllocCombiner
combineAdjustments() erases instructions while iterating in reverse
over the basic block with llvm::reverse(BB). This invalidates the
reverse iterator, which _GLIBCXX_DEBUG detects as "attempt to
decrement a singular iterator".
Defer all erasures until after the reverse iteration completes by
collecting them in a SmallVector and erasing afterward.
---
bolt/lib/Passes/AllocCombiner.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Passes/AllocCombiner.cpp b/bolt/lib/Passes/AllocCombiner.cpp
index 38ef7d02a47d9..ea620bb82184d 100644
--- a/bolt/lib/Passes/AllocCombiner.cpp
+++ b/bolt/lib/Passes/AllocCombiner.cpp
@@ -64,6 +64,7 @@ static void runForAllWeCare(std::map<uint64_t, BinaryFunction> &BFs,
void AllocCombinerPass::combineAdjustments(BinaryFunction &BF) {
BinaryContext &BC = BF.getBinaryContext();
for (BinaryBasicBlock &BB : BF) {
+ SmallVector<MCInst *, 2> ToErase;
MCInst *Prev = nullptr;
for (MCInst &Inst : llvm::reverse(BB)) {
if (isIndifferentToSP(Inst, BC))
@@ -94,12 +95,14 @@ void AllocCombinerPass::combineAdjustments(BinaryFunction &BF) {
Inst.dump();
});
- BB.eraseInstruction(BB.findInstruction(Prev));
+ ToErase.push_back(Prev);
++NumCombined;
DynamicCountCombined += BB.getKnownExecutionCount();
FuncsChanged.insert(&BF);
Prev = &Inst;
}
+ for (MCInst *Inst : ToErase)
+ BB.eraseInstruction(BB.findInstruction(Inst));
}
}
>From 2cd49b2f927ec66c704c3df500c0e373e3969cf9 Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Wed, 8 Apr 2026 07:01:36 -0700
Subject: [PATCH 2/4] [BOLT] Fix iterator bugs in ShrinkWrapping and
DataflowAnalysis
Two related fixes for _GLIBCXX_DEBUG crashes:
ShrinkWrapping::processDeletions() used
std::prev(BB.eraseInstruction(II)) which crashes when II == begin()
because std::prev(begin()) is undefined. Restructure to use
standard forward iteration with erase: eraseInstruction returns
the next valid iterator, so manual increment is only needed for
non-erased instructions.
DataflowAnalysis::run() unconditionally dereferences BB->rbegin()
to get the last instruction. After the ShrinkWrapping fix, basic
blocks can be empty (all instructions erased), making this a
dereference of rbegin() on an empty container. Guard with an
emptiness check.
---
bolt/include/bolt/Passes/DataflowAnalysis.h | 10 ++++++----
bolt/lib/Passes/ShrinkWrapping.cpp | 12 +++++++++---
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/bolt/include/bolt/Passes/DataflowAnalysis.h b/bolt/include/bolt/Passes/DataflowAnalysis.h
index 3b182abb231fc..41b7667a5c9db 100644
--- a/bolt/include/bolt/Passes/DataflowAnalysis.h
+++ b/bolt/include/bolt/Passes/DataflowAnalysis.h
@@ -404,10 +404,12 @@ class DataflowAnalysis {
// Propagate information from first instruction down to the last one
StateTy *PrevState = &St;
const MCInst *LAST = nullptr;
- if (!Backward)
- LAST = &*BB->rbegin();
- else
- LAST = &*BB->begin();
+ if (!BB->empty()) {
+ if (!Backward)
+ LAST = &*BB->rbegin();
+ else
+ LAST = &*BB->begin();
+ }
auto doNext = [&](MCInst &Inst, const BinaryBasicBlock &BB) {
StateTy CurState = derived().computeNext(Inst, *PrevState);
diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp
index b882e2512866d..5f0af2d68182f 100644
--- a/bolt/lib/Passes/ShrinkWrapping.cpp
+++ b/bolt/lib/Passes/ShrinkWrapping.cpp
@@ -1887,13 +1887,16 @@ Expected<bool> ShrinkWrapping::processInsertions() {
void ShrinkWrapping::processDeletions() {
LivenessAnalysis &LA = Info.getLivenessAnalysis();
for (BinaryBasicBlock &BB : BF) {
- for (auto II = BB.begin(); II != BB.end(); ++II) {
+ for (auto II = BB.begin(); II != BB.end();) {
MCInst &Inst = *II;
auto TodoList = BC.MIB->tryGetAnnotationAs<std::vector<WorklistItem>>(
Inst, getAnnotationIndex());
- if (!TodoList)
+ if (!TodoList) {
+ ++II;
continue;
+ }
// Process all deletions
+ bool Erased = false;
for (WorklistItem &Item : *TodoList) {
if (Item.Action != WorklistItem::Erase &&
Item.Action != WorklistItem::ChangeToAdjustment)
@@ -1916,9 +1919,12 @@ void ShrinkWrapping::processDeletions() {
dbgs() << "Erasing: ";
BC.printInstruction(dbgs(), Inst);
});
- II = std::prev(BB.eraseInstruction(II));
+ II = BB.eraseInstruction(II);
+ Erased = true;
break;
}
+ if (!Erased)
+ ++II;
}
}
}
>From 62e838ffa1b1446ffbfa4d2498ab4229e80cd308 Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Wed, 8 Apr 2026 07:01:43 -0700
Subject: [PATCH 3/4] [BOLT] Fix end iterator dereference in
IndirectCallPromotion
rewriteCall() computes a past-the-end pointer via
&(*IndCallBlock.end()), which dereferences the end() iterator.
With _GLIBCXX_DEBUG this is caught as "attempt to dereference a
past-the-end iterator".
Replace with an equivalent bounds check using &IndCallBlock.back()
which avoids the end() dereference while preserving the same
semantics: the loop advances TailInst only while it is not the
last instruction in the block.
---
bolt/lib/Passes/IndirectCallPromotion.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/bolt/lib/Passes/IndirectCallPromotion.cpp b/bolt/lib/Passes/IndirectCallPromotion.cpp
index 8a01cb974c5da..6e5d041468abd 100644
--- a/bolt/lib/Passes/IndirectCallPromotion.cpp
+++ b/bolt/lib/Passes/IndirectCallPromotion.cpp
@@ -775,8 +775,7 @@ IndirectCallPromotion::rewriteCall(
InstructionListType TailInsts;
const MCInst *TailInst = &CallInst;
if (IsTailCallOrJT)
- while (TailInst + 1 < &(*IndCallBlock.end()) &&
- MIB->isPseudo(*(TailInst + 1)))
+ while (TailInst != &IndCallBlock.back() && MIB->isPseudo(*(TailInst + 1)))
TailInsts.push_back(*++TailInst);
InstructionListType MovedInst = IndCallBlock.splitInstructions(&CallInst);
>From 1831f8b15feb16c0fc4123cb0db66e5d5755aa1d Mon Sep 17 00:00:00 2001
From: Brian Cain <brian.cain at oss.qualcomm.com>
Date: Wed, 8 Apr 2026 07:02:16 -0700
Subject: [PATCH 4/4] [BOLT] Fix begin iterator decrement in TailDuplication
constantAndCopyPropagate() used
std::prev(OriginalBB.eraseInstruction(Itr)) which crashes when
Itr == begin() because std::prev(begin()) is undefined. With
_GLIBCXX_DEBUG this is caught as "attempt to decrement a
start-of-sequence iterator".
Restructure to use standard forward iteration with erase: move
the loop increment into the body so eraseInstruction's returned
iterator is used directly and no decrement is needed.
---
bolt/lib/Passes/TailDuplication.cpp | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/bolt/lib/Passes/TailDuplication.cpp b/bolt/lib/Passes/TailDuplication.cpp
index e3ecf94a85070..c5565fdf4a7a7 100644
--- a/bolt/lib/Passes/TailDuplication.cpp
+++ b/bolt/lib/Passes/TailDuplication.cpp
@@ -167,18 +167,22 @@ void TailDuplication::constantAndCopyPropagate(
BlocksToPropagate.insert(BlocksToPropagate.begin(), &OriginalBB);
// Iterate through the original instructions to find one to propagate
- for (auto Itr = OriginalBB.begin(); Itr != OriginalBB.end(); ++Itr) {
+ for (auto Itr = OriginalBB.begin(); Itr != OriginalBB.end();) {
MCInst &OriginalInst = *Itr;
// It must be a non conditional
- if (BC.MIB->isConditionalMove(OriginalInst))
+ if (BC.MIB->isConditionalMove(OriginalInst)) {
+ ++Itr;
continue;
+ }
// Move immediate or move register
if ((!BC.MII->get(OriginalInst.getOpcode()).isMoveImmediate() ||
!OriginalInst.getOperand(1).isImm()) &&
(!BC.MII->get(OriginalInst.getOpcode()).isMoveReg() ||
- !OriginalInst.getOperand(1).isReg()))
+ !OriginalInst.getOperand(1).isReg())) {
+ ++Itr;
continue;
+ }
// True if this is constant propagation and not copy propagation
bool ConstantProp = BC.MII->get(OriginalInst.getOpcode()).isMoveImmediate();
@@ -247,7 +251,9 @@ void TailDuplication::constantAndCopyPropagate(
// to replace is active for constant propagation
StaticInstructionDeletionCount++;
DynamicInstructionDeletionCount += OriginalBB.getExecutionCount();
- Itr = std::prev(OriginalBB.eraseInstruction(Itr));
+ Itr = OriginalBB.eraseInstruction(Itr);
+ } else {
+ ++Itr;
}
}
}
More information about the llvm-commits
mailing list