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

diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 076e20c5fbba16fe5179cd5017f8a5b76cba506c..ce9e30ceba1cf0c68617b3e830915c930747fce6 100644 (file)
--- 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 50fdd0a12c775beb8c42bad76b65a0c281d03070..38675a4393342e7407f43e6001a1aa4e60a9599f 100644 (file)
--- 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
-