From: Akira Hatanaka Date: Wed, 22 Aug 2018 13:41:19 +0000 (+0000) Subject: [CodeGen] Look at the type of a block capture field rather than the type X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f79d76960797fa08c863b6ddfdd825194d40658d;p=clang [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 8b8b6ca66f..170e4f0c08 100644 --- 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 index 51b7abf034..0000000000 --- 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 index 0000000000..a538d1bd2b --- /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); }(); }(); +} + +}