objective-C arc: load of a __weak object happens via call to
objc_loadWeak. This retains and autorelease the weakly-refereced
object. This hidden autorelease sometimes makes __weak variable alive even
after the weak reference is erased, because the object is still referenced
by an autorelease pool. This patch overcomes this behavior by loading a
weak object via call to objc_loadWeakRetained(), followng it by objc_release
at appropriate place, thereby removing the hidden autorelease. // rdar://10849570
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@168740 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp
index 2d1d152..c470918 100644
--- a/lib/CodeGen/CGCall.cpp
+++ b/lib/CodeGen/CGCall.cpp
@@ -1740,7 +1740,12 @@
// Create the temporary.
llvm::Value *temp = CGF.CreateTempAlloca(destType->getElementType(),
"icr.temp");
-
+ // Loading an l-value can introduce a cleanup if the l-value is __weak,
+ // and that cleanup will be conditional if we can't prove that the l-value
+ // isn't null, so we need to register a dominating point so that the cleanups
+ // system will make valid IR.
+ CodeGenFunction::ConditionalEvaluation condEval(CGF);
+
// Zero-initialize it if we're not doing a copy-initialization.
bool shouldCopy = CRE->shouldCopy();
if (!shouldCopy) {
@@ -1749,7 +1754,7 @@
cast<llvm::PointerType>(destType->getElementType()));
CGF.Builder.CreateStore(null, temp);
}
-
+
llvm::BasicBlock *contBB = 0;
// If the address is *not* known to be non-null, we need to switch.
@@ -1772,6 +1777,7 @@
llvm::BasicBlock *copyBB = CGF.createBasicBlock("icr.copy");
CGF.Builder.CreateCondBr(isNull, contBB, copyBB);
CGF.EmitBlock(copyBB);
+ condEval.begin(CGF);
}
}
@@ -1788,10 +1794,12 @@
// Use an ordinary store, not a store-to-lvalue.
CGF.Builder.CreateStore(src, temp);
}
-
+
// Finish the control flow if we needed it.
- if (shouldCopy && !provablyNonNull)
+ if (shouldCopy && !provablyNonNull) {
CGF.EmitBlock(contBB);
+ condEval.end(CGF);
+ }
args.addWriteback(srcAddr, srcAddrType, temp);
args.add(RValue::get(finalArgument), CRE->getType());
diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index cb274fc..e709e72 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1119,8 +1119,11 @@
return RValue::get(CGM.getObjCRuntime().EmitObjCWeakRead(*this,
AddrWeakObj));
}
- if (LV.getQuals().getObjCLifetime() == Qualifiers::OCL_Weak)
- return RValue::get(EmitARCLoadWeak(LV.getAddress()));
+ if (LV.getQuals().getObjCLifetime() == Qualifiers::OCL_Weak) {
+ llvm::Value *Object = EmitARCLoadWeakRetained(LV.getAddress());
+ Object = EmitObjCConsumeObject(LV.getType(), Object);
+ return RValue::get(Object);
+ }
if (LV.isSimple()) {
assert(!LV.getType()->isFunctionType());
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 840f04f..66beb34 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -504,6 +504,12 @@
T = T.getUnqualifiedType();
UpdateMarkingForLValueToRValue(E);
+
+ // Loading a __weak object implicitly retains the value, so we need a cleanup to
+ // balance that.
+ if (getLangOpts().ObjCAutoRefCount &&
+ E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
+ ExprNeedsCleanups = true;
ExprResult Res = Owned(ImplicitCastExpr::Create(Context, T, CK_LValueToRValue,
E, 0, VK_RValue));
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 7ed3942..5b7fb46 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -3846,14 +3846,15 @@
/// Determines whether this expression is an acceptable ICR source.
static InvalidICRKind isInvalidICRSource(ASTContext &C, Expr *e,
- bool isAddressOf) {
+ bool isAddressOf, bool &isWeakAccess) {
// Skip parens.
e = e->IgnoreParens();
// Skip address-of nodes.
if (UnaryOperator *op = dyn_cast<UnaryOperator>(e)) {
if (op->getOpcode() == UO_AddrOf)
- return isInvalidICRSource(C, op->getSubExpr(), /*addressof*/ true);
+ return isInvalidICRSource(C, op->getSubExpr(), /*addressof*/ true,
+ isWeakAccess);
// Skip certain casts.
} else if (CastExpr *ce = dyn_cast<CastExpr>(e)) {
@@ -3862,7 +3863,7 @@
case CK_BitCast:
case CK_LValueBitCast:
case CK_NoOp:
- return isInvalidICRSource(C, ce->getSubExpr(), isAddressOf);
+ return isInvalidICRSource(C, ce->getSubExpr(), isAddressOf, isWeakAccess);
case CK_ArrayToPointerDecay:
return IIK_nonscalar;
@@ -3876,6 +3877,11 @@
// If we have a declaration reference, it had better be a local variable.
} else if (isa<DeclRefExpr>(e)) {
+ // set isWeakAccess to true, to mean that there will be an implicit
+ // load which requires a cleanup.
+ if (e->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
+ isWeakAccess = true;
+
if (!isAddressOf) return IIK_nonlocal;
VarDecl *var = dyn_cast<VarDecl>(cast<DeclRefExpr>(e)->getDecl());
@@ -3885,10 +3891,11 @@
// If we have a conditional operator, check both sides.
} else if (ConditionalOperator *cond = dyn_cast<ConditionalOperator>(e)) {
- if (InvalidICRKind iik = isInvalidICRSource(C, cond->getLHS(), isAddressOf))
+ if (InvalidICRKind iik = isInvalidICRSource(C, cond->getLHS(), isAddressOf,
+ isWeakAccess))
return iik;
- return isInvalidICRSource(C, cond->getRHS(), isAddressOf);
+ return isInvalidICRSource(C, cond->getRHS(), isAddressOf, isWeakAccess);
// These are never scalar.
} else if (isa<ArraySubscriptExpr>(e)) {
@@ -3907,8 +3914,13 @@
/// indirect copy/restore.
static void checkIndirectCopyRestoreSource(Sema &S, Expr *src) {
assert(src->isRValue());
-
- InvalidICRKind iik = isInvalidICRSource(S.Context, src, false);
+ bool isWeakAccess = false;
+ InvalidICRKind iik = isInvalidICRSource(S.Context, src, false, isWeakAccess);
+ // If isWeakAccess to true, there will be an implicit
+ // load which requires a cleanup.
+ if (S.getLangOpts().ObjCAutoRefCount && isWeakAccess)
+ S.ExprNeedsCleanups = true;
+
if (iik == IIK_okay) return;
S.Diag(src->getExprLoc(), diag::err_arc_nonlocal_writeback)
diff --git a/test/CodeGenObjC/arc-blocks.m b/test/CodeGenObjC/arc-blocks.m
index 63bb7a3..f87f9a4 100644
--- a/test/CodeGenObjC/arc-blocks.m
+++ b/test/CodeGenObjC/arc-blocks.m
@@ -250,7 +250,7 @@
// 0x42800000 - has signature, copy/dispose helpers, as well as BLOCK_HAS_EXTENDED_LAYOUT
// CHECK: store i32 -1040187392,
// CHECK: [[SLOT:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
- // CHECK-NEXT: [[T0:%.*]] = call i8* @objc_loadWeak(i8** [[VAR]])
+ // CHECK-NEXT: [[T0:%.*]] = call i8* @objc_loadWeakRetained(i8** [[VAR]])
// CHECK-NEXT: call i8* @objc_initWeak(i8** [[SLOT]], i8* [[T0]])
// CHECK: call void @test7_helper(
// CHECK-NEXT: call void @objc_destroyWeak(i8** {{%.*}})
@@ -259,8 +259,9 @@
// CHECK: define internal void @__test7_block_invoke
// CHECK: [[SLOT:%.*]] = getelementptr inbounds [[BLOCK_T]]* {{%.*}}, i32 0, i32 5
- // CHECK-NEXT: [[T0:%.*]] = call i8* @objc_loadWeak(i8** [[SLOT]])
+ // CHECK-NEXT: [[T0:%.*]] = call i8* @objc_loadWeakRetained(i8** [[SLOT]])
// CHECK-NEXT: call void @test7_consume(i8* [[T0]])
+ // CHECK-NEXT: call void @objc_release(i8* [[T0]])
// CHECK-NEXT: ret void
// CHECK: define internal void @__copy_helper_block_
diff --git a/test/CodeGenObjC/arc-foreach.m b/test/CodeGenObjC/arc-foreach.m
index b8d2d30..bb24855 100644
--- a/test/CodeGenObjC/arc-foreach.m
+++ b/test/CodeGenObjC/arc-foreach.m
@@ -109,8 +109,9 @@
// CHECK-LP64: [[D0:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
// CHECK-LP64: [[T0:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
-// CHECK-LP64-NEXT: [[T1:%.*]] = call i8* @objc_loadWeak(i8** [[X]])
+// CHECK-LP64-NEXT: [[T1:%.*]] = call i8* @objc_loadWeakRetained(i8** [[X]])
// CHECK-LP64-NEXT: call i8* @objc_initWeak(i8** [[T0]], i8* [[T1]])
+// CHECK-LP64-NEXT: call void @objc_release(i8* [[T1]])
// CHECK-LP64-NEXT: [[T1:%.*]] = bitcast [[BLOCK_T]]* [[BLOCK]] to
// CHECK-LP64: call void @use_block
// CHECK-LP64-NEXT: call void @objc_destroyWeak(i8** [[D0]])
diff --git a/test/CodeGenObjC/arc-loadweakretained-release.m b/test/CodeGenObjC/arc-loadweakretained-release.m
new file mode 100644
index 0000000..00d25fa
--- /dev/null
+++ b/test/CodeGenObjC/arc-loadweakretained-release.m
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -o - %s | FileCheck %s
+// rdar://10849570
+
+@interface NSObject @end
+
+@interface SomeClass : NSObject
+- (id) init;
+@end
+
+@implementation SomeClass
+- (void)foo {
+}
+- (id) init {
+ return 0;
+}
++ alloc { return 0; }
+@end
+
+int main (int argc, const char * argv[]) {
+ @autoreleasepool {
+ SomeClass *objPtr1 = [[SomeClass alloc] init];
+ __weak SomeClass *weakRef = objPtr1;
+
+ [weakRef foo];
+
+ objPtr1 = (void *)0;
+ return 0;
+ }
+}
+
+// CHECK: [[SIXTEEN:%.*]] = call i8* @objc_loadWeakRetained(i8** {{%.*}})
+// CHECK-NEXT: [[SEVENTEEN:%.*]] = bitcast i8* [[SIXTEEN]] to {{%.*}}
+// CHECK-NEXT: [[EIGHTEEN:%.*]] = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_6"
+// CHECK-NEXT: [[NINETEEN:%.*]] = bitcast %0* [[SEVENTEEN]] to i8*
+// CHECK-NEXT: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend
+// CHECK-NEXT: [[TWENTY:%.*]] = bitcast %0* [[SEVENTEEN]] to i8*
+// CHECK-NEXT: call void @objc_release(i8* [[TWENTY]])
+
+void test1(int cond) {
+ extern void test34_sink(id *);
+ __weak id weak;
+ test34_sink(cond ? &weak : 0);
+}
+
+// CHECK: define void @test1(
+// CHECK: [[CONDADDR:%.*]] = alloca i32
+// CHECK-NEXT: [[WEAK:%.*]] = alloca i8*
+// CHECK-NEXT: [[INCRTEMP:%.*]] = alloca i8*
+// CHECK-NEXT: [[CONDCLEANUPSAVE:%.*]] = alloca i8*
+// CHECK-NEXT: [[CONDCLEANUP:%.*]] = alloca i1
+// CHECK-NEXT: store i32
+// CHECK-NEXT: store i8* null, i8** [[WEAK]]
+// CHECK: [[COND1:%.*]] = phi i8**
+// CHECK-NEXT: [[ICRISNULL:%.*]] = icmp eq i8** [[COND1]], null
+// CHECK-NEXT: [[ICRARGUMENT:%.*]] = select i1 [[ICRISNULL]], i8** null, i8** [[INCRTEMP]]
+// CHECK-NEXT: store i1 false, i1* [[CONDCLEANUP]]
+// CHECK-NEXT: br i1 [[ICRISNULL]], label [[ICRCONT:%.*]], label [[ICRCOPY:%.*]]
+// CHECK: [[ONE:%.*]] = call i8* @objc_loadWeakRetained(
+// CHECK-NEXT: store i8* [[ONE]], i8** [[CONDCLEANUPSAVE]]
+// CHECK-NEXT: store i1 true, i1* [[CONDCLEANUP]]
+// CHECK-NEXT: store i8* [[ONE]], i8** [[INCRTEMP]]
+// CHECK-NEXT: br label
+
+// CHECK: call void @test34_sink(
+// CHECK-NEXT: [[ICRISNULL1:%.*]] = icmp eq i8** [[COND1]], null
+// CHECK-NEXT: br i1 [[ICRISNULL1]], label [[ICRDONE:%.*]], label [[ICRWRITEBACK:%.*]]
+// CHECK: [[TWO:%.*]] = load i8** [[INCRTEMP]]
+// CHECK-NEXT: [[THREE:%.*]] = call i8* @objc_storeWeak(
+// CHECK-NEXT br label [[ICRDONE]]
+// CHECK: [[CLEANUPISACTIVE:%.*]] = load i1* [[CONDCLEANUP]]
+// CHECK-NEXT: br i1 [[CLEANUPISACTIVE]], label [[CLEASNUPACTION:%.*]], label [[CLEANUPDONE:%.*]]
+
+// CHECK: [[FOUR:%.*]] = load i8** [[CONDCLEANUPSAVE]]
+// CHECK-NEXT: call void @objc_release(i8* [[FOUR]])
+// CHECK-NEXT: br label
+// CHECK: call void @objc_destroyWeak(i8** [[WEAK]])
+// CHECK-NEXT: ret void
diff --git a/test/CodeGenObjC/arc.m b/test/CodeGenObjC/arc.m
index 8e38019..43a7a30 100644
--- a/test/CodeGenObjC/arc.m
+++ b/test/CodeGenObjC/arc.m
@@ -958,6 +958,8 @@
// CHECK-NEXT: [[WEAK:%.*]] = alloca i8*
// CHECK-NEXT: [[TEMP1:%.*]] = alloca i8*
// CHECK-NEXT: [[TEMP2:%.*]] = alloca i8*
+ // CHECK-NEXT: [[CONDCLEANUPSAVE:%.*]] = alloca i8*
+ // CHECK-NEXT: [[CONDCLEANUP:%.*]] = alloca i1
// CHECK-NEXT: store i32
// CHECK-NEXT: store i8* null, i8** [[STRONG]]
// CHECK-NEXT: call i8* @objc_initWeak(i8** [[WEAK]], i8* null)
@@ -986,8 +988,11 @@
// CHECK: [[ARG:%.*]] = phi i8**
// CHECK-NEXT: [[T0:%.*]] = icmp eq i8** [[ARG]], null
// CHECK-NEXT: [[T1:%.*]] = select i1 [[T0]], i8** null, i8** [[TEMP2]]
+ // CHECK-NEXT: store i1 false, i1* [[CONDCLEANUP]]
// CHECK-NEXT: br i1 [[T0]],
- // CHECK: [[T0:%.*]] = call i8* @objc_loadWeak(i8** [[ARG]])
+ // CHECK: [[T0:%.*]] = call i8* @objc_loadWeakRetained(i8** [[ARG]])
+ // CHECK-NEXT: store i8* [[T0]], i8** [[CONDCLEANUPSAVE]]
+ // CHECK-NEXT: store i1 true, i1* [[CONDCLEANUP]]
// CHECK-NEXT: store i8* [[T0]], i8** [[TEMP2]]
// CHECK-NEXT: br label
// CHECK: call void @test34_sink(i8** [[T1]])
diff --git a/test/CodeGenObjCXX/arc.mm b/test/CodeGenObjCXX/arc.mm
index f31b993..aa40f93 100644
--- a/test/CodeGenObjCXX/arc.mm
+++ b/test/CodeGenObjCXX/arc.mm
@@ -61,6 +61,8 @@
// CHECK-NEXT: [[WEAK:%.*]] = alloca i8*
// CHECK-NEXT: [[TEMP1:%.*]] = alloca i8*
// CHECK-NEXT: [[TEMP2:%.*]] = alloca i8*
+ // CHECK-NEXT: [[CONDCLEANUPSAVE:%.*]] = alloca i8*
+ // CHECK-NEXT: [[CONDCLEANUP:%.*]] = alloca i1
// CHECK-NEXT: store i32
// CHECK-NEXT: store i8* null, i8** [[STRONG]]
// CHECK-NEXT: call i8* @objc_initWeak(i8** [[WEAK]], i8* null)
@@ -89,8 +91,11 @@
// CHECK: [[ARG:%.*]] = phi i8**
// CHECK-NEXT: [[T0:%.*]] = icmp eq i8** [[ARG]], null
// CHECK-NEXT: [[T1:%.*]] = select i1 [[T0]], i8** null, i8** [[TEMP2]]
+ // CHECK-NEXT: store i1 false, i1* [[CONDCLEANUP]]
// CHECK-NEXT: br i1 [[T0]],
- // CHECK: [[T0:%.*]] = call i8* @objc_loadWeak(i8** [[ARG]])
+ // CHECK: [[T0:%.*]] = call i8* @objc_loadWeakRetained(i8** [[ARG]])
+ // CHECK-NEXT: store i8* [[T0]], i8** [[CONDCLEANUPSAVE]]
+ // CHECK-NEXT: store i1 true, i1* [[CONDCLEANUP]]
// CHECK-NEXT: store i8* [[T0]], i8** [[TEMP2]]
// CHECK-NEXT: br label
// CHECK: call void @_Z11test34_sinkPU15__autoreleasingP11objc_object(i8** [[T1]])
