From 8f2f14bbad855100ea53a7a93ee1da7f46fc0e2a Mon Sep 17 00:00:00 2001 From: Luciano Almeida Date: Fri, 29 May 2020 21:32:27 -0300 Subject: [PATCH 1/6] [CSSimplify] Modify logic on match representations to allow ONLY thick to thin and thick to thick representations on subtype context --- lib/Sema/CSSimplify.cpp | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 6c9a75fa68c27..9634273a77dd6 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1353,16 +1353,41 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2, // corresponding function type representations and the given match kind. static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, FunctionTypeRepresentation rep2, - ConstraintKind kind) { + ConstraintKind kind, + ConstraintLocator *locator) { switch (kind) { case ConstraintKind::Bind: case ConstraintKind::BindParam: case ConstraintKind::BindToPointerType: case ConstraintKind::Equal: return rep1 != rep2; + + case ConstraintKind::Subtype: { + if (!locator->isLastElement()) + return false; + + auto isThin = [](FunctionTypeRepresentation rep) { + return rep == FunctionTypeRepresentation::CFunctionPointer || + rep == FunctionTypeRepresentation::Thin; + }; + + auto isThick = [](FunctionTypeRepresentation rep) { + return rep == FunctionTypeRepresentation::Swift || + rep == swift::FunctionTypeRepresentation::Block; + }; + + // Allowing conventions "thin" (c, thin) to "thick" (swift, block) + if (isThin(rep1) && isThick(rep2)) + return false; + + // Allowing conventions "thick" (swift, block) to "thick" (swift, block) + if (isThick(rep1) && isThick(rep2)) + return false; + + return rep1 != rep2; + } case ConstraintKind::OpaqueUnderlyingType: - case ConstraintKind::Subtype: case ConstraintKind::Conversion: case ConstraintKind::BridgingConversion: case ConstraintKind::ArgumentConversion: @@ -1657,7 +1682,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2, if (matchFunctionRepresentations(func1->getExtInfo().getRepresentation(), func2->getExtInfo().getRepresentation(), - kind)) { + kind, getConstraintLocator(locator))) { return getTypeMatchFailure(locator); } From bacbc574a7f5ae05a91a1babcb41b5a4e8242ce7 Mon Sep 17 00:00:00 2001 From: Luciano Almeida Date: Fri, 29 May 2020 21:33:22 -0300 Subject: [PATCH 2/6] [tests] Adding silgen validation tests for SR-12723 to ensure that certain representation convertions do not crash --- .../compiler_crashers_2_fixed/sr12723.swift | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 validation-test/compiler_crashers_2_fixed/sr12723.swift diff --git a/validation-test/compiler_crashers_2_fixed/sr12723.swift b/validation-test/compiler_crashers_2_fixed/sr12723.swift new file mode 100644 index 0000000000000..2a4e227c9dd42 --- /dev/null +++ b/validation-test/compiler_crashers_2_fixed/sr12723.swift @@ -0,0 +1,83 @@ +// RUN: %target-swift-emit-silgen %s -verify + +func SR12723_thin(_: (@convention(thin) () -> Void) -> Void) {} +func SR12723_block(_: (@convention(block) () -> Void) -> Void) {} +func SR12723_c(_: (@convention(c) () -> Void) -> Void) {} + +func SR12723_function(_: () -> Void) {} + +func context() { + SR12723_c(SR12723_function) + + SR12723_block(SR12723_function) + + SR12723_thin(SR12723_function) +} + +struct SR12723_C { + let function: (@convention(c) () -> Void) -> Void +} + +struct SR12723_Thin { + let function: (@convention(thin) () -> Void) -> Void +} + +struct SR12723_Block { + let function: (@convention(block) () -> Void) -> Void +} + +func proxy(_ f: (() -> Void) -> Void) { + let a = 1 + f { print(a) } +} + +func cContext() { + let c = SR12723_C { app in app() } + + proxy(c.function) + // expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to expected argument type '(() -> Void) -> Void'}} + + let _ : (@convention(block) () -> Void) -> Void = c.function + // expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(@convention(block) () -> Void) -> Void'}} + + let _ : (@convention(c) () -> Void) -> Void = c.function // OK + + let _ : (@convention(thin) () -> Void) -> Void = c.function + // expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(@convention(thin) () -> Void) -> Void'}} + + let _ : (() -> Void) -> Void = c.function + // expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(() -> Void) -> Void'}} + +} + +func thinContext() { + let thin = SR12723_Thin { app in app() } + + proxy(thin.function) + // expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to expected argument type '(() -> Void) -> Void'}} + + let _ : (@convention(block) () -> Void) -> Void = thin.function + // expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(@convention(block) () -> Void) -> Void'}} + + let _ : (@convention(c) () -> Void) -> Void = thin.function + // expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(@convention(c) () -> Void) -> Void'}} + + let _ : (@convention(thin) () -> Void) -> Void = thin.function // OK + + let _ : (() -> Void) -> Void = thin.function + // expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(() -> Void) -> Void'}} +} + +func blockContext() { + let block = SR12723_Block { app in app() } + + proxy(block.function) + + let _ : (@convention(block) () -> Void) -> Void = block.function // OK + + let _ : (@convention(c) () -> Void) -> Void = block.function // OK + + let _ : (@convention(thin) () -> Void) -> Void = block.function // OK + + let _ : (() -> Void) -> Void = block.function // OK +} From baccbde0d1ae1c049794c4180e013a4cf0b6909b Mon Sep 17 00:00:00 2001 From: Luciano Almeida Date: Tue, 2 Jun 2020 06:25:05 -0300 Subject: [PATCH 3/6] [CSSimplify] Simplify logic for valiting to thick conversion on subtype --- lib/Sema/CSSimplify.cpp | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 9634273a77dd6..e23580153ac8b 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1354,7 +1354,7 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2, static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, FunctionTypeRepresentation rep2, ConstraintKind kind, - ConstraintLocator *locator) { + ConstraintLocatorBuilder locator) { switch (kind) { case ConstraintKind::Bind: case ConstraintKind::BindParam: @@ -1363,25 +1363,14 @@ static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, return rep1 != rep2; case ConstraintKind::Subtype: { - if (!locator->isLastElement()) - return false; - - auto isThin = [](FunctionTypeRepresentation rep) { - return rep == FunctionTypeRepresentation::CFunctionPointer || - rep == FunctionTypeRepresentation::Thin; - }; - - auto isThick = [](FunctionTypeRepresentation rep) { - return rep == FunctionTypeRepresentation::Swift || - rep == swift::FunctionTypeRepresentation::Block; - }; - - // Allowing conventions "thin" (c, thin) to "thick" (swift, block) - if (isThin(rep1) && isThick(rep2)) + auto last = locator.last(); + if (!(last && last->is())) return false; - // Allowing conventions "thick" (swift, block) to "thick" (swift, block) - if (isThick(rep1) && isThick(rep2)) + // Allowing all to "thick" (swift, block) conventions + // "thin" (c, thin) to "thick" or "thick" to "thick" + if (rep2 == FunctionTypeRepresentation::Swift || + rep2 == FunctionTypeRepresentation::Block) return false; return rep1 != rep2; @@ -1682,7 +1671,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2, if (matchFunctionRepresentations(func1->getExtInfo().getRepresentation(), func2->getExtInfo().getRepresentation(), - kind, getConstraintLocator(locator))) { + kind, locator)) { return getTypeMatchFailure(locator); } From d6bf34e65ccaca00821c9c3d2ac06ab6bcfe540c Mon Sep 17 00:00:00 2001 From: Luciano Almeida Date: Fri, 5 Jun 2020 06:11:01 -0300 Subject: [PATCH 4/6] [CSSimplify] Thin to thin is also allowed --- lib/Sema/CSSimplify.cpp | 9 +++++++++ validation-test/compiler_crashers_2_fixed/sr12723.swift | 8 +++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index e23580153ac8b..05ac6ad828af5 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1367,6 +1367,15 @@ static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, if (!(last && last->is())) return false; + auto isThin = [](FunctionTypeRepresentation rep) { + return rep == FunctionTypeRepresentation::CFunctionPointer || + rep == FunctionTypeRepresentation::Thin; + }; + + // Allowing "thin" (c, thin) to "thin" conventions + if (isThin(rep1) && isThin(rep2)) + return false; + // Allowing all to "thick" (swift, block) conventions // "thin" (c, thin) to "thick" or "thick" to "thick" if (rep2 == FunctionTypeRepresentation::Swift || diff --git a/validation-test/compiler_crashers_2_fixed/sr12723.swift b/validation-test/compiler_crashers_2_fixed/sr12723.swift index 2a4e227c9dd42..b04103a9f7a8e 100644 --- a/validation-test/compiler_crashers_2_fixed/sr12723.swift +++ b/validation-test/compiler_crashers_2_fixed/sr12723.swift @@ -42,9 +42,8 @@ func cContext() { let _ : (@convention(c) () -> Void) -> Void = c.function // OK - let _ : (@convention(thin) () -> Void) -> Void = c.function - // expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(@convention(thin) () -> Void) -> Void'}} - + let _ : (@convention(thin) () -> Void) -> Void = c.function // OK + let _ : (() -> Void) -> Void = c.function // expected-error@-1 {{cannot convert value of type '(@convention(c) () -> Void) -> Void' to specified type '(() -> Void) -> Void'}} @@ -59,8 +58,7 @@ func thinContext() { let _ : (@convention(block) () -> Void) -> Void = thin.function // expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(@convention(block) () -> Void) -> Void'}} - let _ : (@convention(c) () -> Void) -> Void = thin.function - // expected-error@-1 {{cannot convert value of type '(@convention(thin) () -> Void) -> Void' to specified type '(@convention(c) () -> Void) -> Void'}} + let _ : (@convention(c) () -> Void) -> Void = thin.function // OK let _ : (@convention(thin) () -> Void) -> Void = thin.function // OK From adba283a00eb7332c4ff8b27bf51f8bad9a08555 Mon Sep 17 00:00:00 2001 From: Luciano Almeida Date: Tue, 9 Jun 2020 06:19:54 -0300 Subject: [PATCH 5/6] [CSSimplify] Abstract function conversion validation into static method --- lib/Sema/CSSimplify.cpp | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 05ac6ad828af5..252fb22c95f0c 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1349,6 +1349,29 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2, return getTypeMatchSuccess(); } +// Determine if a function representation conversion is allowed returning +// 'false' (i.e. no error) if the conversion is valid. +static bool +matchFunctionConversionRepresentations(FunctionTypeRepresentation rep1, + FunctionTypeRepresentation rep2) { + auto isThin = [](FunctionTypeRepresentation rep) { + return rep == FunctionTypeRepresentation::CFunctionPointer || + rep == FunctionTypeRepresentation::Thin; + }; + + // Allowing "thin" (c, thin) to "thin" conventions + if (isThin(rep1) && isThin(rep2)) + return false; + + // Allowing all to "thick" (swift, block) conventions + // "thin" (c, thin) to "thick" or "thick" to "thick" + if (rep2 == FunctionTypeRepresentation::Swift || + rep2 == FunctionTypeRepresentation::Block) + return false; + + return rep1 != rep2; +} + // Returns 'false' (i.e. no error) if it is legal to match functions with the // corresponding function type representations and the given match kind. static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, @@ -1367,22 +1390,7 @@ static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, if (!(last && last->is())) return false; - auto isThin = [](FunctionTypeRepresentation rep) { - return rep == FunctionTypeRepresentation::CFunctionPointer || - rep == FunctionTypeRepresentation::Thin; - }; - - // Allowing "thin" (c, thin) to "thin" conventions - if (isThin(rep1) && isThin(rep2)) - return false; - - // Allowing all to "thick" (swift, block) conventions - // "thin" (c, thin) to "thick" or "thick" to "thick" - if (rep2 == FunctionTypeRepresentation::Swift || - rep2 == FunctionTypeRepresentation::Block) - return false; - - return rep1 != rep2; + return matchFunctionConversionRepresentations(rep1, rep2); } case ConstraintKind::OpaqueUnderlyingType: From da53129beba61ed4a805d7283048d566872f808a Mon Sep 17 00:00:00 2001 From: Luciano Almeida Date: Mon, 15 Jun 2020 06:40:57 -0300 Subject: [PATCH 6/6] [CSSimplify] Adjusting naming and comments of method that validates representation conversions. --- lib/Sema/CSSimplify.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 252fb22c95f0c..3fa6c530a3890 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1349,11 +1349,11 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2, return getTypeMatchSuccess(); } -// Determine if a function representation conversion is allowed returning -// 'false' (i.e. no error) if the conversion is valid. +// Determine whether conversion is allowed between two function types +// based on their representations. static bool -matchFunctionConversionRepresentations(FunctionTypeRepresentation rep1, - FunctionTypeRepresentation rep2) { +isConversionAllowedBetween(FunctionTypeRepresentation rep1, + FunctionTypeRepresentation rep2) { auto isThin = [](FunctionTypeRepresentation rep) { return rep == FunctionTypeRepresentation::CFunctionPointer || rep == FunctionTypeRepresentation::Thin; @@ -1361,15 +1361,15 @@ matchFunctionConversionRepresentations(FunctionTypeRepresentation rep1, // Allowing "thin" (c, thin) to "thin" conventions if (isThin(rep1) && isThin(rep2)) - return false; + return true; // Allowing all to "thick" (swift, block) conventions // "thin" (c, thin) to "thick" or "thick" to "thick" if (rep2 == FunctionTypeRepresentation::Swift || rep2 == FunctionTypeRepresentation::Block) - return false; + return true; - return rep1 != rep2; + return rep1 == rep2; } // Returns 'false' (i.e. no error) if it is legal to match functions with the @@ -1390,7 +1390,9 @@ static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1, if (!(last && last->is())) return false; - return matchFunctionConversionRepresentations(rep1, rep2); + // Inverting the result because matchFunctionRepresentations + // returns false in conversions are allowed. + return !isConversionAllowedBetween(rep1, rep2); } case ConstraintKind::OpaqueUnderlyingType: