Skip to content

Commit a9a1d80

Browse files
authored
late-gc-lowering: Prevent infinite recursion (#42445)
Phi nodes are allowed to recursively depend on themselves, so we need to keep track of which ones we have seen to avoid an infinite recursion here. Apparently this doesn't usually come up, but I did see if with tsan enabled.
1 parent f1baec3 commit a9a1d80

File tree

2 files changed

+19
-9
lines changed

2 files changed

+19
-9
lines changed

src/llvm-final-gc-lowering.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,11 @@ bool FinalLowerGC::doInitialization(Module &M) {
218218

219219
bool FinalLowerGC::doFinalization(Module &M)
220220
{
221+
GlobalValue *functionList[] = {queueRootFunc, poolAllocFunc, bigAllocFunc};
222+
queueRootFunc = poolAllocFunc = bigAllocFunc = nullptr;
221223
auto used = M.getGlobalVariable("llvm.compiler.used");
222224
if (!used)
223225
return false;
224-
GlobalValue *functionList[] = {queueRootFunc, poolAllocFunc, bigAllocFunc};
225226
SmallPtrSet<Constant*, 16> InitAsSet(
226227
functionList,
227228
functionList + sizeof(functionList) / sizeof(void*));

src/llvm-late-gc-lowering.cpp

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,12 +1153,14 @@ static bool isConstGV(GlobalVariable *gv)
11531153
return gv->isConstant() || gv->getMetadata("julia.constgv");
11541154
}
11551155

1156-
static bool isLoadFromConstGV(LoadInst *LI, bool &task_local);
1157-
static bool isLoadFromConstGV(Value *v, bool &task_local)
1156+
typedef llvm::SmallPtrSet<PHINode*, 1> PhiSet;
1157+
1158+
static bool isLoadFromConstGV(LoadInst *LI, bool &task_local, PhiSet *seen = nullptr);
1159+
static bool isLoadFromConstGV(Value *v, bool &task_local, PhiSet *seen = nullptr)
11581160
{
11591161
v = v->stripInBoundsOffsets();
11601162
if (auto LI = dyn_cast<LoadInst>(v))
1161-
return isLoadFromConstGV(LI, task_local);
1163+
return isLoadFromConstGV(LI, task_local, seen);
11621164
if (auto gv = dyn_cast<GlobalVariable>(v))
11631165
return isConstGV(gv);
11641166
// null pointer
@@ -1169,12 +1171,19 @@ static bool isLoadFromConstGV(Value *v, bool &task_local)
11691171
return (CE->getOpcode() == Instruction::IntToPtr &&
11701172
isa<ConstantData>(CE->getOperand(0)));
11711173
if (auto SL = dyn_cast<SelectInst>(v))
1172-
return (isLoadFromConstGV(SL->getTrueValue(), task_local) &&
1173-
isLoadFromConstGV(SL->getFalseValue(), task_local));
1174+
return (isLoadFromConstGV(SL->getTrueValue(), task_local, seen) &&
1175+
isLoadFromConstGV(SL->getFalseValue(), task_local, seen));
11741176
if (auto Phi = dyn_cast<PHINode>(v)) {
1177+
PhiSet ThisSet(&Phi, &Phi);
1178+
if (!seen)
1179+
seen = &ThisSet;
1180+
else if (seen->count(Phi))
1181+
return true;
1182+
else
1183+
seen->insert(Phi);
11751184
auto n = Phi->getNumIncomingValues();
11761185
for (unsigned i = 0; i < n; ++i) {
1177-
if (!isLoadFromConstGV(Phi->getIncomingValue(i), task_local)) {
1186+
if (!isLoadFromConstGV(Phi->getIncomingValue(i), task_local, seen)) {
11781187
return false;
11791188
}
11801189
}
@@ -1206,7 +1215,7 @@ static bool isLoadFromConstGV(Value *v, bool &task_local)
12061215
//
12071216
// The white list implemented here and above in `isLoadFromConstGV(Value*)` should
12081217
// cover all the cases we and LLVM generates.
1209-
static bool isLoadFromConstGV(LoadInst *LI, bool &task_local)
1218+
static bool isLoadFromConstGV(LoadInst *LI, bool &task_local, PhiSet *seen)
12101219
{
12111220
// We only emit single slot GV in codegen
12121221
// but LLVM global merging can change the pointer operands to GEPs/bitcasts
@@ -1216,7 +1225,7 @@ static bool isLoadFromConstGV(LoadInst *LI, bool &task_local)
12161225
{"jtbaa_immut", "jtbaa_const", "jtbaa_datatype"})) {
12171226
if (gv)
12181227
return true;
1219-
return isLoadFromConstGV(load_base, task_local);
1228+
return isLoadFromConstGV(load_base, task_local, seen);
12201229
}
12211230
if (gv)
12221231
return isConstGV(gv);

0 commit comments

Comments
 (0)