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

lib/CodeGen/CGCall.cpp

@@ -1740,7 +1740,12 @@ static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
   // 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 @@ static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
         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 @@ static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
       llvm::BasicBlock *copyBB = CGF.createBasicBlock("icr.copy");
       CGF.Builder.CreateCondBr(isNull, contBB, copyBB);
       CGF.EmitBlock(copyBB);
+      condEval.begin(CGF);
     }
   }
 
@@ -1788,10 +1794,12 @@ static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
     // 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());

lib/CodeGen/CGExpr.cpp

@@ -1119,8 +1119,11 @@ RValue CodeGenFunction::EmitLoadOfLValue(LValue LV) {
     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());

lib/Sema/SemaExpr.cpp

@@ -504,6 +504,12 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
     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));

lib/Sema/SemaInit.cpp

@@ -3846,14 +3846,15 @@ enum InvalidICRKind { IIK_okay, IIK_nonlocal, IIK_nonscalar };
 
 /// 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 @@ static InvalidICRKind isInvalidICRSource(ASTContext &C, Expr *e,
     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 @@ static InvalidICRKind isInvalidICRSource(ASTContext &C, Expr *e,
 
   // 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 @@ static InvalidICRKind isInvalidICRSource(ASTContext &C, Expr *e,
 
   // 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 @@ static InvalidICRKind isInvalidICRSource(ASTContext &C, Expr *e,
 /// 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)

test/CodeGenObjC/arc-blocks.m

@@ -250,7 +250,7 @@ void test7(void) {
   // 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 @@ void test7(void) {
 
   // 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_

test/CodeGenObjC/arc-foreach.m

@@ -109,8 +109,9 @@ void test1(NSArray *array) {
 
 // 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]])

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

test/CodeGenObjC/arc.m

@@ -958,6 +958,8 @@ void test34(int cond) {
   // 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 @@ void test34(int cond) {
   // 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]])

test/CodeGenObjCXX/arc.mm

@@ -61,6 +61,8 @@ void test34(int cond) {
   // 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 @@ void test34(int cond) {
   // 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]])