From 97ab1917845fa9426ba913139fc8a007cba1d7ce Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 27 May 2024 08:54:11 +0200 Subject: [PATCH] [PPCMergeStringPool] Only replace constant once (#92996) In #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. The case where this happens is constant users where the replacement might result in the destruction of the original constant. Fixes https://github.com/llvm/llvm-project/issues/92991. (cherry picked from commit 9f85bc834b07ebfec9e5e02deb9255a0f6ec5cc7) --- .../lib/Target/PowerPC/PPCMergeStringPool.cpp | 37 ++++--------------- .../PowerPC/mergeable-string-pool-pr92991.ll | 20 ++++++++++ 2 files changed, 27 insertions(+), 30 deletions(-) create mode 100644 llvm/test/CodeGen/PowerPC/mergeable-string-pool-pr92991.ll diff --git a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp index ebd876d50c44e..0830b02370cd0 100644 --- a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp +++ b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp @@ -290,13 +290,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. @@ -307,29 +300,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 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(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 diff --git a/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pr92991.ll b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pr92991.ll new file mode 100644 index 0000000000000..4e9c69e5fe4cf --- /dev/null +++ b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pr92991.ll @@ -0,0 +1,20 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5 +; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s + +@g = private constant [4 x i32] [i32 122, i32 67, i32 35, i32 56] +@g2 = private constant [1 x i64] [i64 1], align 8 + +define void @test(ptr %p, ptr %p2) { +; CHECK-LABEL: test: +; CHECK: # %bb.0: +; CHECK-NEXT: addis 5, 2, .L__ModuleStringPool@toc@ha +; CHECK-NEXT: addi 5, 5, .L__ModuleStringPool@toc@l +; CHECK-NEXT: addi 6, 5, 12 +; CHECK-NEXT: std 6, 0(3) +; CHECK-NEXT: addi 3, 5, 16 +; CHECK-NEXT: std 3, 0(4) +; CHECK-NEXT: blr + store ptr getelementptr inbounds ([4 x i32], ptr @g, i64 0, i64 1), ptr %p + store ptr getelementptr inbounds ([4 x i32], ptr @g, i64 0, i64 2), ptr %p2 + ret void +}