Skip to content

Commit

Permalink
[PPCMergeStringPool] Only replace constant once
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nikic committed May 23, 2024
1 parent f647321 commit 870147d
Showing 1 changed file with 7 additions and 30 deletions.
37 changes: 7 additions & 30 deletions llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,13 +302,6 @@ bool PPCMergeStringPool::mergeModuleStringPool(Module &M) {
return true;
}

static bool userHasOperand(User *TheUser, GlobalVariable *GVOperand) {
for (Value *Op : TheUser->operands())
if (Op == GVOperand)
return true;
return false;
}

// For pooled strings we need to add the offset into the pool for each string.
// This is done by adding a Get Element Pointer (GEP) before each user. This
// function adds the GEP.
Expand All @@ -319,29 +312,13 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
Indices.push_back(ConstantInt::get(Type::getInt32Ty(*Context), 0));
Indices.push_back(ConstantInt::get(Type::getInt32Ty(*Context), ElementIndex));

// Need to save a temporary copy of each user list because we remove uses
// as we replace them.
SmallVector<User *> Users;
for (User *CurrentUser : GlobalToReplace->users())
Users.push_back(CurrentUser);

for (User *CurrentUser : Users) {
// The user was not found so it must have been replaced earlier.
if (!userHasOperand(CurrentUser, GlobalToReplace))
continue;

// We cannot replace operands in globals so we ignore those.
if (isa<GlobalValue>(CurrentUser))
continue;

Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
PooledStructType, GPool, Indices);
LLVM_DEBUG(dbgs() << "Replacing this global:\n");
LLVM_DEBUG(GlobalToReplace->dump());
LLVM_DEBUG(dbgs() << "with this:\n");
LLVM_DEBUG(ConstGEP->dump());
GlobalToReplace->replaceAllUsesWith(ConstGEP);
}
Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
PooledStructType, GPool, Indices);
LLVM_DEBUG(dbgs() << "Replacing this global:\n");
LLVM_DEBUG(GlobalToReplace->dump());
LLVM_DEBUG(dbgs() << "with this:\n");
LLVM_DEBUG(ConstGEP->dump());
GlobalToReplace->replaceAllUsesWith(ConstGEP);
}

} // namespace
Expand Down

0 comments on commit 870147d

Please sign in to comment.