Skip to content

Commit b97e272

Browse files
tshortlitshortli
committed
Sema: Fix opaque type accessors with inactive availability conditions.
Opaque type metadata accessor functions could be miscompiled for functions that contain `if #available` checks for inactive platforms. For example, this function will always return `A` when compiled for macOS, but the opaque type accessor would instead return the type metadata for `B`: ``` func f() -> some P { if #available(iOS 99, *) { return A() // Returns an A on macOS } else { return B() } } ``` Resolves rdar://139487970.
1 parent 0d581c4 commit b97e272

File tree

4 files changed

+132
-20
lines changed

4 files changed

+132
-20
lines changed

‎lib/Sema/MiscDiagnostics.cpp‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3468,7 +3468,6 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
34683468
}
34693469

34703470
// Check whether all of the underlying type candidates match up.
3471-
// TODO [OPAQUE SUPPORT]: diagnose multiple opaque types
34723471

34733472
// There is a single unique signature, which means that all returns
34743473
// matched.
@@ -3595,6 +3594,7 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
35953594

35963595
SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *, 4>
35973596
conditionalSubstitutions;
3597+
SubstitutionMap universalSubstMap = std::get<1>(universallyAvailable);
35983598

35993599
for (const auto &entry : Candidates) {
36003600
auto availabilityContext = entry.first;
@@ -3603,25 +3603,46 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
36033603
if (!availabilityContext)
36043604
continue;
36053605

3606+
unsigned neverAvailableCount = 0, alwaysAvailableCount = 0;
36063607
SmallVector<AvailabilityCondition, 4> conditions;
36073608

36083609
for (const auto &elt : availabilityContext->getCond()) {
36093610
auto condition = elt.getAvailability();
36103611

36113612
auto availabilityRange = condition->getAvailableRange();
3612-
// If there is no lower endpoint it means that the
3613-
// current platform is unrelated to this condition
3614-
// and we can ignore it.
3615-
if (!availabilityRange.hasLowerEndpoint())
3613+
// If there is no lower endpoint it means that the condition has no
3614+
// OS version specification that matches the target platform.
3615+
if (!availabilityRange.hasLowerEndpoint()) {
3616+
// An inactive #unavailable condition trivially evaluates to false.
3617+
if (condition->isUnavailability()) {
3618+
neverAvailableCount++;
3619+
continue;
3620+
}
3621+
3622+
// An inactive #available condition trivially evaluates to true.
3623+
alwaysAvailableCount++;
36163624
continue;
3625+
}
36173626

36183627
conditions.push_back(
36193628
{availabilityRange, condition->isUnavailability()});
36203629
}
36213630

3622-
if (conditions.empty())
3631+
// If there were any conditions that were always false, then this
3632+
// candidate is unreachable at runtime.
3633+
if (neverAvailableCount > 0)
36233634
continue;
36243635

3636+
// If all the conditions were trivially true, then this candidate is
3637+
// effectively a universally available candidate and the rest of the
3638+
// candidates should be ignored since they are unreachable.
3639+
if (alwaysAvailableCount == availabilityContext->getCond().size()) {
3640+
universalSubstMap = std::get<1>(candidate);
3641+
break;
3642+
}
3643+
3644+
ASSERT(conditions.size() > 0);
3645+
36253646
conditionalSubstitutions.push_back(
36263647
OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
36273648
Ctx, conditions,
@@ -3632,8 +3653,7 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
36323653
conditionalSubstitutions.push_back(
36333654
OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
36343655
Ctx, {{VersionRange::all(), /*unavailable=*/false}},
3635-
std::get<1>(universallyAvailable)
3636-
.mapReplacementTypesOutOfContext()));
3656+
universalSubstMap.mapReplacementTypesOutOfContext()));
36373657

36383658
OpaqueDecl->setConditionallyAvailableSubstitutions(
36393659
conditionalSubstitutions);

‎test/IRGen/opaque_result_with_conditional_availability.swift‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ struct C : P {
2424
func hello() { print("Hello from C") }
2525
}
2626

