Skip to content

Commit b590292

Browse files
authored
[DFAJumpThreading] Prevent pass from using too much memory. (#145482)
The limit 'dfa-max-num-paths' that is used to control number of enumerated paths was not checked against inside getPathsFromStateDefMap. It may lead to large memory consumption for complex enough switch statements.
1 parent 093439c commit b590292

File tree

1 file changed

+24
-14
lines changed

1 file changed

+24
-14
lines changed

llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -582,15 +582,17 @@ struct AllSwitchPaths {
582582
VisitedBlocks VB;
583583
// Get paths from the determinator BBs to SwitchPhiDefBB
584584
std::vector<ThreadingPath> PathsToPhiDef =
585-
getPathsFromStateDefMap(StateDef, SwitchPhi, VB);
585+
getPathsFromStateDefMap(StateDef, SwitchPhi, VB, MaxNumPaths);
586586
if (SwitchPhiDefBB == SwitchBlock) {
587587
TPaths = std::move(PathsToPhiDef);
588588
return;
589589
}
590590

591+
assert(MaxNumPaths >= PathsToPhiDef.size());
592+
auto PathsLimit = MaxNumPaths / PathsToPhiDef.size();
591593
// Find and append paths from SwitchPhiDefBB to SwitchBlock.
592594
PathsType PathsToSwitchBB =
593-
paths(SwitchPhiDefBB, SwitchBlock, VB, /* PathDepth = */ 1);
595+
paths(SwitchPhiDefBB, SwitchBlock, VB, /* PathDepth = */ 1, PathsLimit);
594596
if (PathsToSwitchBB.empty())
595597
return;
596598

@@ -611,13 +613,16 @@ struct AllSwitchPaths {
611613
typedef DenseMap<const BasicBlock *, const PHINode *> StateDefMap;
612614
std::vector<ThreadingPath> getPathsFromStateDefMap(StateDefMap &StateDef,
613615
PHINode *Phi,
614-
VisitedBlocks &VB) {
616+
VisitedBlocks &VB,
617+
unsigned PathsLimit) {
615618
std::vector<ThreadingPath> Res;
616619
auto *PhiBB = Phi->getParent();
617620
VB.insert(PhiBB);
618621

619622
VisitedBlocks UniqueBlocks;
620623
for (auto *IncomingBB : Phi->blocks()) {
624+
if (Res.size() >= PathsLimit)
625+
break;
621626
if (!UniqueBlocks.insert(IncomingBB).second)
622627
continue;
623628
if (!SwitchOuterLoop->contains(IncomingBB))
@@ -653,8 +658,9 @@ struct AllSwitchPaths {
653658

654659
// Direct predecessor, just add to the path.
655660
if (IncomingPhiDefBB == IncomingBB) {
656-
std::vector<ThreadingPath> PredPaths =
657-
getPathsFromStateDefMap(StateDef, IncomingPhi, VB);
661+
assert(PathsLimit > Res.size());
662+
std::vector<ThreadingPath> PredPaths = getPathsFromStateDefMap(
663+
StateDef, IncomingPhi, VB, PathsLimit - Res.size());
658664
for (ThreadingPath &Path : PredPaths) {
659665
Path.push_back(PhiBB);
660666
Res.push_back(std::move(Path));
@@ -667,13 +673,17 @@ struct AllSwitchPaths {
667673
continue;
668674

669675
PathsType IntermediatePaths;
670-
IntermediatePaths =
671-
paths(IncomingPhiDefBB, IncomingBB, VB, /* PathDepth = */ 1);
676+
assert(PathsLimit > Res.size());
677+
auto InterPathLimit = PathsLimit - Res.size();
678+
IntermediatePaths = paths(IncomingPhiDefBB, IncomingBB, VB,
679+
/* PathDepth = */ 1, InterPathLimit);
672680
if (IntermediatePaths.empty())
673681
continue;
674682

683+
assert(InterPathLimit >= IntermediatePaths.size());
684+
auto PredPathLimit = InterPathLimit / IntermediatePaths.size();
675685
std::vector<ThreadingPath> PredPaths =
676-
getPathsFromStateDefMap(StateDef, IncomingPhi, VB);
686+
getPathsFromStateDefMap(StateDef, IncomingPhi, VB, PredPathLimit);
677687
for (const ThreadingPath &Path : PredPaths) {
678688
for (const PathType &IPath : IntermediatePaths) {
679689
ThreadingPath NewPath(Path);
@@ -688,7 +698,7 @@ struct AllSwitchPaths {
688698
}
689699

690700
PathsType paths(BasicBlock *BB, BasicBlock *ToBB, VisitedBlocks &Visited,
691-
unsigned PathDepth) {
701+
unsigned PathDepth, unsigned PathsLimit) {
692702
PathsType Res;
693703

694704
// Stop exploring paths after visiting MaxPathLength blocks
@@ -715,6 +725,8 @@ struct AllSwitchPaths {
715725
// is used to prevent a duplicate path from being generated
716726
SmallSet<BasicBlock *, 4> Successors;
717727
for (BasicBlock *Succ : successors(BB)) {
728+
if (Res.size() >= PathsLimit)
729+
break;
718730
if (!Successors.insert(Succ).second)
719731
continue;
720732

@@ -736,14 +748,12 @@ struct AllSwitchPaths {
736748
// coverage and compile time.
737749
if (LI->getLoopFor(Succ) != CurrLoop)
738750
continue;
739-
740-
PathsType SuccPaths = paths(Succ, ToBB, Visited, PathDepth + 1);
751+
assert(PathsLimit > Res.size());
752+
PathsType SuccPaths =
753+
paths(Succ, ToBB, Visited, PathDepth + 1, PathsLimit - Res.size());
741754
for (PathType &Path : SuccPaths) {
742755
Path.push_front(BB);
743756
Res.push_back(Path);
744-
if (Res.size() >= MaxNumPaths) {
745-
return Res;
746-
}
747757
}
748758
}
749759
// This block could now be visited again from a different predecessor. Note

0 commit comments

Comments
 (0)