[llvm] c6f1713 - [ObjC][ARC] Don't remove autoreleaseRV/retainRV pairs if the call isn't

[Akira Hatanaka via llvm-commits]

Author: Akira Hatanaka
Date: 2020-03-13T13:52:14-07:00
New Revision: c6f1713c46e61bbb8ece9ac5ac329d02e7f93228

URL: https://github.com/llvm/llvm-project/commit/c6f1713c46e61bbb8ece9ac5ac329d02e7f93228
DIFF: https://github.com/llvm/llvm-project/commit/c6f1713c46e61bbb8ece9ac5ac329d02e7f93228.diff

LOG: [ObjC][ARC] Don't remove autoreleaseRV/retainRV pairs if the call isn't
a tail call

This reapplies the patch in https://reviews.llvm.org/rG1f5b471b8bf4,
which was reverted because it was causing crashes.

https://bugs.chromium.org/p/chromium/issues/detail?id=1061289#c2

Check that HasSafePathToCall is true before checking the call is a tail
call.

Original commit message:

Previosly ARC optimizer removed the autoreleaseRV/retainRV pair in the
following code, which caused the object returned by @something to be
placed in the autorelease pool because the call to @something isn't a
tail call:

```
  %call = call i8* @something(...)
  %2 = call i8* @objc_retainAutoreleasedReturnValue(i8* %call)
  %3 = call i8* @objc_autoreleaseReturnValue(i8* %2)
  ret i8* %3
```

Fix the bug by checking whether @something is a tail call.

rdar://problem/59275894

Added: 
    

Modified: 
    llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
    llvm/test/Transforms/ObjCARC/rv.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index 577a744f3062..9da202eba95e 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -2344,6 +2344,14 @@ void ObjCARCOpt::OptimizeReturns(Function &F) {
     bool HasSafePathToCall = HasSafePathToPredecessorCall(Arg, Retain,
                                                           DependingInstructions,
                                                           Visited, PA);
+
+    // Don't remove retainRV/autoreleaseRV pairs if the call isn't a tail call.
+    if (HasSafePathToCall &&
+        GetBasicARCInstKind(Retain) == ARCInstKind::RetainRV &&
+        GetBasicARCInstKind(Autorelease) == ARCInstKind::AutoreleaseRV &&
+        !cast<CallInst>(*DependingInstructions.begin())->isTailCall())
+      continue;
+
     DependingInstructions.clear();
     Visited.clear();
 

diff  --git a/llvm/test/Transforms/ObjCARC/rv.ll b/llvm/test/Transforms/ObjCARC/rv.ll
index 3d0d56ca0e53..0a1f4665afc6 100644
--- a/llvm/test/Transforms/ObjCARC/rv.ll
+++ b/llvm/test/Transforms/ObjCARC/rv.ll
@@ -21,6 +21,8 @@ declare void @callee()
 declare void @callee_fnptr(void ()*)
 declare void @invokee()
 declare i8* @returner()
+declare i8* @returner1(i8*)
+declare i32 @__gxx_personality_v0(...)
 
 ; Test that retain+release elimination is suppressed when the
 ; retain is an objc_retainAutoreleasedReturnValue, since it's
@@ -77,7 +79,7 @@ define void @test2() {
 ; CHECK-NEXT: ret i8* %call
 define i8* @test3() {
 entry:
-  %call = call i8* @returner()
+  %call = tail call i8* @returner()
   %0 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %call) nounwind
   %1 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %0) nounwind
   ret i8* %1
@@ -387,6 +389,44 @@ bb3:
   ret i32* %retval
 }
 
+; Don't eliminate the retainRV/autoreleaseRV pair if the call isn't a tail call.
+
+; CHECK-LABEL: define i8* @test28(
+; CHECK: call i8* @returner()
+; CHECK: call i8* @llvm.objc.retainAutoreleasedReturnValue(
+; CHECK: call i8* @llvm.objc.autoreleaseReturnValue(
+define i8* @test28() {
+entry:
+  %call = call i8* @returner()
+  %0 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %call) nounwind
+  %1 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %0) nounwind
+  ret i8* %1
+}
+
+; CHECK-LABEL: define i8* @test29(
+; CHECK: call i8* @llvm.objc.retainAutoreleasedReturnValue(
+; CHECK: call i8* @llvm.objc.autoreleaseReturnValue(
+
+define i8* @test29(i8* %k) local_unnamed_addr personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %0 = tail call i8* @llvm.objc.retain(i8* %k)
+  %call = invoke i8* @returner1(i8* %k)
+          to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+  %1 = bitcast i8* %call to i8*
+  %2 = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %1)
+  tail call void @llvm.objc.release(i8* %k), !clang.imprecise_release !0
+  %3 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %1)
+  ret i8* %call
+
+lpad:
+  %4 = landingpad { i8*, i32 }
+          cleanup
+  tail call void @llvm.objc.release(i8* %k) #1, !clang.imprecise_release !0
+  resume { i8*, i32 } %4
+}
+
 !0 = !{}
 
 ; CHECK: attributes [[NUW]] = { nounwind }