27+
@available(iOS 100, *)
28+
struct D : P {
29+
func hello() { print("Hello from D") }
30+
}
31+
2732
func test_multiple_single() -> some P {
2833
if #available(macOS 100.0.1, *) {
2934
return A()
@@ -61,6 +66,44 @@ func test_multiple_single() -> some P {
6166
// CHECK-NEXT: ret ptr @"$s43opaque_result_with_conditional_availability1CVAA1PAAWP"
6267
// CHECK-NEXT: }
6368

69+
func test_multiple_single_multiplatform() -> some P {
70+
if #available(macOS 100.0.1, *), #available(iOS 100, *) {
71+
return A()
72+
}
73+
74+
return C()
75+
}
76+
77+
// CHECK: define private ptr @"get_underlying_type_ref 43opaque_result_with_conditional_availabilityAA34test_multiple_single_multiplatformQryFQOQr"(ptr %0)
78+
// CHECK-NEXT: entry:
79+
// CHECK-NEXT: br label %conditional-0
80+
// CHECK: conditional-0: ; preds = %entry
81+
// CHECK-NEXT: br label %cond-0-0
82+
// CHECK: cond-0-0: ; preds = %conditional-0
83+
// CHECK-NEXT: [[COND_1:%.*]] = call i32 @__isPlatformVersionAtLeast(i32 1, i32 100, i32 0, i32 1)
84+
// CHECK-NEXT: [[IS_AT_LEAST:%.*]] = icmp ne i32 [[COND_1]], 0
85+
// CHECK-NEXT: br i1 [[IS_AT_LEAST]], label %result-0, label %universal
86+
// CHECK: result-0: ; preds = %cond-0-0
87+
// CHECK-NEXT: ret ptr {{.*}} @"$s43opaque_result_with_conditional_availability1AVMf"
88+
// CHECK: universal: ; preds = %cond-0-0
89+
// CHECK-NEXT: ret ptr {{.*}} @"$s43opaque_result_with_conditional_availability1CVMf"
90+
// CHECK-NEXT: }
91+
92+
// CHECK: define private ptr @"get_underlying_witness 43opaque_result_with_conditional_availabilityAA34test_multiple_single_multiplatformQryFQOxAA1PHC"(ptr %0)
93+
// CHECK-NEXT: entry:
94+
// CHECK-NEXT: br label %conditional-0
95+
// CHECK: conditional-0: ; preds = %entry
96+
// CHECK-NEXT: br label %cond-0-0
97+
// CHECK: cond-0-0: ; preds = %conditional-0
98+
// CHECK-NEXT: [[COND_1:%.*]] = call i32 @__isPlatformVersionAtLeast(i32 1, i32 100, i32 0, i32 1)
99+
// CHECK-NEXT: [[IS_AT_LEAST:%.*]] = icmp ne i32 [[COND_1]], 0
100+
// CHECK-NEXT: br i1 [[IS_AT_LEAST]], label %result-0, label %universal
101+
// CHECK: result-0: ; preds = %cond-0-0
102+
// CHECK-NEXT: ret ptr @"$s43opaque_result_with_conditional_availability1AVAA1PAAWP"
103+
// CHECK: universal: ; preds = %cond-0-0
104+
// CHECK-NEXT: ret ptr @"$s43opaque_result_with_conditional_availability1CVAA1PAAWP"
105+
// CHECK-NEXT: }
106+
64107
func test_multiple_conds() -> some P {
65108
if #available(macOS 100.0.1, *) {
66109
return A()
@@ -208,3 +251,51 @@ func test_cross_module() -> some SomeProtocol {
208251
// CHECK-NEXT: [[R0:%.*]] = call ptr @"$s49opaque_result_with_conditional_availability_types12PublicStructVAcA12SomeProtocolAAWl"()
209252
// CHECK-NEXT: ret ptr [[R0]]
210253
// CHECK: }
254+
255+
func test_other_platform_available() -> some P {
256+
if #available(iOS 100, *) {
257+
return D() // Always executed on macOS
258+
}
259+
260+
return C() // Never executed on macOS
261+
}
262+
263+
// CHECK: define private ptr @"get_underlying_type_ref 43opaque_result_with_conditional_availabilityAA29test_other_platform_availableQryFQOQr"(ptr %0)
264+
// CHECK-NEXT: entry:
265+
// CHECK-NEXT: br label %universal
266+
267+
// CHECK: universal:
268+
// CHECK-NEXT: ret ptr getelementptr inbounds (<{ ptr, ptr, i64, ptr }>, ptr @"$s43opaque_result_with_conditional_availability1DVMf", i32 0, i32 2)
269+
// CHECK-NEXT: }
270+
271+
// CHECK: define private ptr @"get_underlying_witness 43opaque_result_with_conditional_availabilityAA29test_other_platform_availableQryFQOxAA1PHC"(ptr %0)
272+
// CHECK-NEXT: entry:
273+
// CHECK-NEXT: br label %universal
274+
275+
// CHECK: universal:
276+
// CHECK-NEXT: ret ptr @"$s43opaque_result_with_conditional_availability1DVAA1PAAWP"
277+
// CHECK-NEXT: }
278+
279+
func test_other_platform_unavailable() -> some P {
280+
if #unavailable(iOS 100) {
281+
return C() // Never executed on macOS
282+
}
283+
284+
return D() // Always executed on macOS
285+
}
286+
287+
// CHECK: define private ptr @"get_underlying_type_ref 43opaque_result_with_conditional_availabilityAA31test_other_platform_unavailableQryFQOQr"(ptr %0)
288+
// CHECK-NEXT: entry:
289+
// CHECK-NEXT: br label %universal
290+
291+
// CHECK: universal:
292+
// CHECK-NEXT: ret ptr getelementptr inbounds (<{ ptr, ptr, i64, ptr }>, ptr @"$s43opaque_result_with_conditional_availability1DVMf", i32 0, i32 2)
293+
// CHECK-NEXT: }
294+
295+
// CHECK: define private ptr @"get_underlying_witness 43opaque_result_with_conditional_availabilityAA31test_other_platform_unavailableQryFQOxAA1PHC"(ptr %0)
296+
// CHECK-NEXT: entry:
297+
// CHECK-NEXT: br label %universal
298+
299+
// CHECK: universal:
300+
// CHECK-NEXT: ret ptr @"$s43opaque_result_with_conditional_availability1DVAA1PAAWP"
301+
// CHECK-NEXT: }

‎test/Serialization/Inputs/opaque_with_limited_availability.swift‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,37 +44,34 @@ public struct Example {
4444
}
4545
}
4646

