[PATCH] D156735: Do not let Value::stripPointerCasts() look through returned arg functions.

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs created this revision.
jroelofs added reviewers: jrtc27, rjmccall, majnemer, hfinkel.
Herald added a subscriber: hiraditya.
Herald added a project: All.
jroelofs requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

We can allow this in alias analysis context, but it should be avoided for the general case where looking through would cause a semantic difference, i.e. in an objc_retain call (which will soon be marked `thisreturn` [1]). FWIW, the "bug" here was introduced in [2].

1: https://reviews.llvm.org/D156710
2: https://reviews.llvm.org/D9383


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156735

Files:
  llvm/include/llvm/IR/Value.h
  llvm/lib/IR/Value.cpp
  llvm/unittests/IR/InstructionsTest.cpp


Index: llvm/unittests/IR/InstructionsTest.cpp
===================================================================
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -458,6 +458,35 @@
                                                  wrap(V2Int32PtrAS1Ty), true));
 }
 
+TEST(InstructionsTest, StripPointerCasts) {
+  LLVMContext C;
+  Type *I64PtrTy = Type::getInt64PtrTy(C);
+  Type *I32PtrTy = Type::getInt32PtrTy(C);
+
+  Module M("M", C);
+  FunctionType *FTy =
+      FunctionType::get(I64PtrTy, {I64PtrTy}, false);
+  auto *F = Function::Create(FTy, Function::ExternalLinkage, "", M);
+  auto *BB = BasicBlock::Create(C, "bb", F);
+  IRBuilder<> Builder(C);
+  Builder.SetInsertPoint(BB);
+
+  Value *A0 = F->getArg(0);
+  Function *SSACopy = Intrinsic::getDeclaration(&M, Intrinsic::ssa_copy, {A0->getType()});
+  CallInst *Call = Builder.CreateCall(SSACopy, {A0});
+  Value *Cast = Builder.CreateBitCast(Call, I32PtrTy);
+
+  // When stripping for alias analysis, it is okay to look through returned
+  // argument functions, since the pointer is unchanged.
+  EXPECT_EQ(Cast->stripPointerCastsForAliasAnalysis(), A0);
+
+  // Otherwise, this is not okay.
+  EXPECT_EQ(Cast->stripPointerCasts(), Call);
+  EXPECT_EQ(Cast->stripPointerCastsAndAliases(), Call);
+  EXPECT_EQ(Cast->stripPointerCastsSameRepresentation(), Call);
+  EXPECT_EQ(Cast->stripInBoundsConstantOffsets(), Call);
+}
+
 TEST(InstructionsTest, VectorGep) {
   LLVMContext C;
 
Index: llvm/lib/IR/Value.cpp
===================================================================
--- llvm/lib/IR/Value.cpp
+++ llvm/lib/IR/Value.cpp
@@ -662,7 +662,8 @@
       V = cast<PHINode>(V)->getIncomingValue(0);
     } else {
       if (const auto *Call = dyn_cast<CallBase>(V)) {
-        if (const Value *RV = Call->getReturnedArgOperand()) {
+        if (const Value *RV = Call->getReturnedArgOperand();
+            RV && StripKind == PSK_ForAliasAnalysis) {
           V = RV;
           continue;
         }
Index: llvm/include/llvm/IR/Value.h
===================================================================
--- llvm/include/llvm/IR/Value.h
+++ llvm/include/llvm/IR/Value.h
@@ -656,7 +656,7 @@
   }
 
   /// Strip off pointer casts, all-zero GEPs, single-argument phi nodes and
-  /// invariant group info.
+  /// invariant group info.  Looks through returned arg functions.
   ///
   /// Returns the original uncasted value.  If this is called on a non-pointer
   /// value, it returns 'this'. This function should be used only in


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156735.545778.patch
Type: text/x-patch
Size: 2540 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230731/dd79587e/attachment.bin>