Skip to content

Commit 4dfaa77

Browse files
nate-chandlernate-chandler
committed
[IRGen] Pack extra data pattern structs.
Previously, the extra data pattern structs for struct and enums were not packed. On 32 bit, this resulted in an extra data pattern struct which was 4 bytes too large whenever there was an odd number of fields in the struct. The result was writing past the end of the allocated struct. That bug only caused occasional crashes because (1) for the most part there was additional space beyond the end of the allocation intended for the struct metadata in the bump allocator and (2) while half of the trailing flags field would be overwritten, because those bits of the trailing flags being nonzero did not have an observable effect since those bits of the trailing flags field are not yet used. Here, the structs are marked packed, resulting in the appropriate size for the extra data pattern structs on 32 bit platforms. rdar://problem/68997282
1 parent 5aab24b commit 4dfaa77

10 files changed

+30
-28
lines changed

‎lib/IRGen/GenMeta.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4234,6 +4234,7 @@ namespace {
42344234
PartialPattern buildExtraDataPattern() {
42354235
ConstantInitBuilder builder(IGM);
42364236
auto init = builder.beginStruct();
4237+
init.setPacked(true);
42374238

42384239
struct Scanner : StructMetadataScanner<Scanner> {
42394240
GenericStructMetadataBuilder &Outer;
@@ -4602,6 +4603,7 @@ namespace {
46024603
PartialPattern buildExtraDataPattern() {
46034604
ConstantInitBuilder builder(IGM);
46044605
auto init = builder.beginStruct();
4606+
init.setPacked(true);
46054607

46064608
auto &layout = IGM.getMetadataLayout(Target);
46074609

‎test/IRGen/enum_value_semantics_future.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ enum GenericFixedLayout<T> {
161161
// CHECK-SAME: }>
162162

163163
// CHECK: @"$s27enum_value_semantics_future18GenericFixedLayoutOWV" = internal constant %swift.enum_vwtable
164-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant { i64 } zeroinitializer
164+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{ i64 }> zeroinitializer
165165

166166
// CHECK-LABEL: @"$s27enum_value_semantics_future18GenericFixedLayoutOMn" = hidden constant
167167
// CHECK-SAME: [16 x i8*]* @"$s27enum_value_semantics_future18GenericFixedLayoutOMI"
@@ -177,7 +177,7 @@ enum GenericFixedLayout<T> {
177177
// Value witness table.
178178
// CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (%swift.enum_vwtable* @"$s27enum_value_semantics_future18GenericFixedLayoutOWV" to i64), i64 ptrtoint (i32* getelementptr inbounds (<{ i32, i32, i32, i32, i32, i16, i16 }>, <{ i32, i32, i32, i32, i32, i16, i16 }>* @"$s27enum_value_semantics_future18GenericFixedLayoutOMP", i32 0, i32 3) to i64)) to i32)
179179
// Extra data pattern.
180-
// CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint ({ i64 }* [[EXTRA_DATA_PATTERN]] to i64), i64 ptrtoint (i32* getelementptr inbounds (<{ i32, i32, i32, i32, i32, i16, i16 }>, <{ i32, i32, i32, i32, i32, i16, i16 }>* @"$s27enum_value_semantics_future18GenericFixedLayoutOMP", i32 0, i32 4) to i64)) to i32)
180+
// CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (<{ i64 }>* [[EXTRA_DATA_PATTERN]] to i64), i64 ptrtoint (i32* getelementptr inbounds (<{ i32, i32, i32, i32, i32, i16, i16 }>, <{ i32, i32, i32, i32, i32, i16, i16 }>* @"$s27enum_value_semantics_future18GenericFixedLayoutOMP", i32 0, i32 4) to i64)) to i32)
181181
// CHECK-SAME: }>
182182

183183
sil @single_payload_nontrivial_copy_destroy : $(@owned SinglePayloadNontrivial) -> () {

‎test/IRGen/generic_structs.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import Builtin
88

99
// -- Generic structs with fixed layout should have no completion function
1010
// and emit the field offset vector as part of the pattern.
11-
// CHECK: [[PATTERN:@.*]] = internal constant { i32, i32, i32, [4 x i8] } { i32 0, i32 1, i32 8, [4 x i8] zeroinitializer }, align 8
11+
// CHECK: [[PATTERN:@.*]] = internal constant <{ i32, i32, i32, [4 x i8] }> <{ i32 0, i32 1, i32 8, [4 x i8] zeroinitializer }>, align 8
1212
// CHECK-LABEL: @"$s15generic_structs18FixedLayoutGenericVMP" = internal constant <{ {{.*}} }> <{
1313
// -- instantiation function
1414
// CHECK-SAME: %swift.type* (%swift.type_descriptor*, i8**, i8*)* @"$s15generic_structs18FixedLayoutGenericVMi"
@@ -19,7 +19,7 @@ import Builtin
1919
// -- vwtable pointer
2020
// CHECK-SAME: @"$s15generic_structs18FixedLayoutGenericVWV"
2121
// -- extra data pattern
22-
// CHECK-SAME: { i32, i32, i32, [4 x i8] }* [[PATTERN]]
22+
// CHECK-SAME: <{ i32, i32, i32, [4 x i8] }>* [[PATTERN]]
2323
// CHECK-SAME: i16 1,
2424
// CHECK-SAME: i16 2 }>
2525

‎test/IRGen/generic_structs_future.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import Builtin
99

1010
// -- Generic structs with fixed layout should have no completion function
1111
// and emit the field offset vector as part of the pattern.
12-
// CHECK: [[PATTERN:@.*]] = internal constant { i32, i32, i32, [4 x i8], i64 } { i32 0, i32 1, i32 8, [4 x i8] zeroinitializer, i64 0 }, align 8
12+
// CHECK: [[PATTERN:@.*]] = internal constant <{ i32, i32, i32, [4 x i8], i64 }> <{ i32 0, i32 1, i32 8, [4 x i8] zeroinitializer, i64 0 }>, align 8
1313
// CHECK-LABEL: @"$s22generic_structs_future18FixedLayoutGenericVMP" = internal constant <{ {{.*}} }> <{
1414
// -- instantiation function
1515
// CHECK-SAME: %swift.type* (%swift.type_descriptor*, i8**, i8*)* @"$s22generic_structs_future18FixedLayoutGenericVMi"
@@ -20,7 +20,7 @@ import Builtin
2020
// -- vwtable pointer
2121
// CHECK-SAME: @"$s22generic_structs_future18FixedLayoutGenericVWV"
2222
// -- extra data pattern
23-
// CHECK-SAME: { i32, i32, i32, [4 x i8], i64 }* [[PATTERN]]
23+
// CHECK-SAME: <{ i32, i32, i32, [4 x i8], i64 }>* [[PATTERN]]
2424
// CHECK-SAME: i16 1,
2525
// CHECK-SAME: i16 3 }>
2626

‎test/IRGen/prespecialized-metadata/enum-extradata-no_payload_size-trailing_flags.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// UNSUPPORTED: CPU=armv7 && OS=ios
66
// UNSUPPORTED: CPU=armv7s && OS=ios
77

8-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant { i64 } zeroinitializer, align [[ALIGNMENT]]
8+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{ i64 }> zeroinitializer, align [[ALIGNMENT]]
99

1010
// CHECK: @"$s4main6EitherOMP" = internal constant <{
1111
// : i32,
@@ -68,9 +68,9 @@
6868
// : i32 trunc (
6969
// CHECK-SAME: [[INT]] sub (
7070
// CHECK-SAME: [[INT]] ptrtoint (
71-
// CHECK-SAME: {
71+
// CHECK-SAME: <{
7272
// CHECK-SAME: i64
73-
// CHECK-SAME: }* [[EXTRA_DATA_PATTERN]] to [[INT]]
73+
// CHECK-SAME: }>* [[EXTRA_DATA_PATTERN]] to [[INT]]
7474
// CHECK-SAME: ),
7575
// CHECK-SAME: [[INT]] ptrtoint (
7676
// CHECK-SAME: i32* getelementptr inbounds (

‎test/IRGen/prespecialized-metadata/enum-extradata-payload_size-no_trailing_flags.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
// UNSUPPORTED: CPU=armv7s && OS=ios
77

88

9-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant { [[INT]] } {
9+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{ [[INT]] }> <{
1010
// The payload size is 8: the larger payload is the size of an Int64.
1111
// CHECK-SAME: [[INT]] 8
12-
// CHECK-SAME: }, align [[ALIGNMENT]]
12+
// CHECK-SAME: }>, align [[ALIGNMENT]]
1313

1414

1515
// CHECK: @"$s4main6EitherOMP" = internal constant <{
@@ -55,7 +55,7 @@
5555
// : i32 trunc (
5656
// CHECK-SAME: [[INT]] sub (
5757
// CHECK-SAME: [[INT]] ptrtoint (
58-
// CHECK-SAME: { [[INT]] }* [[EXTRA_DATA_PATTERN]] to [[INT]]
58+
// CHECK-SAME: <{ [[INT]] }>* [[EXTRA_DATA_PATTERN]] to [[INT]]
5959
// CHECK-SAME: ),
6060
// CHECK-SAME: [[INT]] ptrtoint (
6161
// CHECK-SAME: i32* getelementptr inbounds (

‎test/IRGen/prespecialized-metadata/enum-extradata-payload_size-trailing_flags.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
66
// UNSUPPORTED: CPU=armv7s && OS=ios
77

88

9-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant { [[INT]], i64 } {
9+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{ [[INT]], i64 }> <{
1010
// The payload size is 8: the larger payload is the size of an Int64.
1111
// CHECK-SAME: [[INT]] 8,
1212
// CHECK-SAME: i64 0
13-
// CHECK-SAME: }, align [[ALIGNMENT]]
13+
// CHECK-SAME: }>, align [[ALIGNMENT]]
1414

1515

1616
// CHECK: @"$s4main6EitherOMP" = internal constant <{
@@ -56,7 +56,7 @@
5656
// : i32 trunc (
5757
// CHECK-SAME: [[INT]] sub (
5858
// CHECK-SAME: [[INT]] ptrtoint (
59-
// CHECK-SAME: { [[INT]], i64 }* [[EXTRA_DATA_PATTERN]] to [[INT]]
59+
// CHECK-SAME: <{ [[INT]], i64 }>* [[EXTRA_DATA_PATTERN]] to [[INT]]
6060
// CHECK-SAME: ),
6161
// CHECK-SAME: [[INT]] ptrtoint (
6262
// CHECK-SAME: i32* getelementptr inbounds (

‎test/IRGen/prespecialized-metadata/struct-extradata-field_offsets-no_trailing_flags.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,17 @@
55
// UNSUPPORTED: CPU=armv7 && OS=ios
66
// UNSUPPORTED: CPU=armv7s && OS=ios
77

8-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant {
8+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{
99
// CHECK-SAME: i32
1010
// CHECK-SAME: , i32
1111
// CHECK-SAME: , i32
1212
// : , [4 x i8]
13-
// CHECK-SAME: } {
13+
// CHECK-SAME: }> <{
1414
// CHECK-SAME: i32 0,
1515
// CHECK-SAME: i32 8,
1616
// CHECK-SAME: i32 16
1717
// : , [4 x i8] zeroinitializer
18-
// CHECK-SAME: }, align [[ALIGNMENT]]
18+
// CHECK-SAME: }>, align [[ALIGNMENT]]
1919
// CHECK: @"$s4main4PairVMP" = internal constant <{
2020
// : i32,
2121
// : i32,
@@ -60,11 +60,11 @@
6060
// : i32 trunc (
6161
// CHECK-SAME: [[INT]] sub (
6262
// CHECK-SAME: [[INT]] ptrtoint (
63-
// CHECK-SAME: { i32
63+
// CHECK-SAME: <{ i32
6464
// CHECK-SAME: , i32
6565
// CHECK-SAME: , i32
6666
// : , [4 x i8]
67-
// CHECK-SAME: }* [[EXTRA_DATA_PATTERN]] to [[INT]]
67+
// CHECK-SAME: }>* [[EXTRA_DATA_PATTERN]] to [[INT]]
6868
// CHECK-SAME: ),
6969
// CHECK-SAME: [[INT]] ptrtoint (
7070
// CHECK-SAME: i32* getelementptr inbounds (

‎test/IRGen/prespecialized-metadata/struct-extradata-field_offsets-trailing_flags.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,19 @@
55
// UNSUPPORTED: CPU=armv7 && OS=ios
66
// UNSUPPORTED: CPU=armv7s && OS=ios
77

8-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant {
8+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{
99
// CHECK-SAME: i32,
1010
// CHECK-SAME: i32,
1111
// CHECK-SAME: i32,
1212
// : , [4 x i8],
1313
// CHECK-SAME: i64
14-
// CHECK-SAME: } {
14+
// CHECK-SAME: }> <{
1515
// CHECK-SAME: i32 0,
1616
// CHECK-SAME: i32 8,
1717
// CHECK-SAME: i32 16,
1818
// : [4 x i8] zeroinitializer,
1919
// CHECK-SAME: i64 0
20-
// CHECK-SAME: }, align [[ALIGNMENT]]
20+
// CHECK-SAME: }>, align [[ALIGNMENT]]
2121
// CHECK: @"$s4main4PairVMP" = internal constant <{
2222
// : i32,
2323
// : i32,
@@ -62,13 +62,13 @@
6262
// : i32 trunc (
6363
// CHECK-SAME: [[INT]] sub (
6464
// CHECK-SAME: [[INT]] ptrtoint (
65-
// CHECK-SAME: {
65+
// CHECK-SAME: <{
6666
// CHECK-SAME: i32,
6767
// CHECK-SAME: i32,
6868
// CHECK-SAME: i32,
6969
// : [4 x i8],
7070
// CHECK-SAME: i64
71-
// CHECK-SAME: }* [[EXTRA_DATA_PATTERN]] to [[INT]]
71+
// CHECK-SAME: }>* [[EXTRA_DATA_PATTERN]] to [[INT]]
7272
// CHECK-SAME: ),
7373
// CHECK-SAME: [[INT]] ptrtoint (
7474
// CHECK-SAME: i32* getelementptr inbounds (

‎test/IRGen/prespecialized-metadata/struct-extradata-no_field_offsets-trailing_flags.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// UNSUPPORTED: CPU=armv7 && OS=ios
66
// UNSUPPORTED: CPU=armv7s && OS=ios
77

8-
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant { i64 } zeroinitializer, align [[ALIGNMENT]]
8+
// CHECK: [[EXTRA_DATA_PATTERN:@[0-9]+]] = internal constant <{ i64 }> zeroinitializer, align [[ALIGNMENT]]
99

1010
// CHECK: @"$s4main4PairVMP" = internal constant <{
1111
// : i32,
@@ -68,9 +68,9 @@
6868
// : i32 trunc (
6969
// CHECK-SAME: [[INT]] sub (
7070
// CHECK-SAME: [[INT]] ptrtoint (
71-
// CHECK-SAME: {
71+
// CHECK-SAME: <{
7272
// CHECK-SAME: i64
73-
// CHECK-SAME: }* [[EXTRA_DATA_PATTERN]] to [[INT]]
73+
// CHECK-SAME: }>* [[EXTRA_DATA_PATTERN]] to [[INT]]
7474
// CHECK-SAME: ),
7575
// CHECK-SAME: [[INT]] ptrtoint (
7676
// CHECK-SAME: i32* getelementptr inbounds (

0 commit comments

Comments
 (0)