Skip to content

Commit ba1caec

Browse files
nate-chandlernate-chandler
committed
[metadata prespecialization] Zero trailing flags.
Previously, the trailing flags field of runtime instantiated generic metadata for types which had prespecialization enabled were not zeroed. Consequently, the field always contained garbage. Often, metadata was instantiated on new (and so, zeroed) pages, so the garbage happened to be zero as is appropriate. However, when the metadata was instantiated on pages which had previously been dirtied, the garbage value would sometimes indicate that the metadata was canonical statically specialized. When that occurred, swift_checkMetadataState would incorrectly return a metadata state of complete, despite the fact that the metadata might not in fact be complete. As a result, the runtime was trafficking in incomplete metadata as if it were complete, resulting in various crashes that would arise from for example missing a witness table or a value witness table. Here the problem is corrected. The trailing flags field of structs and enums that have the field is set to 0. For structs, this is accomplished by modifying the extra data pattern to exist not only when there is fixed type info for the type but also when the type is being prespecialized. Specifically, the extra data for a struct is, rather than an i32 array of field offsets, a struct consisting of none, one, or both of the following: (1) the array of field offsets, (2) the trailing flags. Similarly, enums now have an extra data pattern which consists of none, one, or both of the following: (1) the payload size, (2) the trailing flags. Enum metadata extra data setting was previously achieved by customizing the metadata allocation function; that customization is now eliminated, being replaced with the shared runtime code for copying extra data into place, a modest code size savings. rdar://problem/61465515
1 parent cf66f7d commit ba1caec

27 files changed

+1371
-35
lines changed

‎lib/IRGen/ConstantBuilder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ class ConstantAggregateBuilderBase
7979

8080
void addInt64(uint64_t value) { addInt(IGM().Int64Ty, value); }
8181

