Emit the copy and dipose helpers for ARC __strong

block-typed __block variables using objc_retainBlock
and objc_dispose.  Previously we were using
_Block_object_assign and _Block_object_destroy
with BLOCK_BYREF_CALLER, which causes the runtime
to completely ignore the retain and release.
In most cases this doesn't cause catastrophe
because the retain/release are balanced and
because the block in the variable was copied
upon assignment there.  However, the stack
copy of the variable will be released when
it goes out of scope, which is a problem if
that value was released due to an assignment
to the heap copy.  Similarly, a leak can occur
if the variable is assigned after the copy to
the heap.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@144162 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 76 insertions, 11 deletions

diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 076e20c5fb..ce9e30ceba 100644
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -1342,15 +1342,23 @@ public:
     // Do a "move" by copying the value and then zeroing out the old
     // variable.
 
-    llvm::Value *value = CGF.Builder.CreateLoad(srcField);
+    llvm::LoadInst *value = CGF.Builder.CreateLoad(srcField);
+    value->setAlignment(Alignment.getQuantity());
+    
     llvm::Value *null =
       llvm::ConstantPointerNull::get(cast<llvm::PointerType>(value->getType()));
-    CGF.Builder.CreateStore(value, destField);
-    CGF.Builder.CreateStore(null, srcField);
+
+    llvm::StoreInst *store = CGF.Builder.CreateStore(value, destField);
+    store->setAlignment(Alignment.getQuantity());
+
+    store = CGF.Builder.CreateStore(null, srcField);
+    store->setAlignment(Alignment.getQuantity());
   }
 
   void emitDispose(CodeGenFunction &CGF, llvm::Value *field) {
-    llvm::Value *value = CGF.Builder.CreateLoad(field);
+    llvm::LoadInst *value = CGF.Builder.CreateLoad(field);
+    value->setAlignment(Alignment.getQuantity());
+
     CGF.EmitARCRelease(value, /*precise*/ false);
   }
 
@@ -1360,6 +1368,39 @@ public:
   }
 };
 
