diff options
author | Dan Gohman <gohman@apple.com> | 2012-05-08 23:34:08 +0000 |
---|---|---|
committer | Dan Gohman <gohman@apple.com> | 2012-05-08 23:34:08 +0000 |
commit | 4670dace66f5ef6ab0b153cd482b5bc827467c73 (patch) | |
tree | 9e79febdb512aa646228ab0380eaef3d13daff27 | |
parent | f191b43103113119e60b19b8e78966803a20c655 (diff) | |
download | llvm-4670dace66f5ef6ab0b153cd482b5bc827467c73.tar.gz llvm-4670dace66f5ef6ab0b153cd482b5bc827467c73.tar.bz2 llvm-4670dace66f5ef6ab0b153cd482b5bc827467c73.tar.xz |
Fix objc_storeStrong pattern matching to catch a potential use of the
old value after the store but before it is released.
This fixes rdar:/11116986.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@156442 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | lib/Transforms/Scalar/ObjCARC.cpp | 38 | ||||
-rw-r--r-- | test/Transforms/ObjCARC/contract-storestrong.ll | 76 |
2 files changed, 105 insertions, 9 deletions
diff --git a/lib/Transforms/Scalar/ObjCARC.cpp b/lib/Transforms/Scalar/ObjCARC.cpp index a75e8dd903..6e6771e4bb 100644 --- a/lib/Transforms/Scalar/ObjCARC.cpp +++ b/lib/Transforms/Scalar/ObjCARC.cpp @@ -3925,18 +3925,38 @@ void ObjCARCContract::ContractRelease(Instruction *Release, BasicBlock *BB = Release->getParent(); if (Load->getParent() != BB) return; - // Walk down to find the store. + // Walk down to find the store and the release, which may be in either order. BasicBlock::iterator I = Load, End = BB->end(); ++I; AliasAnalysis::Location Loc = AA->getLocation(Load); - while (I != End && - (&*I == Release || - IsRetain(GetBasicInstructionClass(I)) || - !(AA->getModRefInfo(I, Loc) & AliasAnalysis::Mod))) - ++I; - StoreInst *Store = dyn_cast<StoreInst>(I); - if (!Store || !Store->isSimple()) return; - if (Store->getPointerOperand() != Loc.Ptr) return; + StoreInst *Store = 0; + bool SawRelease = false; + for (; !Store || !SawRelease; ++I) { + Instruction *Inst = I; + if (Inst == Release) { + SawRelease = true; + continue; + } + + InstructionClass Class = GetBasicInstructionClass(Inst); + + // Unrelated retains are harmless. + if (IsRetain(Class)) + continue; + + if (Store) { + // The store is the point where we're going to put the objc_storeStrong, + // so make sure there are no uses after it. + if (CanUse(Inst, Load, PA, Class)) + return; + } else if (AA->getModRefInfo(Inst, Loc) & AliasAnalysis::Mod) { + // We are moving the load down to the store, so check for anything + // else which writes to the memory between the load and the store. + Store = dyn_cast<StoreInst>(Inst); + if (!Store || !Store->isSimple()) return; + if (Store->getPointerOperand() != Loc.Ptr) return; + } + } Value *New = StripPointerCastsAndObjCCalls(Store->getValueOperand()); diff --git a/test/Transforms/ObjCARC/contract-storestrong.ll b/test/Transforms/ObjCARC/contract-storestrong.ll index 4ff0596fbb..376120cf8b 100644 --- a/test/Transforms/ObjCARC/contract-storestrong.ll +++ b/test/Transforms/ObjCARC/contract-storestrong.ll @@ -4,6 +4,7 @@ target datalayout = "e-p:64:64:64" declare i8* @objc_retain(i8*) declare void @objc_release(i8*) +declare void @use_pointer(i8*) @x = external global i8* @@ -57,3 +58,78 @@ entry: tail call void @objc_release(i8* %tmp) nounwind ret void } + +; Don't do this if there's a use of the old pointer vlaue between the store +; and the release. + +; CHECK: define void @test3(i8* %newValue) { +; CHECK-NEXT: entry: +; CHECK-NEXT: %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind +; CHECK-NEXT: %x1 = load i8** @x, align 8 +; CHECK-NEXT: store i8* %x0, i8** @x, align 8 +; CHECK-NEXT: tail call void @use_pointer(i8* %x1), !clang.arc.no_objc_arc_exceptions !0 +; CHECK-NEXT: tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0 +; CHECK-NEXT: ret void +; CHECK-NEXT: } +define void @test3(i8* %newValue) { +entry: + %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind + %x1 = load i8** @x, align 8 + store i8* %newValue, i8** @x, align 8 + tail call void @use_pointer(i8* %x1), !clang.arc.no_objc_arc_exceptions !0 + tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0 + ret void +} + +; Like test3, but with an icmp use instead of a call, for good measure. + +; CHECK: define i1 @test4(i8* %newValue, i8* %foo) { +; CHECK-NEXT: entry: +; CHECK-NEXT: %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind +; CHECK-NEXT: %x1 = load i8** @x, align 8 +; CHECK-NEXT: store i8* %x0, i8** @x, align 8 +; CHECK-NEXT: %t = icmp eq i8* %x1, %foo +; CHECK-NEXT: tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0 +; CHECK-NEXT: ret i1 %t +; CHECK-NEXT: } +define i1 @test4(i8* %newValue, i8* %foo) { +entry: + %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind + %x1 = load i8** @x, align 8 + store i8* %newValue, i8** @x, align 8 + %t = icmp eq i8* %x1, %foo + tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0 + ret i1 %t +} + +; Do form an objc_storeStrong here, because the use is before the store. + +; CHECK: define i1 @test5(i8* %newValue, i8* %foo) { +; CHECK: %t = icmp eq i8* %x1, %foo +; CHECK: tail call void @objc_storeStrong(i8** @x, i8* %newValue) nounwind +define i1 @test5(i8* %newValue, i8* %foo) { +entry: + %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind + %x1 = load i8** @x, align 8 + %t = icmp eq i8* %x1, %foo + store i8* %newValue, i8** @x, align 8 + tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0 + ret i1 %t +} + +; Like test5, but the release is before the store. + +; CHECK: define i1 @test6(i8* %newValue, i8* %foo) { +; CHECK: %t = icmp eq i8* %x1, %foo +; CHECK: tail call void @objc_storeStrong(i8** @x, i8* %newValue) nounwind +define i1 @test6(i8* %newValue, i8* %foo) { +entry: + %x0 = tail call i8* @objc_retain(i8* %newValue) nounwind + %x1 = load i8** @x, align 8 + tail call void @objc_release(i8* %x1) nounwind, !clang.imprecise_release !0 + %t = icmp eq i8* %x1, %foo + store i8* %newValue, i8** @x, align 8 + ret i1 %t +} + +!0 = metadata !{} |