From e8c161a92451ad38919525ea73ae3c6936c24bdf Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Sat, 12 Jan 2013 01:25:15 +0000 Subject: [PATCH] Fixed a bug where we were tail calling objc_autorelease causing an object to not be placed into an autorelease pool. The reason that this occurs is that tail calling objc_autorelease eventually tail calls -[NSObject autorelease] which supports fast autorelease. This can cause us to violate the semantic gaurantees of __autoreleasing variables that assignment to an __autoreleasing variables always yields an object that is placed into the innermost autorelease pool. The fix included in this patch works by: 1. In the peephole optimization function OptimizeIndividualFunctions, always remove tail call from objc_autorelease. 2. Whenever we convert to/from an objc_autorelease, set/unset the tail call keyword as appropriate. *NOTE* I also handled the case where objc_autorelease is converted in OptimizeReturns to an autoreleaseRV which still violates the ARC semantics. I will be removing that in a later patch and I wanted to make sure that the tree is in a consistent state vis-a-vis ARC always. Additionally some test cases are provided and all tests that have tail call marked objc_autorelease keywords have been modified so that tail call has been removed. *NOTE* One test fails due to a separate bug that I am going to commit soon. Thus I marked the check line TMP: instead of CHECK: so make check does not fail. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@172287 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/ObjCARC.cpp | 27 +++++- test/Transforms/ObjCARC/basic.ll | 6 +- test/Transforms/ObjCARC/contract.ll | 10 +-- .../move-and-form-retain-autorelease.ll | 2 +- test/Transforms/ObjCARC/rv.ll | 8 +- .../tail-call-invariant-enforcement.ll | 84 +++++++++++++++++++ 6 files changed, 122 insertions(+), 15 deletions(-) create mode 100644 test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll diff --git a/lib/Transforms/Scalar/ObjCARC.cpp b/lib/Transforms/Scalar/ObjCARC.cpp index 34700ebb02c..1607e8e80a5 100644 --- a/lib/Transforms/Scalar/ObjCARC.cpp +++ b/lib/Transforms/Scalar/ObjCARC.cpp @@ -426,10 +426,20 @@ static bool IsAlwaysTail(InstructionClass Class) { // IC_RetainBlock may be given a stack argument. return Class == IC_Retain || Class == IC_RetainRV || - Class == IC_Autorelease || Class == IC_AutoreleaseRV; } +/// \brief Test if the given class represents instructions which are never safe +/// to mark with the "tail" keyword. +static bool IsNeverTail(InstructionClass Class) { + /// It is never safe to tail call objc_autorelease since by tail calling + /// objc_autorelease, we also tail call -[NSObject autorelease] which supports + /// fast autoreleasing causing our object to be potentially reclaimed from the + /// autorelease pool which violates the semantics of __autoreleasing types in + /// ARC. + return Class == IC_Autorelease; +} + /// IsNoThrow - Test if the given class represents instructions which are always /// safe to mark with the nounwind attribute.. static bool IsNoThrow(InstructionClass Class) { @@ -2306,8 +2316,10 @@ ObjCARCOpt::OptimizeAutoreleaseRVCall(Function &F, Instruction *AutoreleaseRV) { " Old: " << *AutoreleaseRV << "\n"); - cast(AutoreleaseRV)-> + CallInst *AutoreleaseRVCI = cast(AutoreleaseRV); + AutoreleaseRVCI-> setCalledFunction(getAutoreleaseCallee(F.getParent())); + AutoreleaseRVCI->setTailCall(false); // Never tail call objc_autorelease. DEBUG(dbgs() << " New: " << *AutoreleaseRV << "\n"); @@ -2449,6 +2461,16 @@ void ObjCARCOpt::OptimizeIndividualCalls(Function &F) { cast(Inst)->setTailCall(); } + // Ensure that functions that can never have a "tail" keyword due to the + // semantics of ARC truly do not do so. + if (IsNeverTail(Class)) { + Changed = true; + DEBUG(dbgs() << "ObjCARCOpt::OptimizeIndividualCalls: Removing tail keyword" + " from function: " << *Inst << + "\n"); + cast(Inst)->setTailCall(false); + } + // Set nounwind as needed. if (IsNoThrow(Class)) { Changed = true; @@ -3756,6 +3778,7 @@ void ObjCARCOpt::OptimizeReturns(Function &F) { Autorelease->setCalledFunction(getAutoreleaseRVCallee(F.getParent())); DEBUG(dbgs() << " Out: " << *Autorelease << "\n"); + Autorelease->setTailCall(); // Always tail call autoreleaseRV. AutoreleaseClass = IC_AutoreleaseRV; } diff --git a/test/Transforms/ObjCARC/basic.ll b/test/Transforms/ObjCARC/basic.ll index 7b64b1be7c6..4faffa55e7d 100644 --- a/test/Transforms/ObjCARC/basic.ll +++ b/test/Transforms/ObjCARC/basic.ll @@ -405,7 +405,7 @@ entry: ; CHECK: define void @test11( ; CHECK: tail call i8* @objc_retain(i8* %x) nounwind -; CHECK: tail call i8* @objc_autorelease(i8* %0) nounwind +; CHECK: call i8* @objc_autorelease(i8* %0) nounwind ; CHECK: } define void @test11(i8* %x) nounwind { entry: @@ -465,7 +465,7 @@ entry: ; CHECK: tail call i8* @objc_retain(i8* %x) nounwind ; CHECK: tail call i8* @objc_retain(i8* %x) nounwind ; CHECK: @use_pointer(i8* %x) -; CHECK: tail call i8* @objc_autorelease(i8* %x) nounwind +; CHECK: call i8* @objc_autorelease(i8* %x) nounwind ; CHECK: } define void @test13(i8* %x, i64 %n) { entry: @@ -1452,7 +1452,7 @@ define void @test45(i8** %pp, i8** %qq) { ; CHECK: define void @test46( ; CHECK: tail call i8* @objc_retain(i8* %p) nounwind ; CHECK: true: -; CHECK: tail call i8* @objc_autorelease(i8* %p) nounwind +; CHECK: call i8* @objc_autorelease(i8* %p) nounwind define void @test46(i8* %p, i1 %a) { entry: call i8* @objc_retain(i8* %p) diff --git a/test/Transforms/ObjCARC/contract.ll b/test/Transforms/ObjCARC/contract.ll index c48f8a534fa..40b11a9d0e2 100644 --- a/test/Transforms/ObjCARC/contract.ll +++ b/test/Transforms/ObjCARC/contract.ll @@ -39,7 +39,7 @@ entry: define void @test2(i8* %x) nounwind { entry: %0 = tail call i8* @objc_retain(i8* %x) nounwind - tail call i8* @objc_autorelease(i8* %0) nounwind + call i8* @objc_autorelease(i8* %0) nounwind call void @use_pointer(i8* %x) ret void } @@ -66,7 +66,7 @@ define void @test3(i8* %x, i64 %n) { entry: tail call i8* @objc_retain(i8* %x) nounwind call void @use_pointer(i8* %x) - tail call i8* @objc_autorelease(i8* %x) nounwind + call i8* @objc_autorelease(i8* %x) nounwind ret void } @@ -84,7 +84,7 @@ define void @test4(i8* %x, i64 %n) { entry: tail call i8* @objc_retain(i8* %x) nounwind call void @use_pointer(i8* %x) - tail call i8* @objc_autorelease(i8* %x) nounwind + call i8* @objc_autorelease(i8* %x) nounwind tail call void @objc_release(i8* %x) nounwind ret void } @@ -94,7 +94,7 @@ entry: ; CHECK: define void @test5( ; CHECK: tail call i8* @objc_retain(i8* %p) nounwind ; CHECK: true: -; CHECK: tail call i8* @objc_autorelease(i8* %0) nounwind +; CHECK: call i8* @objc_autorelease(i8* %0) nounwind ; CHECK: } define void @test5(i8* %p, i1 %a) { entry: @@ -102,7 +102,7 @@ entry: br i1 %a, label %true, label %false true: - tail call i8* @objc_autorelease(i8* %p) nounwind + call i8* @objc_autorelease(i8* %p) nounwind call void @use_pointer(i8* %p) ret void diff --git a/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll b/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll index 170d0a99c98..d7a54ab4308 100644 --- a/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll +++ b/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll @@ -212,7 +212,7 @@ bb99: ; preds = %bb57 br label %bb104 bb104: ; preds = %bb99, %bb57 - %tmp105 = tail call i8* @objc_autorelease(i8* %tmp72) nounwind + %tmp105 = call i8* @objc_autorelease(i8* %tmp72) nounwind %tmp106 = bitcast i8* %tmp105 to %14* tail call void @objc_release(i8* %tmp85) nounwind %tmp107 = bitcast %18* %tmp47 to i8* diff --git a/test/Transforms/ObjCARC/rv.ll b/test/Transforms/ObjCARC/rv.ll index 9353a19f71a..638b89ccc72 100644 --- a/test/Transforms/ObjCARC/rv.ll +++ b/test/Transforms/ObjCARC/rv.ll @@ -150,7 +150,7 @@ define void @test8() { ; Don't apply the RV optimization to autorelease if there's no retain. ; CHECK: define i8* @test9(i8* %p) -; CHECK: tail call i8* @objc_autorelease(i8* %p) +; CHECK: call i8* @objc_autorelease(i8* %p) define i8* @test9(i8* %p) { call i8* @objc_autorelease(i8* %p) ret i8* %p @@ -174,7 +174,7 @@ define i8* @test10(i8* %p) { ; CHECK: define i8* @test11(i8* %p) ; CHECK: tail call i8* @objc_retain(i8* %p) ; CHECK-NEXT: call void @use_pointer(i8* %p) -; CHECK: tail call i8* @objc_autorelease(i8* %p) +; CHECK: call i8* @objc_autorelease(i8* %p) ; CHECK-NEXT: ret i8* %p define i8* @test11(i8* %p) { %1 = call i8* @objc_retain(i8* %p) @@ -201,7 +201,7 @@ define i8* @test12(i8* %p) { ; CHECK: define i8* @test13( ; CHECK: tail call i8* @objc_retainAutoreleasedReturnValue(i8* %p) -; CHECK: tail call i8* @objc_autorelease(i8* %p) +; CHECK: call i8* @objc_autorelease(i8* %p) ; CHECK: ret i8* %p define i8* @test13() { %p = call i8* @returner() @@ -323,7 +323,7 @@ define i8* @test22(i8* %p) { ; Convert autoreleaseRV to autorelease. ; CHECK: define void @test23( -; CHECK: tail call i8* @objc_autorelease(i8* %p) nounwind +; CHECK: call i8* @objc_autorelease(i8* %p) nounwind define void @test23(i8* %p) { store i8 0, i8* %p call i8* @objc_autoreleaseReturnValue(i8* %p) diff --git a/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll b/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll new file mode 100644 index 00000000000..d9e2c0c6ea3 --- /dev/null +++ b/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll @@ -0,0 +1,84 @@ +; RUN: opt -objc-arc -S < %s | FileCheck %s + +declare i8* @objc_release(i8* %x) +declare i8* @objc_retain(i8* %x) +declare i8* @objc_autorelease(i8* %x) +declare i8* @objc_autoreleaseReturnValue(i8* %x) +declare i8* @objc_retainAutoreleasedReturnValue(i8* %x) + +; Never tail call objc_autorelease. +define i8* @test0(i8* %x) { +entry: + ; CHECK: %tmp0 = call i8* @objc_autorelease(i8* %x) + %tmp0 = call i8* @objc_autorelease(i8* %x) + ; CHECK: %tmp1 = call i8* @objc_autorelease(i8* %x) + %tmp1 = tail call i8* @objc_autorelease(i8* %x) + + ret i8* %x +} + +; Always tail call autoreleaseReturnValue. +define i8* @test1(i8* %x) { +entry: + ; CHECK: %tmp0 = tail call i8* @objc_autoreleaseReturnValue(i8* %x) + %tmp0 = call i8* @objc_autoreleaseReturnValue(i8* %x) + ; CHECK: %tmp1 = tail call i8* @objc_autoreleaseReturnValue(i8* %x) + %tmp1 = tail call i8* @objc_autoreleaseReturnValue(i8* %x) + ret i8* %x +} + +; Always tail call objc_retain. +define i8* @test2(i8* %x) { +entry: + ; CHECK: %tmp0 = tail call i8* @objc_retain(i8* %x) + %tmp0 = call i8* @objc_retain(i8* %x) + ; CHECK: %tmp1 = tail call i8* @objc_retain(i8* %x) + %tmp1 = tail call i8* @objc_retain(i8* %x) + ret i8* %x +} + +define i8* @tmp(i8* %x) { + ret i8* %x +} + +; Always tail call objc_retainAutoreleasedReturnValue. +define i8* @test3(i8* %x) { +entry: + %y = call i8* @tmp(i8* %x) + ; CHECK: %tmp0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %y) + %tmp0 = call i8* @objc_retainAutoreleasedReturnValue(i8* %y) + %z = call i8* @tmp(i8* %x) + ; CHECK: %tmp1 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %z) + %tmp1 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %z) + ret i8* %x +} + +; By itself, we should never change whether or not objc_release is tail called. +define i8* @test4(i8* %x) { +entry: + ; CHECK: %tmp0 = call i8* @objc_release(i8* %x) + %tmp0 = call i8* @objc_release(i8* %x) + ; CHECK: %tmp1 = tail call i8* @objc_release(i8* %x) + %tmp1 = tail call i8* @objc_release(i8* %x) + ret i8* %x +} + +; If we convert a tail called @objc_autoreleaseReturnValue to an +; @objc_autorelease, ensure that the tail call is removed. +define i8* @test5(i8* %x) { +entry: + ; TMP: %tmp0 = call i8* @objc_autorelease(i8* %x) + %tmp0 = tail call i8* @objc_autoreleaseReturnValue(i8* %x) + ret i8* %tmp0 +} + +; If we convert a called @objc_autorelease to an @objc_autoreleaseReturnValue, +; ensure that the tail call is added. +define i8* @test6(i8* %x) { +entry: + ; CHECK: %tmp0 = tail call i8* @objc_retain(i8* %x) + %tmp0 = tail call i8* @objc_retain(i8* %x) + ; CHECK: %tmp1 = tail call i8* @objc_autoreleaseReturnValue(i8* %x) + %tmp1 = call i8* @objc_autorelease(i8* %x) + ret i8* %x +} -- 2.34.1