From 3a753e15a107a50483d0905067f6ef1d2314c679 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Tue, 25 Apr 2017 04:06:35 +0000 Subject: [PATCH] [ObjCARC] Do not sink an objc_retain past a clang.arc.use. We need to do this to prevent a miscompile which sinks an objc_retain past an objc_release that releases the object objc_retain retains. This happens because the top-down and bottom-up traversals each determines the insert point for retain or release individually without knowing where the other instruction is moved. For example, when the following IR is fed to the ARC optimizer, the top-down traversal decides to insert objc_retain right before objc_release and the bottom-up traversal decides to insert objc_release right after clang.arc.use. (IR before ARC optimizer) %11 = call i8* @objc_retain(i8* %10) call void (...) @clang.arc.use(%0* %5) call void @llvm.dbg.value(...) call void @objc_release(i8* %6) This reverses the order of objc_release and objc_retain, which causes the object to be destructed prematurely. (IR after ARC optimizer) call void (...) @clang.arc.use(%0* %5) call void @objc_release(i8* %6) call void @llvm.dbg.value(...) %11 = call i8* @objc_retain(i8* %10) rdar://problem/30530580 llvm-svn: 301289 --- lib/Transforms/ObjCARC/PtrState.cpp | 6 ++- .../ObjCARC/clang-arc-use-barrier.ll | 45 +++++++++++++++++++ test/Transforms/ObjCARC/intrinsic-use.ll | 11 ++--- 3 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 test/Transforms/ObjCARC/clang-arc-use-barrier.ll diff --git a/lib/Transforms/ObjCARC/PtrState.cpp b/lib/Transforms/ObjCARC/PtrState.cpp index a5afc8ad977..c1bbc4e96b1 100644 --- a/lib/Transforms/ObjCARC/PtrState.cpp +++ b/lib/Transforms/ObjCARC/PtrState.cpp @@ -351,8 +351,10 @@ bool TopDownPtrState::HandlePotentialAlterRefCount(Instruction *Inst, const Value *Ptr, ProvenanceAnalysis &PA, ARCInstKind Class) { - // Check for possible releases. - if (!CanAlterRefCount(Inst, Ptr, PA, Class)) + // Check for possible releases. Treat clang.arc.use as a releasing instruction + // to prevent sinking a retain past it. + if (!CanAlterRefCount(Inst, Ptr, PA, Class) && + Class != ARCInstKind::IntrinsicUser) return false; DEBUG(dbgs() << " CanAlterRefCount: Seq: " << GetSeq() << "; " << *Ptr diff --git a/test/Transforms/ObjCARC/clang-arc-use-barrier.ll b/test/Transforms/ObjCARC/clang-arc-use-barrier.ll new file mode 100644 index 00000000000..98d49ec512e --- /dev/null +++ b/test/Transforms/ObjCARC/clang-arc-use-barrier.ll @@ -0,0 +1,45 @@ +; RUN: opt -objc-arc -S %s | FileCheck %s + +%0 = type opaque + +; Make sure ARC optimizer doesn't sink @obj_retain past @clang.arc.use. + +; CHECK: call i8* @objc_retain( +; CHECK: call void (...) @clang.arc.use( +; CHECK: call i8* @objc_retain( +; CHECK: call void (...) @clang.arc.use( + +define void @runTest() local_unnamed_addr { + %1 = alloca %0*, align 8 + %2 = alloca %0*, align 8 + %3 = tail call %0* @foo0() + %4 = bitcast %0* %3 to i8* + %5 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %4) + store %0* %3, %0** %1, align 8 + call void @foo1(%0** nonnull %1) + %6 = load %0*, %0** %1, align 8 + %7 = bitcast %0* %6 to i8* + %8 = call i8* @objc_retain(i8* %7) + call void (...) @clang.arc.use(%0* %3) + call void @objc_release(i8* %4) + store %0* %6, %0** %2, align 8 + call void @foo1(%0** nonnull %2) + %9 = load %0*, %0** %2, align 8 + %10 = bitcast %0* %9 to i8* + %11 = call i8* @objc_retain(i8* %10) + call void (...) @clang.arc.use(%0* %6) + %tmp1 = load %0*, %0** %2, align 8 + call void @objc_release(i8* %7) + call void @foo2(%0* %9) + call void @objc_release(i8* %10) + ret void +} + +declare %0* @foo0() local_unnamed_addr +declare void @foo1(%0**) local_unnamed_addr +declare void @foo2(%0*) local_unnamed_addr + +declare i8* @objc_retainAutoreleasedReturnValue(i8*) local_unnamed_addr +declare i8* @objc_retain(i8*) local_unnamed_addr +declare void @clang.arc.use(...) local_unnamed_addr +declare void @objc_release(i8*) local_unnamed_addr diff --git a/test/Transforms/ObjCARC/intrinsic-use.ll b/test/Transforms/ObjCARC/intrinsic-use.ll index f75b1872b17..f5956201454 100644 --- a/test/Transforms/ObjCARC/intrinsic-use.ll +++ b/test/Transforms/ObjCARC/intrinsic-use.ll @@ -14,23 +14,20 @@ declare void @test0_helper(i8*, i8**) ; Ensure that we honor clang.arc.use as a use and don't miscompile ; the reduced test case from . ; -; FIXME: the fact that we re-order retains w.r.t. @clang.arc.use could -; be problematic if we get run twice, e.g. under LTO. -; ; CHECK-LABEL: define void @test0( ; CHECK: @objc_retain(i8* %x) ; CHECK-NEXT: store i8* %y, i8** %temp0 ; CHECK-NEXT: @objc_retain(i8* %y) ; CHECK-NEXT: call void @test0_helper ; CHECK-NEXT: [[VAL1:%.*]] = load i8*, i8** %temp0 -; CHECK-NEXT: call void (...) @clang.arc.use(i8* %y) ; CHECK-NEXT: @objc_retain(i8* [[VAL1]]) +; CHECK-NEXT: call void (...) @clang.arc.use(i8* %y) ; CHECK-NEXT: @objc_release(i8* %y) ; CHECK-NEXT: store i8* [[VAL1]], i8** %temp1 ; CHECK-NEXT: call void @test0_helper ; CHECK-NEXT: [[VAL2:%.*]] = load i8*, i8** %temp1 -; CHECK-NEXT: call void (...) @clang.arc.use(i8* [[VAL1]]) ; CHECK-NEXT: @objc_retain(i8* [[VAL2]]) +; CHECK-NEXT: call void (...) @clang.arc.use(i8* [[VAL1]]) ; CHECK-NEXT: @objc_release(i8* [[VAL1]]) ; CHECK-NEXT: @objc_autorelease(i8* %x) ; CHECK-NEXT: store i8* %x, i8** %out @@ -71,14 +68,14 @@ entry: ; CHECK-NEXT: @objc_retain(i8* %y) ; CHECK-NEXT: call void @test0_helper ; CHECK-NEXT: [[VAL1:%.*]] = load i8*, i8** %temp0 -; CHECK-NEXT: call void (...) @clang.arc.use(i8* %y) ; CHECK-NEXT: @objc_retain(i8* [[VAL1]]) +; CHECK-NEXT: call void (...) @clang.arc.use(i8* %y) ; CHECK-NEXT: @objc_release(i8* %y) ; CHECK-NEXT: store i8* [[VAL1]], i8** %temp1 ; CHECK-NEXT: call void @test0_helper ; CHECK-NEXT: [[VAL2:%.*]] = load i8*, i8** %temp1 -; CHECK-NEXT: call void (...) @clang.arc.use(i8* [[VAL1]]) ; CHECK-NEXT: @objc_retain(i8* [[VAL2]]) +; CHECK-NEXT: call void (...) @clang.arc.use(i8* [[VAL1]]) ; CHECK-NEXT: @objc_release(i8* [[VAL1]]) ; CHECK-NEXT: @objc_autorelease(i8* %x) ; CHECK-NEXT: @objc_release(i8* [[VAL2]])