Skip to content

Commit 870147d

Browse files
committed
[PPCMergeStringPool] Only replace constant once
In llvm#88846 I changed this code to use RAUW to perform the replacement instead of manual updates -- but kept the outer loop, which means we try to perform RAUW once per user. However, some of the users might be freed by the RAUW operation, resulting in use-after-free. I think the case where this happens is constant users where the replacement might result in the destruction of the original constant. I wasn't able to come up with a test case though. This is intended to fix llvm#92991.
1 parent f647321 commit 870147d

File tree

1 file changed

+7
-30
lines changed

1 file changed

+7
-30
lines changed

llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -302,13 +302,6 @@ bool PPCMergeStringPool::mergeModuleStringPool(Module &M) {
302302
return true;
303303
}
304304

305-
static bool userHasOperand(User *TheUser, GlobalVariable *GVOperand) {
306-
for (Value *Op : TheUser->operands())
307-
if (Op == GVOperand)
308-
return true;
309-
return false;
310-
}
311-
312305
// For pooled strings we need to add the offset into the pool for each string.
313306
// This is done by adding a Get Element Pointer (GEP) before each user. This
314307
// function adds the GEP.
@@ -319,29 +312,13 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
319312
Indices.push_back(ConstantInt::get(Type::getInt32Ty(*Context), 0));
320313
Indices.push_back(ConstantInt::get(Type::getInt32Ty(*Context), ElementIndex));
321314

322-
// Need to save a temporary copy of each user list because we remove uses
323-
// as we replace them.
324-
SmallVector<User *> Users;
325-
for (User *CurrentUser : GlobalToReplace->users())
326-
Users.push_back(CurrentUser);
327-
328-
for (User *CurrentUser : Users) {
329-
// The user was not found so it must have been replaced earlier.
330-
if (!userHasOperand(CurrentUser, GlobalToReplace))
331-
continue;
332-
333-
// We cannot replace operands in globals so we ignore those.
334-
if (isa<GlobalValue>(CurrentUser))
335-
continue;
336-
337-
Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
338-
PooledStructType, GPool, Indices);
339-
LLVM_DEBUG(dbgs() << "Replacing this global:\n");
340-
LLVM_DEBUG(GlobalToReplace->dump());
341-
LLVM_DEBUG(dbgs() << "with this:\n");
342-
LLVM_DEBUG(ConstGEP->dump());
343-
GlobalToReplace->replaceAllUsesWith(ConstGEP);
344-
}
315+
Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
316+
PooledStructType, GPool, Indices);
317+
LLVM_DEBUG(dbgs() << "Replacing this global:\n");
318+
LLVM_DEBUG(GlobalToReplace->dump());
319+
LLVM_DEBUG(dbgs() << "with this:\n");
320+
LLVM_DEBUG(ConstGEP->dump());
321+
GlobalToReplace->replaceAllUsesWith(ConstGEP);
345322
}
346323

347324
} // namespace

0 commit comments

Comments
 (0)