82+
void addSize(Size size) { addInt(IGM().SizeTy, size.getValue()); }
83+
8284
void addRelativeAddressOrNull(llvm::Constant *target) {
8385
if (target) {
8486
addRelativeAddress(target);

‎lib/IRGen/GenMeta.cpp

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3731,7 +3731,7 @@ namespace {
37313731
GenericMetadataPatternFlags getPatternFlags() {
37323732
auto flags = super::getPatternFlags();
37333733

3734-
if (IGM.shouldPrespecializeGenericMetadata()) {
3734+
if (hasTrailingFlags()) {
37353735
flags.setHasTrailingFlags(true);
37363736
}
37373737

@@ -3744,7 +3744,11 @@ namespace {
37443744
HasDependentVWT);
37453745
}
37463746

3747-
bool hasExtraDataPattern() {
3747+
bool hasTrailingFlags() {
3748+
return IGM.shouldPrespecializeGenericMetadata();
3749+
}
3750+
3751+
bool hasKnownFieldOffsets() {
37483752
auto &ti = IGM.getTypeInfo(getLoweredType());
37493753
if (!isa<FixedTypeInfo>(ti))
37503754
return false;
@@ -3755,19 +3759,31 @@ namespace {
37553759
return true;
37563760
}
37573761

3758-
/// Fill in a constant field offset vector if possible.
3762+
bool hasExtraDataPattern() {
3763+
return hasKnownFieldOffsets() || hasTrailingFlags();
3764+
}
3765+
3766+
/// If present, the extra data pattern consists of one or both of the
3767+
/// following:
3768+
///
3769+
/// - the field offset vector
3770+
/// - the trailing flags
37593771
PartialPattern buildExtraDataPattern() {
37603772
ConstantInitBuilder builder(IGM);
3761-
auto init = builder.beginArray(IGM.Int32Ty);
3773+
auto init = builder.beginStruct();
37623774

37633775
struct Scanner : StructMetadataScanner<Scanner> {
3776+
GenericStructMetadataBuilder &Outer;
37643777
SILType Type;
3765-
ConstantArrayBuilder &B;
3766-
Scanner(IRGenModule &IGM, StructDecl *target, SILType type,
3767-
ConstantArrayBuilder &B)
3768-
: StructMetadataScanner(IGM, target), Type(type), B(B) {}
3778+
ConstantStructBuilder &B;
3779+
Scanner(GenericStructMetadataBuilder &outer, IRGenModule &IGM, StructDecl *target, SILType type,
3780+
ConstantStructBuilder &B)
3781+
: StructMetadataScanner(IGM, target), Outer(outer), Type(type), B(B) {}
37693782

37703783
void addFieldOffset(VarDecl *field) {
3784+
if (!Outer.hasKnownFieldOffsets()) {
3785+
return;
3786+
}
37713787
auto offset = emitPhysicalStructMemberFixedOffset(IGM, Type, field);
37723788
if (offset) {
37733789
B.add(offset);
@@ -3783,22 +3799,28 @@ namespace {
37833799
void noteEndOfFieldOffsets() {
37843800
B.addAlignmentPadding(IGM.getPointerAlignment());
37853801
}
3802+
3803+
void addTrailingFlags() { B.addInt64(0); }
37863804
};
3787-
Scanner(IGM, Target, getLoweredType(), init).layout();
3788-
Size vectorSize = init.getNextOffsetFromGlobal();
3805+
Scanner(*this, IGM, Target, getLoweredType(), init).layout();
3806+
Size structSize = init.getNextOffsetFromGlobal();
37893807

37903808
auto global = init.finishAndCreateGlobal("", IGM.getPointerAlignment(),
37913809
/*constant*/ true);
37923810

37933811
auto &layout = IGM.getMetadataLayout(Target);
3812+
3813+
bool offsetUpToTrailingFlags = hasTrailingFlags() && !hasKnownFieldOffsets();
3814+
Size zeroingStart = IGM.getOffsetOfStructTypeSpecificMetadataMembers();
3815+
Offset zeroingEnd = offsetUpToTrailingFlags
3816+
? layout.getTrailingFlagsOffset()
3817+
: layout.getFieldOffsetVectorOffset();
37943818
return { global,
3795-
layout.getFieldOffsetVectorOffset().getStatic()
3796-
- IGM.getOffsetOfStructTypeSpecificMetadataMembers(),
3797-
vectorSize };
3819+
zeroingEnd.getStatic()
3820+
- zeroingStart,
3821+
structSize };
37983822
}
37993823

3800-
void addTrailingFlags() { this->B.addInt(IGM.Int64Ty, 0); }
3801-
38023824
bool hasCompletionFunction() {
38033825
return !isa<FixedTypeInfo>(IGM.getTypeInfo(getLoweredType()));
38043826
}
@@ -4172,24 +4194,58 @@ namespace {
41724194
descriptor = emitPointerAuthSign(IGF, descriptor, authInfo);
41734195
}
41744196

4175-
auto metadata =
4197+
return
41764198
IGF.Builder.CreateCall(IGM.getAllocateGenericValueMetadataFn(),
41774199
{descriptor, arguments, templatePointer,
41784200
extraSizeV});
4201+
}
4202+
4203+
bool hasTrailingFlags() {
4204+
return IGM.shouldPrespecializeGenericMetadata();
4205+
}
4206+
4207+
bool hasKnownPayloadSize() {
4208+
auto &layout = IGM.getMetadataLayout(Target);
4209+
return layout.hasPayloadSizeOffset() && (bool)getConstantPayloadSize(IGM, Target);
4210+
}
4211+
4212+
bool hasExtraDataPattern() {
4213+
return hasKnownPayloadSize() || hasTrailingFlags();
4214+
}
4215+
4216+
/// If present, the extra data pattern consists of one or both of the
4217+
/// following:
4218+
///
4219+
/// - the payload-size
4220+
/// - the trailing flags
4221+
PartialPattern buildExtraDataPattern() {
4222+
ConstantInitBuilder builder(IGM);
4223+
auto init = builder.beginStruct();
4224+
4225+
auto &layout = IGM.getMetadataLayout(Target);
41794226

4180-
// Initialize the payload-size field if we have a constant value for it.
4181-
// This is so small that we just do it inline instead of bothering
4182-
// with a pattern.
41834227
if (layout.hasPayloadSizeOffset()) {
41844228
if (auto size = getConstantPayloadSize(IGM, Target)) {
4185-
auto offset = layout.getPayloadSizeOffset();
4186-
auto slot = IGF.emitAddressAtOffset(metadata, offset, IGM.SizeTy,
4187-
IGM.getPointerAlignment());
4188-
IGF.Builder.CreateStore(IGM.getSize(*size), slot);
4229+
init.addSize(*size);
41894230
}
41904231
}
4232+
if (hasTrailingFlags()) {
4233+
init.addInt64(0);
4234+
}
4235+
Size structSize = init.getNextOffsetFromGlobal();
41914236

4192-
return metadata;
4237+
auto global = init.finishAndCreateGlobal("", IGM.getPointerAlignment(),
4238+
/*constant*/ true);
4239+
4240+
bool offsetUpToTrailingFlags = hasTrailingFlags() && !hasKnownPayloadSize();
4241+
Size zeroingStart = IGM.getOffsetOfEnumTypeSpecificMetadataMembers();
4242+
Offset zeroingEnd = offsetUpToTrailingFlags
4243+
? layout.getTrailingFlagsOffset()
4244+
: layout.getPayloadSizeOffset();
4245+
return { global,
4246+
zeroingEnd.getStatic()
4247+
- zeroingStart,
4248+
structSize };
41934249
}
41944250

41954251
llvm::Constant *emitNominalTypeDescriptor() {
@@ -4199,7 +4255,7 @@ namespace {
41994255
GenericMetadataPatternFlags getPatternFlags() {
42004256
auto flags = super::getPatternFlags();
42014257

4202-
if (IGM.shouldPrespecializeGenericMetadata()) {
4258+
if (hasTrailingFlags()) {
42034259
flags.setHasTrailingFlags(true);
42044260
}
42054261

‎lib/IRGen/MetadataLayout.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,11 @@ EnumMetadataLayout::EnumMetadataLayout(IRGenModule &IGM, EnumDecl *decl)
543543
super::noteStartOfGenericRequirements();
544544
}
545545

546+
void addTrailingFlags() {
547+
Layout.TrailingFlagsOffset = getNextOffset();
548+
super::addTrailingFlags();
549+
}
550+
546551
void layout() {
547552
super::layout();
548553
Layout.TheSize = getMetadataSize();
@@ -558,6 +563,11 @@ EnumMetadataLayout::getPayloadSizeOffset() const {
558563
return Offset(PayloadSizeOffset.getStaticOffset());
559564
}
560565

566+
Offset EnumMetadataLayout::getTrailingFlagsOffset() const {
567+
assert(TrailingFlagsOffset.isStatic());
568+
return Offset(TrailingFlagsOffset.getStaticOffset());
569+
}
570+
561571
/********************************** STRUCTS ***********************************/
562572

563573
StructMetadataLayout::StructMetadataLayout(IRGenModule &IGM, StructDecl *decl)
@@ -594,6 +604,11 @@ StructMetadataLayout::StructMetadataLayout(IRGenModule &IGM, StructDecl *decl)
594604
super::noteEndOfFieldOffsets();
595605
}
596606

607+
void addTrailingFlags() {
608+
Layout.TrailingFlagsOffset = getNextOffset();
609+
super::addTrailingFlags();
610+
}
611+
597612
void layout() {
598613
super::layout();
599614
Layout.TheSize = getMetadataSize();
@@ -620,6 +635,11 @@ StructMetadataLayout::getFieldOffsetVectorOffset() const {
620635
return Offset(FieldOffsetVector.getStaticOffset());
621636
}
622637

638+
Offset
639+
StructMetadataLayout::getTrailingFlagsOffset() const {
640+
assert(TrailingFlagsOffset.isStatic());
641+
return Offset(TrailingFlagsOffset.getStaticOffset());
642+
}
623643
/****************************** FOREIGN CLASSES *******************************/
624644
ForeignClassMetadataLayout::ForeignClassMetadataLayout(IRGenModule &IGM,
625645
ClassDecl *theClass)

‎lib/IRGen/MetadataLayout.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ class ClassMetadataLayout : public NominalMetadataLayout {
284284
class EnumMetadataLayout : public NominalMetadataLayout {
285285
/// The offset of the payload size field, if there is one.
286286
StoredOffset PayloadSizeOffset;
287+
StoredOffset TrailingFlagsOffset;
287288

288289
// TODO: presumably it would be useful to store *something* here
289290
// for resilience.
@@ -301,6 +302,7 @@ class EnumMetadataLayout : public NominalMetadataLayout {
301302
}
302303

303304
Offset getPayloadSizeOffset() const;
305+
Offset getTrailingFlagsOffset() const;
304306

305307
static bool classof(const MetadataLayout *layout) {
306308
return layout->getKind() == Kind::Enum;
@@ -310,6 +312,7 @@ class EnumMetadataLayout : public NominalMetadataLayout {
310312
/// Layout for struct type metadata.
311313
class StructMetadataLayout : public NominalMetadataLayout {
312314
llvm::DenseMap<VarDecl*, StoredOffset> FieldOffsets;
315+
StoredOffset TrailingFlagsOffset;
313316

314317
/// The start of the field-offset vector.
315318
StoredOffset FieldOffsetVector;
@@ -338,6 +341,7 @@ class StructMetadataLayout : public NominalMetadataLayout {
338341
Size getStaticFieldOffset(VarDecl *field) const;
339342

340343
Offset getFieldOffsetVectorOffset() const;
344+
Offset getTrailingFlagsOffset() const;
341345

342346
static bool classof(const MetadataLayout *layout) {
343347
return layout->getKind() == Kind::Struct;

‎test/IRGen/enum_future.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,7 @@ entry(%x : $*MyOptional):
26602660
// CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} internal %swift.type* @"$s11enum_future16DynamicSingletonOMi"(%swift.type_descriptor* %0, i8** %1, i8* %2) {{.*}} {
26612661
// CHECK: [[T0:%.*]] = bitcast i8** %1 to %swift.type**
26622662
// CHECK: [[T:%T]] = load %swift.type*, %swift.type** [[T0]],
2663-
// CHECK: [[METADATA:%.*]] = call %swift.type* @swift_allocateGenericValueMetadata(%swift.type_descriptor* %0, i8** %1, i8* %2, [[WORD]] {{4|8|16}})
2663+
// CHECK: [[METADATA:%.*]] = call %swift.type* @swift_allocateGenericValueMetadata(%swift.type_descriptor* %0, i8** %1, i8* %2, [[WORD]] {{4|8|12|16}})
26642664
// CHECK-NEXT: ret %swift.type* [[METADATA]]
26652665

26662666
// CHECK-LABEL: define{{( protected)?}} internal swiftcc %swift.metadata_response @"$s11enum_future16DynamicSingletonOMr"

‎test/IRGen/enum_value_semantics_future.sil

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,20 +160,24 @@ enum GenericFixedLayout<T> {
160160
// CHECK-SAME: {{.*}}* @"$s27enum_value_semantics_future23SinglePayloadNontrivialOMn"
161161
// CHECK-SAME: }>
162162

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

164166
// CHECK-LABEL: @"$s27enum_value_semantics_future18GenericFixedLayoutOMn" = hidden constant
165167
// CHECK-SAME: [16 x i8*]* @"$s27enum_value_semantics_future18GenericFixedLayoutOMI"
166168
// CHECK-SAME: @"$s27enum_value_semantics_future18GenericFixedLayoutOMP"
167169

168170
// CHECK-LABEL: @"$s27enum_value_semantics_future18GenericFixedLayoutOMP" = internal constant <{ {{.*}} }> <{
169171
// Instantiation function.
170-
// CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (%swift.type* (%swift.type_descriptor*, i8**, i8*)* @"$s27enum_value_semantics_future18GenericFixedLayoutOMi" to i64), i64 ptrtoint (<{ i32, i32, i32, i32 }>* @"$s27enum_value_semantics_future18GenericFixedLayoutOMP" to i64)) to i32),
172+
// CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (%swift.type* (%swift.type_descriptor*, i8**, i8*)* @"$s27enum_value_semantics_future18GenericFixedLayoutOMi" to i64), i64 ptrtoint (<{ i32, i32, i32, i32, i32, i16, i16 }>* @"$s27enum_value_semantics_future18GenericFixedLayoutOMP" to i64)) to i32),
171173
// Completion function.
172174
// CHECK-SAME: i32 0,
173-
// Pattern flags. 0x4020_0002 == (MetadataKind::Enum << 21 | HasTrailingFlags).
174-
// CHECK-SAME: i32 1075838978,
175+
// Pattern flags. 0x4020_0003 == (MetadataKind::Enum << 21 | HasTrailingFlags | HasExtraDataPattern).
176+
// CHECK-SAME: i32 1075838979,
175177
// Value witness table.
176-
// 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, i32, i32, i32 }>* @"$s27enum_value_semantics_future18GenericFixedLayoutOMP", i32 0, i32 3) to i64)) to i32)
178+
// 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)
179+
// 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)
177181
// CHECK-SAME: }>
178182

179183
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 [4 x i32] [i32 0, i32 1, i32 8, i32 0]
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: [4 x i32]* [[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: 3 additions & 3 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 [4 x i32] [i32 0, i32 1, i32 8, i32 0]
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,9 +20,9 @@ import Builtin
2020
// -- vwtable pointer
2121
// CHECK-SAME: @"$s22generic_structs_future18FixedLayoutGenericVWV"
2222
// -- extra data pattern
23-
// CHECK-SAME: [4 x i32]* [[PATTERN]]
23+
// CHECK-SAME: { i32, i32, i32, [4 x i8], i64 }* [[PATTERN]]
2424
// CHECK-SAME: i16 1,
25-
// CHECK-SAME: i16 2 }>
25+
// CHECK-SAME: i16 3 }>
2626

2727
// -- Generic structs with dynamic layout contain the vwtable pattern as part
2828
// of the metadata pattern, and no independent vwtable symbol

‎test/IRGen/prespecialized-metadata/Inputs/enum-extra-data-fields.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
public struct First<T> {
2+
public let value: Int32
3+
}
4+
5+
public struct Second {
6+
public let value: Int64
7+
}
8+
9+
@frozen
10+
public enum FixedPayloadSize<T> {
11+
case first(First<T>)
12+
case second(Second)
13+
}
14+
15+
public enum DynamicPayloadSize<T> {
16+
case first(First<T>)
17+
case second(Second)
18+
}

0 commit comments

Comments
 (0)