1
0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-22 02:33:06 +01:00

[ObjC][ARC] Do not perform code motion on precise release calls

This fixes a bug where an object can get deallocated before reaching the
end of its full formal lifetime.

rdar://72110887
rdar://74123176
This commit is contained in:
Akira Hatanaka 2021-02-15 17:39:36 -08:00
parent 5e80e5c119
commit 4bfb393b4c
5 changed files with 41 additions and 53 deletions

View File

@ -1229,7 +1229,6 @@ static void CheckForUseCFGHazard(const Sequence SuccSSeq,
SomeSuccHasSame = true;
break;
case S_Stop:
case S_Release:
case S_MovableRelease:
if (!S.IsKnownSafe() && !SuccSRRIKnownSafe)
AllSuccsHaveSame = false;
@ -1257,7 +1256,6 @@ static void CheckForCanReleaseCFGHazard(const Sequence SuccSSeq,
SomeSuccHasSame = true;
break;
case S_Stop:
case S_Release:
case S_MovableRelease:
case S_Use:
if (!S.IsKnownSafe() && !SuccSRRIKnownSafe)
@ -1343,7 +1341,6 @@ ObjCARCOpt::CheckForCFGHazards(const BasicBlock *BB,
case S_Retain:
case S_None:
case S_Stop:
case S_Release:
case S_MovableRelease:
break;
}

View File

@ -44,8 +44,6 @@ raw_ostream &llvm::objcarc::operator<<(raw_ostream &OS, const Sequence S) {
return OS << "S_CanRelease";
case S_Use:
return OS << "S_Use";
case S_Release:
return OS << "S_Release";
case S_MovableRelease:
return OS << "S_MovableRelease";
case S_Stop:
@ -75,12 +73,10 @@ static Sequence MergeSeqs(Sequence A, Sequence B, bool TopDown) {
} else {
// Choose the side which is further along in the sequence.
if ((A == S_Use || A == S_CanRelease) &&
(B == S_Use || B == S_Release || B == S_Stop || B == S_MovableRelease))
(B == S_Use || B == S_Stop || B == S_MovableRelease))
return A;
// If both sides are releases, choose the more conservative one.
if (A == S_Stop && (B == S_Release || B == S_MovableRelease))
return A;
if (A == S_Release && B == S_MovableRelease)
if (A == S_Stop && B == S_MovableRelease)
return A;
}
@ -184,7 +180,7 @@ bool BottomUpPtrState::InitBottomUp(ARCMDKindCache &Cache, Instruction *I) {
// pairs by making PtrState hold a stack of states, but this is
// simple and avoids adding overhead for the non-nested case.
bool NestingDetected = false;
if (GetSeq() == S_Release || GetSeq() == S_MovableRelease) {
if (GetSeq() == S_MovableRelease) {
LLVM_DEBUG(
dbgs() << " Found nested releases (i.e. a release pair)\n");
NestingDetected = true;
@ -192,8 +188,10 @@ bool BottomUpPtrState::InitBottomUp(ARCMDKindCache &Cache, Instruction *I) {
MDNode *ReleaseMetadata =
I->getMetadata(Cache.get(ARCMDKindID::ImpreciseRelease));
Sequence NewSeq = ReleaseMetadata ? S_MovableRelease : S_Release;
Sequence NewSeq = ReleaseMetadata ? S_MovableRelease : S_Stop;
ResetSequenceProgress(NewSeq);
if (NewSeq == S_Stop)
InsertReverseInsertPt(I);
SetReleaseMetadata(ReleaseMetadata);
SetKnownSafe(HasKnownPositiveRefCount());
SetTailCallRelease(cast<CallInst>(I)->isTailCall());
@ -208,7 +206,6 @@ bool BottomUpPtrState::MatchWithRetain() {
Sequence OldSeq = GetSeq();
switch (OldSeq) {
case S_Stop:
case S_Release:
case S_MovableRelease:
case S_Use:
// If OldSeq is not S_Use or OldSeq is S_Use and we are tracking an
@ -243,7 +240,6 @@ bool BottomUpPtrState::HandlePotentialAlterRefCount(Instruction *Inst,
SetSeq(S_CanRelease);
return true;
case S_CanRelease:
case S_Release:
case S_MovableRelease:
case S_Stop:
case S_None:
@ -292,17 +288,11 @@ void BottomUpPtrState::HandlePotentialUse(BasicBlock *BB, Instruction *Inst,
// Check for possible direct uses.
switch (GetSeq()) {
case S_Release:
case S_MovableRelease:
if (CanUse(Inst, Ptr, PA, Class)) {
LLVM_DEBUG(dbgs() << " CanUse: Seq: " << GetSeq() << "; "
<< *Ptr << "\n");
SetSeqAndInsertReverseInsertPt(S_Use);
} else if (Seq == S_Release && IsUser(Class)) {
LLVM_DEBUG(dbgs() << " PreciseReleaseUse: Seq: " << GetSeq()
<< "; " << *Ptr << "\n");
// Non-movable releases depend on any possible objc pointer use.
SetSeqAndInsertReverseInsertPt(S_Stop);
} else if (const auto *Call = getreturnRVOperand(*Inst, Class)) {
if (CanUse(Call, Ptr, PA, GetBasicARCInstKind(Call))) {
LLVM_DEBUG(dbgs() << " ReleaseUse: Seq: " << GetSeq() << "; "
@ -378,7 +368,6 @@ bool TopDownPtrState::MatchWithRelease(ARCMDKindCache &Cache,
case S_None:
return false;
case S_Stop:
case S_Release:
case S_MovableRelease:
llvm_unreachable("top-down pointer in bottom up state!");
}
@ -418,7 +407,6 @@ bool TopDownPtrState::HandlePotentialAlterRefCount(
case S_None:
return false;
case S_Stop:
case S_Release:
case S_MovableRelease:
llvm_unreachable("top-down pointer in release state!");
}
@ -442,7 +430,6 @@ void TopDownPtrState::HandlePotentialUse(Instruction *Inst, const Value *Ptr,
case S_None:
return;
case S_Stop:
case S_Release:
case S_MovableRelease:
llvm_unreachable("top-down pointer in release state!");
}

View File

@ -43,8 +43,7 @@ enum Sequence {
S_Retain, ///< objc_retain(x).
S_CanRelease, ///< foo(x) -- x could possibly see a ref count decrement.
S_Use, ///< any use of x.
S_Stop, ///< like S_Release, but code motion is stopped.
S_Release, ///< objc_release(x).
S_Stop, ///< code motion is stopped.
S_MovableRelease ///< objc_release(x), !clang.imprecise_release.
};

View File

@ -723,14 +723,15 @@ return:
; CHECK-LABEL: define void @test8c(
; CHECK: entry:
; CHECK: t:
; CHECK: @llvm.objc.retain
; CHECK-NOT: @llvm.objc.
; CHECK: f:
; CHECK: @llvm.objc.retain
; CHECK-NOT: @llvm.objc.
; CHECK: mid:
; CHECK: u:
; CHECK: @llvm.objc.retain
; CHECK: @llvm.objc.release
; CHECK: g:
; CHECK: @llvm.objc.release
; CHECK-NOT: @llvm.objc.
; CHECK: return:
; CHECK: }
define void @test8c(i32* %x, i1 %p, i1 %q) nounwind {
@ -1549,17 +1550,14 @@ done:
; Like test28. but with two releases.
; CHECK-LABEL: define void @test29(
; CHECK-NOT: @llvm.objc.
; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
; CHECK: true:
; CHECK: call void @callee()
; CHECK: store
; CHECK: call void @llvm.objc.release
; CHECK-NOT: @llvm.objc.release
; CHECK: done:
; CHECK-NOT: @llvm.objc.
; CHECK: call void @llvm.objc.release
; CHECK: ohno:
; CHECK-NOT: @llvm.objc.
; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test29(i8* %p, i1 %x, i1 %y) {
entry:
@ -1584,19 +1582,15 @@ ohno:
; with an extra release.
; CHECK-LABEL: define void @test30(
; CHECK-NOT: @llvm.objc.
; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
; CHECK: true:
; CHECK: call void @callee()
; CHECK: store
; CHECK: call void @llvm.objc.release
; CHECK-NOT: @llvm.objc.release
; CHECK: false:
; CHECK-NOT: @llvm.objc.
; CHECK: done:
; CHECK-NOT: @llvm.objc.
; CHECK: call void @llvm.objc.release
; CHECK: ohno:
; CHECK-NOT: @llvm.objc.
; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test30(i8* %p, i1 %x, i1 %y, i1 %z) {
entry:
@ -1626,14 +1620,11 @@ ohno:
; CHECK: call i8* @llvm.objc.retain(i8* %p)
; CHECK: call void @callee()
; CHECK: store
; CHECK: call void @llvm.objc.release
; CHECK-NOT: @llvm.objc.release
; CHECK: true:
; CHECK-NOT: @llvm.objc.release
; CHECK: call void @llvm.objc.release
; CHECK: false:
; CHECK-NOT: @llvm.objc.release
; CHECK: call void @llvm.objc.release
; CHECK: ret void
; CHECK-NOT: @llvm.objc.release
; CHECK: }
define void @test31(i8* %p, i1 %x) {
entry:
@ -1652,14 +1643,12 @@ false:
; Don't consider bitcasts or getelementptrs direct uses.
; CHECK-LABEL: define void @test32(
; CHECK-NOT: @llvm.objc.
; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
; CHECK: true:
; CHECK: call void @callee()
; CHECK: store
; CHECK: call void @llvm.objc.release
; CHECK: done:
; CHECK-NOT: @llvm.objc.
; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test32(i8* %p, i1 %x) {
entry:
@ -1681,14 +1670,12 @@ done:
; Do consider icmps to be direct uses.
; CHECK-LABEL: define void @test33(
; CHECK-NOT: @llvm.objc.
; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
; CHECK: true:
; CHECK: call void @callee()
; CHECK: icmp
; CHECK: call void @llvm.objc.release
; CHECK: done:
; CHECK-NOT: @llvm.objc.
; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test33(i8* %p, i1 %x, i8* %y) {
entry:

View File

@ -3,6 +3,8 @@
declare void @alterRefCount()
declare void @use(i8*)
@g0 = global i8* null, align 8
; Check that ARC optimizer doesn't reverse the order of the retain call and the
; release call when there are debug instructions.
@ -20,6 +22,22 @@ define i32 @test(i8* %x, i8* %y, i8 %z, i32 %i) {
ret i32 %i
}
; ARC optimizer shouldn't move the release call, which is a precise release call
; past the call to @alterRefCount.
; CHECK-LABEL: define void @test2(
; CHECK: call void @alterRefCount(
; CHECK: call void @llvm.objc.release(
define void @test2() {
%v0 = load i8*, i8** @g0, align 8
%v1 = tail call i8* @llvm.objc.retain(i8* %v0)
tail call void @use(i8* %v0)
tail call void @alterRefCount()
tail call void @llvm.objc.release(i8* %v0)
ret void
}
declare void @llvm.dbg.declare(metadata, metadata, metadata)
declare i8* @llvm.objc.retain(i8*) local_unnamed_addr
declare void @llvm.objc.release(i8*) local_unnamed_addr