From 322ce4e3f48627a0055b812fa7c8f007a0a2e682 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 29 Nov 2023 22:58:29 +0000 Subject: [PATCH] [Profiler] Fix handling of property wrapper backing inits Previously we were walking into the PropertyWrapperValuePlaceholderExpr when generating coverage for a property wrapper backing initializer. This meant that we were duplicating the coverage of the initializer expression, and it could cause crashes if a refined counter was introduced within the top-most expression region, such as with a throwing expression in a `try!`. rdar://118939162 --- lib/SIL/IR/SILProfiler.cpp | 18 ++++ test/Profiler/coverage_errors.swift | 92 +++++++++++++++++-- .../coverage_property_wrapper_backing.swift | 43 +++++++++ 3 files changed, 143 insertions(+), 10 deletions(-) diff --git a/lib/SIL/IR/SILProfiler.cpp b/lib/SIL/IR/SILProfiler.cpp index 3cd1fbba76676..20960b2ffb5a6 100644 --- a/lib/SIL/IR/SILProfiler.cpp +++ b/lib/SIL/IR/SILProfiler.cpp @@ -295,6 +295,12 @@ struct MapRegionCounters : public ASTWalker { return LazyInitializerWalking::InAccessor; } + bool shouldWalkIntoPropertyWrapperPlaceholderValue() override { + // Don't walk into PropertyWrapperValuePlaceholderExprs, these should be + // mapped as part of the wrapped value initialization. + return false; + } + void mapRegion(ASTNode N) { mapRegion(ProfileCounterRef::node(N)); } @@ -683,6 +689,12 @@ struct PGOMapping : public ASTWalker { return LazyInitializerWalking::InAccessor; } + bool shouldWalkIntoPropertyWrapperPlaceholderValue() override { + // Don't walk into PropertyWrapperValuePlaceholderExprs, these should be + // mapped as part of the wrapped value initialization. + return false; + } + MacroWalking getMacroWalkingBehavior() const override { return MacroWalking::Expansion; } @@ -1102,6 +1114,12 @@ struct CoverageMapping : public ASTWalker { return LazyInitializerWalking::InAccessor; } + bool shouldWalkIntoPropertyWrapperPlaceholderValue() override { + // Don't walk into PropertyWrapperValuePlaceholderExprs, these should be + // mapped as part of the wrapped value initialization. + return false; + } + MacroWalking getMacroWalkingBehavior() const override { return MacroWalking::Expansion; } diff --git a/test/Profiler/coverage_errors.swift b/test/Profiler/coverage_errors.swift index d60089583cc8d..a7d829cfb17fd 100644 --- a/test/Profiler/coverage_errors.swift +++ b/test/Profiler/coverage_errors.swift @@ -248,7 +248,7 @@ func test6( } // CHECK-NEXT: } // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors5test7s5Int32VyF" -func test7() -> Int32 { // CHECK-NEXT: [[@LINE]]:23 -> [[@LINE+31]]:2 : 0 +func test7() -> Int32 { // CHECK-NEXT: [[@LINE]]:23 -> [[@LINE+29]]:2 : 0 var x : Int32 = 0 do { // CHECK-NEXT: [[@LINE]]:6 -> [[@LINE+3]]:4 : 0 @@ -271,8 +271,6 @@ func test7() -> Int32 { // CHECK-NEXT: [[@LINE]]:23 -> [[@LINE+31]]:2 : 0 } catch _ {} // CHECK-NEXT: [[@LINE]]:13 -> [[@LINE]]:15 : 6 // CHECK-NEXT: [[@LINE-1]]:15 -> {{[0-9:]+}} : (((((1 + 2) + 4) + 6) - 3) - 5) - // TODO: We ought to realize that everything after try! is unreachable - // This is similar issue to rdar://100896177 try! test6 { () throws -> () in return @@ -817,8 +815,8 @@ func test65() throws { // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE+5]]:2 : 0 // CHECK-NEXT: [[@LINE-1]]:29 -> [[@LINE+1]]:2 : (0 - 2) } // CHECK-NEXT: } -struct TestInit { - // CHECK-LABEL: sil_coverage_map {{.*}}// coverage_errors.TestInit.init() -> coverage_errors.TestInit +struct TestType1 { + // CHECK-LABEL: sil_coverage_map {{.*}}// coverage_errors.TestType1.init() -> coverage_errors.TestType1 init() { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE+5]]:4 : 0 do { // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+2]]:6 : 0 throw SomeErr.Err1 @@ -827,20 +825,33 @@ struct TestInit { } // CHECK-NEXT: } } -struct TestProp { +@propertyWrapper +struct Wrapper { + var wrappedValue: T + + init(wrappedValue: T) { + self.wrappedValue = wrappedValue + } + + init(wrappedValue: T, x: T, y: T? = nil) { + self.wrappedValue = wrappedValue + } +} + +struct TestType2 { let a = try? throwingFn() - // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1aSiSgvpfi" + // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1aSiSgvpfi" // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:28 : 0 // CHECK-NEXT: } let b = try? (throwingFn(), 0) - // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1bSi_SitSgvpfi" + // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1bSi_SitSgvpfi" // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:33 : 0 // CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:33 : (0 - 1) // CHECK-NEXT: } let c = try? (throwingFn(), .random() ? 0 : 1) - // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1cSi_SitSgvpfi" + // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1cSi_SitSgvpfi" // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:49 : 0 // CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:49 : (0 - 1) // CHECK-NEXT: [[@LINE-4]]:43 -> [[@LINE-4]]:44 : 2 @@ -848,10 +859,71 @@ struct TestProp { // CHECK-NEXT: } let d = (try? (throwingFn(), .random() ? 0 : 1), 0) - // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1dSi_SitSg_Sitvpfi" + // CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1dSi_SitSg_Sitvpfi" // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:54 : 0 // CHECK-NEXT: [[@LINE-3]]:30 -> [[@LINE-3]]:50 : (0 - 1) // CHECK-NEXT: [[@LINE-4]]:44 -> [[@LINE-4]]:45 : 2 // CHECK-NEXT: [[@LINE-5]]:48 -> [[@LINE-5]]:49 : ((0 - 1) - 2) // CHECK-NEXT: } } + +// rdar://118939162 - Make sure we don't crash when generating coverage for 'try!' in a property wrapper init. +struct TestType3 { + // CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType3.x + // CHECK-NEXT: [[@LINE+2]]:4 -> [[@LINE+2]]:13 : 0 + // CHECK-NEXT: } + @Wrapper() + var x = try! throwingFn() + // CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType3.(_x + // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:28 : 0 + // CHECK-NEXT: } +} + +struct TestType4 { + // CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType4.x + // CHECK-NEXT: [[@LINE+2]]:4 -> [[@LINE+2]]:13 : 0 + // CHECK-NEXT: } + @Wrapper() + var x = try! (throwingFn(), 0) + // CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType4.(_x + // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:33 : 0 + // CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:33 : (0 - 1) + // CHECK-NEXT: } +} + +struct TestType5 { + // CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType5.x + // CHECK-NEXT: [[@LINE+3]]:4 -> [[@LINE+3]]:33 : 0 + // CHECK-NEXT: [[@LINE+2]]:32 -> [[@LINE+2]]:33 : (0 - 1) + // CHECK-NEXT: } + @Wrapper(x: try! throwingFn()) + var x = 0 + // CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType5.(_x + // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:12 : 0 + // CHECK-NEXT: } +} + +struct TestType6 { + // CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType6.x + // CHECK-NEXT: [[@LINE+3]]:4 -> [[@LINE+3]]:39 : 0 + // CHECK-NEXT: [[@LINE+2]]:32 -> [[@LINE+2]]:39 : (0 - 1) + // CHECK-NEXT: } + @Wrapper(x: try! throwingFn(), y: 0) + var x = 0 + // CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType6.(_x + // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:12 : 0 + // CHECK-NEXT: } +} + +struct TestType7 { + // CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType7.x + // CHECK-NEXT: [[@LINE+3]]:4 -> [[@LINE+3]]:49 : 0 + // CHECK-NEXT: [[@LINE+2]]:33 -> [[@LINE+2]]:49 : (0 - 1) + // CHECK-NEXT: } + @Wrapper(x: try! (throwingFn(), 0), y: (0, 0)) + var x = try! (throwingFn(), 0) + // CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType7.(_x + // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:33 : 0 + // CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:33 : (0 - 1) + // CHECK-NEXT: } +} diff --git a/test/Profiler/coverage_property_wrapper_backing.swift b/test/Profiler/coverage_property_wrapper_backing.swift index 70fdf7506331b..8fd9fce7ff1e9 100644 --- a/test/Profiler/coverage_property_wrapper_backing.swift +++ b/test/Profiler/coverage_property_wrapper_backing.swift @@ -25,6 +25,29 @@ struct PassThroughWrapper { // CHECK: [[BB]]: // CHECK-NEXT: increment_profiler_counter 1 +// CHECK-LABEL: sil hidden @$s33coverage_property_wrapper_backing1UV1xSivpfP : $@convention(thin) (Int) -> Wrapper +// CHECK: function_ref @$sSb6randomSbyFZ : $@convention(method) (@thin Bool.Type) -> Bool +// CHECK: cond_br {{%[0-9]+}}, [[BB_TRUE:bb[0-9]+]], [[BB_FALSE:bb[0-9]+]] +// +// CHECK: [[BB_FALSE]]: +// CHECK: integer_literal {{.*}}, 2 +// +// CHECK: [[BB_TRUE]]: +// CHECK: increment_profiler_counter 1 +// CHECK: integer_literal {{.*}}, 1 + +// CHECK-LABEL: sil hidden [transparent] @$s33coverage_property_wrapper_backing1UV2_{{.*}}7WrapperVySiGvpfi : $@convention(thin) () -> Int +// CHECK: increment_profiler_counter 0 +// CHECK: function_ref @$sSb6randomSbyFZ : $@convention(method) (@thin Bool.Type) -> Bool +// CHECK: cond_br {{%[0-9]+}}, [[BB_TRUE:bb[0-9]+]], [[BB_FALSE:bb[0-9]+]] + +// CHECK: [[BB_FALSE]]: +// CHECK: integer_literal {{.*}}, 4 +// +// CHECK: [[BB_TRUE]]: +// CHECK: increment_profiler_counter 1 +// CHECK: integer_literal {{.*}}, 3 + struct S { // CHECK-LABEL: sil_coverage_map {{.*}} "$s33coverage_property_wrapper_backing1SV1iSivpfP" {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.S.i // CHECK-NEXT: [[@LINE+4]]:4 -> [[@LINE+4]]:30 : 0 @@ -55,3 +78,23 @@ struct T { @Wrapper(.random() ? 1 : 2) var k = 3 } + +// rdar://118939162 - Make sure we don't include the initialization expression +// in the backing initializer. +struct U { + // CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.U.x + // CHECK-NEXT: [[@LINE+4]]:4 -> [[@LINE+4]]:30 : 0 + // CHECK-NEXT: [[@LINE+3]]:24 -> [[@LINE+3]]:25 : 1 + // CHECK-NEXT: [[@LINE+2]]:28 -> [[@LINE+2]]:29 : (0 - 1) + // CHECK-NEXT: } + @Wrapper(.random() ? 1 : 2) + var x = if .random() { 3 } else { 4 } + // CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_property_wrapper_backing.U.(_x + // CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:40 : 0 + // CHECK-NEXT: [[@LINE-3]]:14 -> [[@LINE-3]]:23 : 0 + // CHECK-NEXT: [[@LINE-4]]:24 -> [[@LINE-4]]:29 : 1 + // CHECK-NEXT: [[@LINE-5]]:29 -> [[@LINE-5]]:40 : 0 + // CHECK-NEXT: [[@LINE-6]]:35 -> [[@LINE-6]]:40 : (0 - 1) + // CHECK-NEXT: } +} +