47-
public func test() -> some P {
47+
public func testAvailableQueryWithUniversalResult() -> some P {
4848
if #available(macOS 100.0.1, *) {
4949
return Tuple<(Int, Int)>((0, 0))
5050
}
5151

5252
return Empty()
5353
}
5454

55-
public func testUnavailable() -> some P {
55+
public func testUnavailableQueryWithLimitedResult() -> some P {
5656
if #unavailable(macOS 100.0.1) {
5757
return Tuple<(Int, Int)>((0, 1))
5858
}
5959

6060
return Empty()
6161
}
6262

63-
public func test_return_from_conditional() -> some P {
63+
public func testAvailableQueryWithLimitedResult() -> some P {
6464
if #available(macOS 10.15, *) {
6565
return Named()
6666
}
6767

6868
return Tuple<(String, Int)>(("", 0))
6969
}
7070

71-
// This used to crash during serialization because
72-
// @available negates availability condition.
73-
@available(macOS, unavailable)
74-
public func testUnusable() -> some P {
71+
public func testInactiveAvailableQuery() -> some P {
7572
if #available(iOS 50, *) {
76-
return Named()
73+
return Empty()
7774
}
7875

79-
return Tuple<(String, Int)>(("", 0))
76+
return Named()
8077
}

‎test/Serialization/opaque_with_availability_cross_module.swift‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,22 @@ public struct Test {
3939
}
4040
}
4141

42-
let result = LimitedAvailOpaque.test()
42+
let result = LimitedAvailOpaque.testAvailableQueryWithUniversalResult()
4343
result.hello()
4444
// CHECK: Hello from Empty
4545

4646
Test().sayHello()
4747
// CHECK: Hello from Empty
4848
// CHECK: Hello from Tuple
4949

50-
let conditionalR = LimitedAvailOpaque.test_return_from_conditional()
50+
let conditionalR = LimitedAvailOpaque.testAvailableQueryWithLimitedResult()
5151
conditionalR.hello()
5252
// CHECK: Hello from Named
5353

54-
let unavailableTest = LimitedAvailOpaque.testUnavailable()
54+
let unavailableTest = LimitedAvailOpaque.testUnavailableQueryWithLimitedResult()
5555
unavailableTest.hello()
5656
// CHECK: Hello from Tuple
57+
58+
let inactiveTest = LimitedAvailOpaque.testInactiveAvailableQuery()
59+
inactiveTest.hello()
60+
// CHECK: Hello from Empty

0 commit comments

Comments
 (0)