jfb created this revision.
jfb added reviewers: rjmccall, pcc, kcc.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.
Automatic initialization [1] of __block variables was happening too late, which
caused self-init usage to crash, such as here:
typedef struct XYZ { void (^block)(); } *xyz_t;
__attribute__((noinline))
xyz_t create(void (^block)()) {
xyz_t myself = malloc(sizeof(struct XYZ));
myself->block = block;
return myself;
}
int main() {
__block xyz_t captured = create(^(){ (void)captured; });
}
This type of code shouldn't be broken by variable auto-init, even if it's
sketchy.
This change moves some GEP generation to earlier to ensure that this
initialization occurs at the right time, hence the extra few tests being
modified.
[1] With -ftrivial-auto-var-init=pattern
rdar://problem/47798396
Repository:
rC Clang
https://reviews.llvm.org/D57797
Files:
lib/CodeGen/CGDecl.cpp
test/CodeGenCXX/trivial-auto-var-init.cpp
test/CodeGenObjC/arc-blocks.m
test/CodeGenObjC/blocks-2.m
test/CodeGenObjC/blocks.m
test/CodeGenObjCXX/arc-blocks.mm
Index: test/CodeGenObjCXX/arc-blocks.mm
===================================================================
--- test/CodeGenObjCXX/arc-blocks.mm
+++ test/CodeGenObjCXX/arc-blocks.mm
@@ -24,14 +24,14 @@
}
// CHECK-LABEL: define void @_ZN5test03fooEv()
// CHECK: [[V:%.*]] = alloca [[BYREF_A:%.*]], align 8
+ // CHECK: [[V1:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 7
// CHECK: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 4
// CHECK-NEXT: store i8* bitcast (void (i8*, i8*)* [[COPY_HELPER:@.*]] to i8*), i8** [[T0]]
// CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 5
// CHECK-NEXT: store i8* bitcast (void (i8*)* [[DISPOSE_HELPER:@.*]] to i8*), i8** [[T0]]
// CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 6
// CHECK-NEXT: store i8* getelementptr inbounds ([3 x i8], [3 x i8]* [[LAYOUT0]], i32 0, i32 0), i8** [[T0]]
- // CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 7
- // CHECK-NEXT: call void @_ZN5test01AC1Ev([[A]]* [[T0]])
+ // CHECK-NEXT: call void @_ZN5test01AC1Ev([[A]]* [[V1]])
// CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 7
// CHECK: bitcast [[BYREF_A]]* [[V]] to i8*
// CHECK: [[T1:%.*]] = bitcast [[BYREF_A]]* [[V]] to i8*
Index: test/CodeGenObjC/blocks.m
===================================================================
--- test/CodeGenObjC/blocks.m
+++ test/CodeGenObjC/blocks.m
@@ -48,6 +48,7 @@
// CHECK-NEXT: [[WEAKX:%.*]] = alloca [[WEAK_T:%.*]],
// CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
// CHECK-NEXT: store [[TEST2]]*
+ // CHECK-NEXT: [[T6:%.*]] = getelementptr inbounds [[WEAK_T]], [[WEAK_T]]* [[WEAKX]], i32 0, i32 6
// isa=1 for weak byrefs.
// CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[WEAK_T]], [[WEAK_T]]* [[WEAKX]], i32 0, i32 0
@@ -72,7 +73,6 @@
// CHECK-NEXT: store i8* bitcast (void (i8*)* @__Block_byref_object_dispose_{{.*}} to i8*), i8** [[T5]]
// Actually capture the value.
- // CHECK-NEXT: [[T6:%.*]] = getelementptr inbounds [[WEAK_T]], [[WEAK_T]]* [[WEAKX]], i32 0, i32 6
// CHECK-NEXT: [[CAPTURE:%.*]] = load [[TEST2]]*, [[TEST2]]** [[X]]
// CHECK-NEXT: store [[TEST2]]* [[CAPTURE]], [[TEST2]]** [[T6]]
Index: test/CodeGenObjC/blocks-2.m
===================================================================
--- test/CodeGenObjC/blocks-2.m
+++ test/CodeGenObjC/blocks-2.m
@@ -20,7 +20,7 @@
// CHECK: [[N:%.*]] = alloca [[N_T:%.*]], align 8
// CHECK: [[T0:%.*]] = getelementptr inbounds [[N_T]], [[N_T]]* [[N]], i32 0, i32 4
- // CHECK-NEXT: store double 1.000000e+{{0?}}01, double* [[T0]], align 8
+ // CHECK: store double 1.000000e+{{0?}}01, double* [[T0]], align 8
__block double n = 10;
// CHECK: invoke void @{{.*}}test1_help
Index: test/CodeGenObjC/arc-blocks.m
===================================================================
--- test/CodeGenObjC/arc-blocks.m
+++ test/CodeGenObjC/arc-blocks.m
@@ -122,11 +122,11 @@
// CHECK-LABEL: define void @test4()
// CHECK: [[VAR:%.*]] = alloca [[BYREF_T:%.*]],
// CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
+ // CHECK: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
// CHECK: [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 2
// 0x02000000 - has copy/dispose helpers strong
// CHECK-NEXT: store i32 838860800, i32* [[T0]]
- // CHECK: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
- // CHECK-NEXT: [[T0:%.*]] = call i8* @test4_source()
+ // CHECK: [[T0:%.*]] = call i8* @test4_source()
// CHECK-NEXT: [[T1:%.*]] = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* [[T0]])
// CHECK-NEXT: store i8* [[T1]], i8** [[SLOT]]
// CHECK-NEXT: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
@@ -207,11 +207,11 @@
// CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
// CHECK-NEXT: [[VARPTR1:%.*]] = bitcast [[BYREF_T]]* [[VAR]] to i8*
// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 48, i8* [[VARPTR1]])
+ // CHECK: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
// CHECK: [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 2
// 0x02000000 - has copy/dispose helpers weak
// CHECK-NEXT: store i32 1107296256, i32* [[T0]]
- // CHECK: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
- // CHECK-NEXT: [[T0:%.*]] = call i8* @test6_source()
+ // CHECK: [[T0:%.*]] = call i8* @test6_source()
// CHECK-NEXT: [[T1:%.*]] = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* [[T0]])
// CHECK-NEXT: call i8* @llvm.objc.initWeak(i8** [[SLOT]], i8* [[T1]])
// CHECK-NEXT: call void @llvm.objc.release(i8* [[T1]])
@@ -219,7 +219,7 @@
// 0x42800000 - has signature, copy/dispose helpers, as well as BLOCK_HAS_EXTENDED_LAYOUT
// CHECK: store i32 -1040187392,
// 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*, i8*, i8*, i64 }* @[[BLOCK_DESCRIPTOR_TMP9]] to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+ // CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @[[BLOCK_DESCRIPTOR_TMP9]] to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
// CHECK: [[T0:%.*]] = bitcast [[BYREF_T]]* [[VAR]] to i8*
// CHECK-NEXT: store i8* [[T0]], i8**
// CHECK: call void @test6_helper(
@@ -401,10 +401,10 @@
// CHECK-LABEL: define void @test10b()
// CHECK: [[BYREF:%.*]] = alloca [[BYREF_T:%.*]],
+ // CHECK: [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[BYREF]], i32 0, i32 6
// Zero-initialize.
- // CHECK: [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[BYREF]], i32 0, i32 6
- // CHECK-NEXT: store void ()* null, void ()** [[T0]], align 8
+ // CHECK: store void ()* null, void ()** [[T0]], align 8
// CHECK-NEXT: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[BYREF]], i32 0, i32 6
Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===================================================================
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -30,6 +30,32 @@
used(block);
}
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init before
+// any of said book-keeping.
+//
+// UNINIT-LABEL: test_block_self_init(
+// ZERO-LABEL: test_block_self_init(
+// ZERO: %captured = alloca %struct.__block_byref_captured, align 8
+// ZERO-NEXT: %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// ZERO-NEXT: %0 = bitcast %struct.__block_byref_captured* %captured to i8*
+// ZERO-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 %0, i8 0, i64 32, i1 false)
+// PATTERN-LABEL: test_block_self_init(
+// PATTERN: %captured = alloca %struct.__block_byref_captured, align 8
+// PATTERN-NEXT: %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// PATTERN-NEXT: %0 = bitcast %struct.__block_byref_captured* %captured to i8*
+// PATTERN-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %0, i8* align 8 bitcast (%struct.__block_byref_captured* @__const.test_block_self_init.captured to i8*), i64 32, i1 false)
+void test_block_self_init() {
+ using Block = void (^)();
+ typedef struct XYZ {
+ Block block;
+ } * xyz_t;
+ extern xyz_t create(Block block);
+ __block xyz_t captured = create(^() {
+ (void)captured;
+ });
+}
+
// This type of code is currently not handled by zero / pattern initialization.
// The test will break when that is fixed.
// UNINIT-LABEL: test_goto_unreachable_value(
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1599,19 +1599,16 @@
EnsureInsertPoint();
}
- // Initialize the structure of a __block variable.
- if (emission.IsEscapingByRef)
- emitByrefStructureInit(emission);
-
// Initialize the variable here if it doesn't have a initializer and it is a
// C struct that is non-trivial to initialize or an array containing such a
// struct.
- if (!Init &&
- type.isNonTrivialToPrimitiveDefaultInitialize() ==
- QualType::PDIK_Struct) {
+ if (!Init && type.isNonTrivialToPrimitiveDefaultInitialize() ==
+ QualType::PDIK_Struct) {
LValue Dst = MakeAddrLValue(emission.getAllocatedAddress(), type);
- if (emission.IsEscapingByRef)
+ if (emission.IsEscapingByRef) {
+ emitByrefStructureInit(emission);
drillIntoBlockVariable(*this, Dst, &D);
+ }
defaultInitNonTrivialCStructVar(Dst);
return;
}
@@ -1633,10 +1630,14 @@
? LangOptions::TrivialAutoVarInitKind::Uninitialized
: getContext().getLangOpts().getTrivialAutoVarInit()));
+ bool trivialInitializationDone = false;
auto initializeWhatIsTechnicallyUninitialized = [&]() {
if (trivialAutoVarInit ==
LangOptions::TrivialAutoVarInitKind::Uninitialized)
return;
+ if (trivialInitializationDone)
+ return;
+ trivialInitializationDone = true;
CharUnits Size = getContext().getTypeSizeInChars(type);
if (!Size.isZero()) {
@@ -1714,6 +1715,12 @@
}
};
+ // Initialize the structure of a __block variable.
+ if (emission.IsEscapingByRef) {
+ initializeWhatIsTechnicallyUninitialized();
+ emitByrefStructureInit(emission);
+ }
+
if (isTrivialInitializer(Init)) {
initializeWhatIsTechnicallyUninitialized();
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
