[CodeGen] Look at the type of a block capture field rather than the type

of the captured variable when determining whether the capture needs
special handing when the block is copied or disposed.

This fixes bugs in the handling of variables captured by a block that is
nested inside a lambda that captures the variables by reference.

rdar://problem/43540889

Differential Revision: https://reviews.llvm.org/D51025

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

diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 8b8b6ca66f6e72e5f8689dbf4d257948014220ec..170e4f0c0836f0b99539727321e5b4c4511df995 100644 (file)
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -557,6 +557,10 @@ static void computeBlockInfo(CodeGenModule &CGM, CodeGenFunction *CGF,
       CharUnits align = CGM.getPointerAlign();
       maxFieldAlign = std::max(maxFieldAlign, align);
 
+      // Since a __block variable cannot be captured by lambdas, its type and
+      // the capture field type should always match.
+      assert(getCaptureFieldType(*CGF, CI) == variable->getType() &&
+             "capture type differs from the variable type");
       layout.push_back(BlockLayoutChunk(align, CGM.getPointerSize(),
                                         Qualifiers::OCL_None, &CI,
                                         CGM.VoidPtrTy, variable->getType()));
@@ -570,10 +574,11 @@ static void computeBlockInfo(CodeGenModule &CGM, CodeGenFunction *CGF,
       continue;
     }
 
+    QualType VT = getCaptureFieldType(*CGF, CI);
+
     // If we have a lifetime qualifier, honor it for capture purposes.
     // That includes *not* copying it if it's __unsafe_unretained.
-    Qualifiers::ObjCLifetime lifetime =
-      variable->getType().getObjCLifetime();
+    Qualifiers::ObjCLifetime lifetime = VT.getObjCLifetime();
     if (lifetime) {
       switch (lifetime) {
       case Qualifiers::OCL_None: llvm_unreachable("impossible");
@@ -587,10 +592,10 @@ static void computeBlockInfo(CodeGenModule &CGM, CodeGenFunction *CGF,
       }
 
     // Block pointers require copy/dispose.  So do Objective-C pointers.
-    } else if (variable->getType()->isObjCRetainableType()) {
+    } else if (VT->isObjCRetainableType()) {
       // But honor the inert __unsafe_unretained qualifier, which doesn't
       // actually make it into the type system.
-       if (variable->getType()->isObjCInertUnsafeUnretainedType()) {
+       if (VT->isObjCInertUnsafeUnretainedType()) {
         lifetime = Qualifiers::OCL_ExplicitNone;
       } else {
         info.NeedsCopyDispose = true;
@@ -602,21 +607,18 @@ static void computeBlockInfo(CodeGenModule &CGM, CodeGenFunction *CGF,
     } else if (CI.hasCopyExpr()) {
       info.NeedsCopyDispose = true;
       info.HasCXXObject = true;
-      if (!variable->getType()->getAsCXXRecordDecl()->isExternallyVisible())
+      if (!VT->getAsCXXRecordDecl()->isExternallyVisible())
         info.CapturesNonExternalType = true;
 
     // So do C structs that require non-trivial copy construction or
     // destruction.
-    } else if (variable->getType().isNonTrivialToPrimitiveCopy() ==
-                   QualType::PCK_Struct ||
-               variable->getType().isDestructedType() ==
-                   QualType::DK_nontrivial_c_struct) {
+    } else if (VT.isNonTrivialToPrimitiveCopy() == QualType::PCK_Struct ||
+               VT.isDestructedType() == QualType::DK_nontrivial_c_struct) {
       info.NeedsCopyDispose = true;
 
     // And so do types with destructors.
     } else if (CGM.getLangOpts().CPlusPlus) {
-      if (const CXXRecordDecl *record =
-            variable->getType()->getAsCXXRecordDecl()) {
+      if (const CXXRecordDecl *record = VT->getAsCXXRecordDecl()) {
         if (!record->hasTrivialDestructor()) {
           info.HasCXXObject = true;
           info.NeedsCopyDispose = true;
@@ -626,7 +628,6 @@ static void computeBlockInfo(CodeGenModule &CGM, CodeGenFunction *CGF,
       }
     }
 
-    QualType VT = getCaptureFieldType(*CGF, CI);
     CharUnits size = C.getTypeSizeInChars(VT);
     CharUnits align = C.getDeclAlign(variable);
 
@@ -1717,10 +1718,9 @@ static void findBlockCapturedManagedEntities(
     if (Capture.isConstant())
       continue;
 
-    auto CopyInfo =
-       computeCopyInfoForBlockCapture(CI, Variable->getType(), LangOpts);
-    auto DisposeInfo =
-       computeDestroyInfoForBlockCapture(CI, Variable->getType(), LangOpts);
+    QualType VT = Capture.fieldType();
+    auto CopyInfo = computeCopyInfoForBlockCapture(CI, VT, LangOpts);
+    auto DisposeInfo = computeDestroyInfoForBlockCapture(CI, VT, LangOpts);
     if (CopyInfo.first != BlockCaptureEntityKind::None ||
         DisposeInfo.first != BlockCaptureEntityKind::None)
       ManagedCaptures.emplace_back(CopyInfo.first, DisposeInfo.first,

diff --git a/test/CodeGenObjCXX/block-nested-in-lambda.cpp b/test/CodeGenObjCXX/block-nested-in-lambda.cpp
deleted file mode 100644 (file)
index 51b7abf..0000000
--- a/test/CodeGenObjCXX/block-nested-in-lambda.cpp
+++ /dev/null
@@ -1,23 +0,0 @@
-// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -o - %s | FileCheck %s
-
-// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
-// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
-// CHECK: %[[V1:.*]] = load i32*, i32** %[[V0]], align 8
-// CHECK: store i32* %[[V1]], i32** %[[BLOCK_CAPTURED0]], align 8
-// CHECK: %[[BLOCK_CAPTURED1:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK]], i32 0, i32 6
-// CHECK: %[[V2:.*]] = getelementptr inbounds %[[LAMBDA_CLASS]], %[[LAMBDA_CLASS]]* %[[THIS]], i32 0, i32 1
-// CHECK: %[[V3:.*]] = load i32*, i32** %[[V2]], align 8
-// CHECK: store i32* %[[V3]], i32** %[[BLOCK_CAPTURED1]], align 8
-
-void foo1(int &, int &);
-
-void block_in_lambda(int &s1, int &s2) {
-  auto lambda = [&s1, &s2]() {
-    auto block = ^{
-      foo1(s1, s2);
-    };
-    block();
-  };
-
-  lambda();
-}

diff --git a/test/CodeGenObjCXX/block-nested-in-lambda.mm b/test/CodeGenObjCXX/block-nested-in-lambda.mm
new file mode 100644 (file)
index 0000000..a538d1b
--- /dev/null
+++ b/test/CodeGenObjCXX/block-nested-in-lambda.mm
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+
+// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
+// CHECK: %[[V1:.*]] = load i32*, i32** %[[V0]], align 8
+// CHECK: store i32* %[[V1]], i32** %[[BLOCK_CAPTURED0]], align 8
+// CHECK: %[[BLOCK_CAPTURED1:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK]], i32 0, i32 6
+// CHECK: %[[V2:.*]] = getelementptr inbounds %[[LAMBDA_CLASS]], %[[LAMBDA_CLASS]]* %[[THIS]], i32 0, i32 1
+// CHECK: %[[V3:.*]] = load i32*, i32** %[[V2]], align 8
+// CHECK: store i32* %[[V3]], i32** %[[BLOCK_CAPTURED1]], align 8
+
+void foo1(int &, int &);
+
+void block_in_lambda(int &s1, int &s2) {
+  auto lambda = [&s1, &s2]() {
+    auto block = ^{
+      foo1(s1, s2);
+    };
+    block();
+  };
+
+  lambda();
+}
+
+namespace CaptureByReference {
+
+id getObj();
+void use(id);
+
+// Block copy/dispose helpers aren't needed because 'a' is captured by
+// reference.
+
+// CHECK-LABEL: define void @_ZN18CaptureByReference5test0Ev(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_1clEv"(
+// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>* %{{.*}}, i32 0, i32 4
+// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i64 }* @"__block_descriptor_40_e5_v8@?0ls32l8" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+
+void test0() {
+  id a = getObj();
+  [&]{ ^{ a = 0; }(); }();
+}
+
+// Block copy/dispose helpers shouldn't have to retain/release 'a' because it
+// is captured by reference.
+
+// CHECK-LABEL: define void @_ZN18CaptureByReference5test1Ev(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_2clEv"(
+// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 4
+// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @"__block_descriptor_56_8_32s40s_e5_v8@?0l" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_8_32s40s(
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 5
+// CHECK: %[[V5:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 5
+// CHECK: %[[BLOCKCOPY_SRC:.*]] = load i8*, i8** %[[V4]], align 8
+// CHECK: store i8* null, i8** %[[V5]], align 8
+// CHECK: call void @objc_storeStrong(i8** %[[V5]], i8* %[[BLOCKCOPY_SRC]])
+// CHECK: %[[V6:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 6
+// CHECK: %[[V7:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 6
+// CHECK: %[[BLOCKCOPY_SRC2:.*]] = load i8*, i8** %[[V6]], align 8
+// CHECK: store i8* null, i8** %[[V7]], align 8
+// CHECK: call void @objc_storeStrong(i8** %[[V7]], i8* %[[BLOCKCOPY_SRC2]])
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: ret void
+
+// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block_8_32s40s(
+// CHECK: %[[V2:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 5
+// CHECK: %[[V3:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 6
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: call void @objc_storeStrong(i8** %[[V3]], i8* null)
+// CHECK: call void @objc_storeStrong(i8** %[[V2]], i8* null)
+// CHECK-NOT: call void @objc_storeStrong(
+// CHECK: ret void
+
+void test1() {
+  id a = getObj(), b = getObj(), c = getObj();
+  [&a, b, c]{ ^{ a = 0; use(b); use(c); }(); }();
+}
+
+}