Skip to content

Commit b8329b6

Browse files
CodaFiCodaFi
authored
Merge pull request #36060 from CodaFi/codicil
Patch a Huge Soundness Hole in Codable Synthesis
2 parents 1102835 + 6c9fb0d commit b8329b6

9 files changed

+203
-264
lines changed

‎lib/Sema/DerivedConformanceCodable.cpp

Lines changed: 162 additions & 217 deletions
Large diffs are not rendered by default.

‎lib/Sema/DerivedConformances.cpp

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ bool DerivedConformance::derivesProtocolConformance(DeclContext *DC,
8888
if (*derivableKind == KnownDerivableProtocolKind::Differentiable)
8989
return true;
9090

91+
if (*derivableKind == KnownDerivableProtocolKind::Encodable) {
92+
return canDeriveEncodable(Nominal);
93+
}
94+
95+
if (*derivableKind == KnownDerivableProtocolKind::Decodable) {
96+
return canDeriveDecodable(Nominal);
97+
}
98+
9199
if (auto *enumDecl = dyn_cast<EnumDecl>(Nominal)) {
92100
switch (*derivableKind) {
93101
// The presence of a raw type is an explicit declaration that
@@ -137,31 +145,13 @@ bool DerivedConformance::derivesProtocolConformance(DeclContext *DC,
137145
default:
138146
return false;
139147
}
140-
} else if (isa<StructDecl>(Nominal) || isa<ClassDecl>(Nominal)) {
141-
// Structs and classes can explicitly derive Encodable and Decodable
142-
// conformance (explicitly meaning we can synthesize an implementation if
143-
// a type conforms manually).
144-
if (*derivableKind == KnownDerivableProtocolKind::Encodable ||
145-
*derivableKind == KnownDerivableProtocolKind::Decodable) {
146-
// FIXME: This is not actually correct. We cannot promise to always
147-
// provide a witness here for all structs and classes. Unfortunately,
148-
// figuring out whether this is actually possible requires much more
149-
// context -- a TypeChecker and the parent decl context at least -- and is
150-
// tightly coupled to the logic within DerivedConformance.
151-
// This unfortunately means that we expect a witness even if one will not
152-
// be produced, which requires DerivedConformance::deriveCodable to output
153-
// its own diagnostics.
154-
return true;
155-
}
156-
157-
// Structs can explicitly derive Equatable conformance.
158-
if (isa<StructDecl>(Nominal)) {
159-
switch (*derivableKind) {
160-
case KnownDerivableProtocolKind::Equatable:
161-
return canDeriveEquatable(DC, Nominal);
162-
default:
163-
return false;
164-
}
148+
} else if (isa<StructDecl>(Nominal)) {
149+
switch (*derivableKind) {
150+
case KnownDerivableProtocolKind::Equatable:
151+
// Structs can explicitly derive Equatable conformance.
152+
return canDeriveEquatable(DC, Nominal);
153+
default:
154+
return false;
165155
}
166156
}
167157
return false;

‎lib/Sema/DerivedConformances.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,11 @@ class DerivedConformance {
278278
/// \returns the derived member, which will also be added to the type.
279279
ValueDecl *deriveBridgedNSError(ValueDecl *requirement);
280280

281+
/// Determine if \c Encodable can be derived for the given type.
282+
static bool canDeriveEncodable(NominalTypeDecl *NTD);
283+
/// Determine if \c Decodable can be derived for the given type.
284+
static bool canDeriveDecodable(NominalTypeDecl *NTD);
285+
281286
/// Derive a CodingKey requirement for an enum type.
282287
///
283288
/// \returns the derived member, which will also be added to the type.

‎lib/Sema/TypeCheckPattern.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -748,8 +748,12 @@ static Type validateTypedPattern(TypedPattern *TP, TypeResolution resolution) {
748748
Type TypeChecker::typeCheckPattern(ContextualPattern pattern) {
749749
DeclContext *dc = pattern.getDeclContext();
750750
ASTContext &ctx = dc->getASTContext();
751-
return evaluateOrDefault(
752-
ctx.evaluator, PatternTypeRequest{pattern}, ErrorType::get(ctx));
751+
if (auto type = evaluateOrDefault(ctx.evaluator, PatternTypeRequest{pattern},
752+
Type())) {
753+
return type;
754+
}
755+
756+
return ErrorType::get(ctx);
753757
}
754758

755759
/// Apply the contextual pattern's context to the type resolution options.

‎lib/Sema/TypeCheckType.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,9 +1213,8 @@ static Type diagnoseUnknownType(TypeResolution resolution,
12131213
return I->second;
12141214
}
12151215

1216-
diags.diagnose(L, diag::cannot_find_type_in_scope,
1217-
comp->getNameRef())
1218-
.highlight(R);
1216+
diags.diagnose(L, diag::cannot_find_type_in_scope, comp->getNameRef())
1217+
.highlight(R);
12191218

12201219
return ErrorType::get(ctx);
12211220
}

‎test/decl/protocol/special/coding/class_codable_nonconforming_property.swift

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,10 @@ class NonConformingClass : Codable { // expected-error {{type 'NonConformingClas
165165
// These lines have to be within the NonConformingClass type because
166166
// CodingKeys should be private.
167167
func foo() {
168-
// They should not get a CodingKeys type.
169-
let _ = NonConformingClass.CodingKeys.self // expected-error {{type 'NonConformingClass' has no member 'CodingKeys'}}
170-
let _ = NonConformingClass.CodingKeys.x // expected-error {{type 'NonConformingClass' has no member 'CodingKeys'}}
171-
let _ = NonConformingClass.CodingKeys.y // expected-error {{type 'NonConformingClass' has no member 'CodingKeys'}}
172-
let _ = NonConformingClass.CodingKeys.z // expected-error {{type 'NonConformingClass' has no member 'CodingKeys'}}
168+
let _ = NonConformingClass.CodingKeys.self
169+
let _ = NonConformingClass.CodingKeys.x
170+
let _ = NonConformingClass.CodingKeys.y
171+
let _ = NonConformingClass.CodingKeys.z // expected-error {{type 'NonConformingClass.CodingKeys' has no member 'z'}}
173172
}
174173
}
175174

@@ -178,4 +177,4 @@ let _ = NonConformingClass.init(from:) // expected-error {{type 'NonConformingCl
178177
let _ = NonConformingClass.encode(to:) // expected-error {{type 'NonConformingClass' has no member 'encode(to:)'}}
179178

180179
// They should not get a CodingKeys type.
181-
let _ = NonConformingClass.CodingKeys.self // expected-error {{type 'NonConformingClass' has no member 'CodingKeys'}}
180+
let _ = NonConformingClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}

‎test/decl/protocol/special/coding/struct_codable_nonconforming_property.swift

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,10 @@ struct NonConformingStruct : Codable { // expected-error {{type 'NonConformingSt
165165
// These lines have to be within the NonConformingStruct type because
166166
// CodingKeys should be private.
167167
func foo() {
168-
// They should not get a CodingKeys type.
169-
let _ = NonConformingStruct.CodingKeys.self // expected-error {{type 'NonConformingStruct' has no member 'CodingKeys'}}
170-
let _ = NonConformingStruct.CodingKeys.x // expected-error {{type 'NonConformingStruct' has no member 'CodingKeys'}}
171-
let _ = NonConformingStruct.CodingKeys.y // expected-error {{type 'NonConformingStruct' has no member 'CodingKeys'}}
172-
let _ = NonConformingStruct.CodingKeys.z // expected-error {{type 'NonConformingStruct' has no member 'CodingKeys'}}
168+
let _ = NonConformingStruct.CodingKeys.self
169+
let _ = NonConformingStruct.CodingKeys.x
170+
let _ = NonConformingStruct.CodingKeys.y
171+
let _ = NonConformingStruct.CodingKeys.z // expected-error {{type 'NonConformingStruct.CodingKeys' has no member 'z'}}
173172
}
174173
}
175174

@@ -178,4 +177,4 @@ let _ = NonConformingStruct.init(from:) // expected-error {{type 'NonConformingS
178177
let _ = NonConformingStruct.encode(to:) // expected-error {{type 'NonConformingStruct' has no member 'encode(to:)'}}
179178

180179
// They should not get a CodingKeys type.
181-
let _ = NonConformingStruct.CodingKeys.self // expected-error {{type 'NonConformingStruct' has no member 'CodingKeys'}}
180+
let _ = NonConformingStruct.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}

‎test/decl/protocol/special/coding/struct_codable_simple.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,19 @@ let _ = SimpleStruct.encode(to:)
3131
let _ = SimpleStruct.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}
3232

3333
// rdar://problem/59655704
34-
struct SR_12248_1: Codable { // expected-error {{type 'SR_12248_1' does not conform to protocol 'Encodable'}}
34+
struct SR_12248_1: Codable { // expected-error {{type 'SR_12248_1' does not conform to protocol 'Encodable'}} expected-error {{type 'SR_12248_1' does not conform to protocol 'Decodable'}}
3535
var x: Int // expected-note {{'x' previously declared here}}
3636
var x: Int // expected-error {{invalid redeclaration of 'x'}}
3737
// expected-note@-1 {{cannot automatically synthesize 'Encodable' because 'Int' does not conform to 'Encodable'}}
3838
// expected-note@-2 {{cannot automatically synthesize 'Encodable' because 'Int' does not conform to 'Encodable'}}
3939
}
4040

41-
struct SR_12248_2: Decodable {
41+
struct SR_12248_2: Decodable { // expected-error {{type 'SR_12248_2' does not conform to protocol 'Decodable'}}
4242
var x: Int // expected-note {{'x' previously declared here}}
4343
var x: Int // expected-error {{invalid redeclaration of 'x'}}
4444
}
4545

46-
struct SR_12248_3: Encodable {
46+
struct SR_12248_3: Encodable { // expected-error {{type 'SR_12248_3' does not conform to protocol 'Encodable'}}
4747
var x: Int // expected-note {{'x' previously declared here}}
4848
var x: Int // expected-error {{invalid redeclaration of 'x'}}
4949
}

‎test/diagnostics/pretty-printed-diagnostics.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,6 @@ foo(b:
136136
// CHECK: [[#LINE]] | struct B: Decodable {
137137
// CHECK: | ^ error: type 'B' does not conform to protocol 'Decodable'
138138
// CHECK: [[#LINE+1]] | let a: Foo
139-
// CHECK: | ^ note: cannot automatically synthesize 'Decodable' because 'Foo' does not conform to 'Decodable'
140-
// CHECK: [[#LINE+2]] | }
141139
// CHECK: Swift.Decodable:2:5
142140
// CHECK: 1 | public protocol Decodable {
143141
// CHECK: 2 | init(from decoder: Decoder) throws

0 commit comments

Comments
 (0)