+/// Emits the copy/dispose helpers for an ARC __block __strong
+/// variable that's of block-pointer type.
+class ARCStrongBlockByrefHelpers : public CodeGenModule::ByrefHelpers {
+public:
+  ARCStrongBlockByrefHelpers(CharUnits alignment) : ByrefHelpers(alignment) {}
+
+  void emitCopy(CodeGenFunction &CGF, llvm::Value *destField,
+                llvm::Value *srcField) {
+    // Do the copy with objc_retainBlock; that's all that
+    // _Block_object_assign would do anyway, and we'd have to pass the
+    // right arguments to make sure it doesn't get no-op'ed.
+    llvm::LoadInst *oldValue = CGF.Builder.CreateLoad(srcField);
+    oldValue->setAlignment(Alignment.getQuantity());
+
+    llvm::Value *copy = CGF.EmitARCRetainBlock(oldValue, /*mandatory*/ true);
+
+    llvm::StoreInst *store = CGF.Builder.CreateStore(copy, destField);
+    store->setAlignment(Alignment.getQuantity());
+  }
+
+  void emitDispose(CodeGenFunction &CGF, llvm::Value *field) {
+    llvm::LoadInst *value = CGF.Builder.CreateLoad(field);
+    value->setAlignment(Alignment.getQuantity());
+
+    CGF.EmitARCRelease(value, /*precise*/ false);
+  }
+
+  void profileImpl(llvm::FoldingSetNodeID &id) const {
+    // 2 is distinguishable from all pointers and byref flags
+    id.AddInteger(2);
+  }
+};
+
 /// Emits the copy/dispose helpers for a __block variable with a
 /// nontrivial copy constructor or destructor.
 class CXXByrefHelpers : public CodeGenModule::ByrefHelpers {
@@ -1586,13 +1627,10 @@ CodeGenFunction::buildByrefHelpers(llvm::StructType &byrefType,
 
     // ARC __strong __block variables need to be retained.
     case Qualifiers::OCL_Strong:
-      // Block-pointers need to be _Block_copy'ed, so we let the
-      // runtime be in charge.  But we can't use the code below
-      // because we don't want to set BYREF_CALLER, which will
-      // just make the runtime ignore us.
+      // Block pointers need to be copied, and there's no direct
+      // transfer possible.
       if (type->isBlockPointerType()) {
-        BlockFieldFlags flags = BLOCK_FIELD_IS_BLOCK;
-        ObjectByrefHelpers byrefInfo(emission.Alignment, flags);
+        ARCStrongBlockByrefHelpers byrefInfo(emission.Alignment);
         return ::buildByrefHelpers(CGM, byrefType, byrefInfo);
 
       // Otherwise, we transfer ownership of the retain from the stack
diff --git a/test/CodeGenObjC/arc-blocks.m b/test/CodeGenObjC/arc-blocks.m
index 50fdd0a12c..38675a4393 100644
--- a/test/CodeGenObjC/arc-blocks.m
+++ b/test/CodeGenObjC/arc-blocks.m
@@ -334,6 +334,34 @@ void test10a(void) {
   // CHECK-NEXT: ret void
 }
 
+// <rdar://problem/10402698>: do this copy and dispose with
+// objc_retainBlock/release instead of _Block_object_assign/destroy.
+// We can also use _Block_object_assign/destroy with
+// BLOCK_FIELD_IS_BLOCK as long as we don't pass BLOCK_BYREF_CALLER.
+
+// CHECK: define internal void @__Block_byref_object_copy
+// CHECK:      [[D0:%.*]] = load i8** {{%.*}}
+// CHECK-NEXT: [[D1:%.*]] = bitcast i8* [[D0]] to [[BYREF_T]]*
+// CHECK-NEXT: [[D2:%.*]] = getelementptr inbounds [[BYREF_T]]* [[D1]], i32 0, i32 6
+// CHECK-NEXT: [[S0:%.*]] = load i8** {{%.*}}
+// CHECK-NEXT: [[S1:%.*]] = bitcast i8* [[S0]] to [[BYREF_T]]*
+// CHECK-NEXT: [[S2:%.*]] = getelementptr inbounds [[BYREF_T]]* [[S1]], i32 0, i32 6
+// CHECK-NEXT: [[T0:%.*]] = load void ()** [[S2]], align 8
+// CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
+// CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retainBlock(i8* [[T1]])
+// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to void ()*
+// CHECK-NEXT: store void ()* [[T3]], void ()** [[D2]], align 8
+// CHECK-NEXT: ret void
+
+// CHECK: define internal void @__Block_byref_object_dispose
+// CHECK:      [[T0:%.*]] = load i8** {{%.*}}
+// CHECK-NEXT: [[T1:%.*]] = bitcast i8* [[T0]] to [[BYREF_T]]*
+// CHECK-NEXT: [[T2:%.*]] = getelementptr inbounds [[BYREF_T]]* [[T1]], i32 0, i32 6
+// CHECK-NEXT: [[T3:%.*]] = load void ()** [[T2]], align 8
+// CHECK-NEXT: [[T4:%.*]] = bitcast void ()* [[T3]] to i8*
+// CHECK-NEXT: call void @objc_release(i8* [[T4]])
+// CHECK-NEXT: ret void
+
 // Test that we correctly assign to __block variables when the
 // assignment captures the variable.
 void test10b(void) {
@@ -427,4 +455,3 @@ void test11b(void) {
 // CHECK:    define internal void @"\01-[Test12 setNblock:]"(
 // CHECK:    call void @objc_setProperty(i8* {{%.*}}, i8* {{%.*}}, i64 {{%.*}}, i8* {{%.*}}, i1 zeroext false, i1 zeroext true)
 @end
-