Skip to content

Commit 322ce4e

Browse files
hamishknighthamishknight
committed
[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
1 parent 6d3017c commit 322ce4e

File tree

3 files changed

+143
-10
lines changed

3 files changed

+143
-10
lines changed

‎lib/SIL/IR/SILProfiler.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,12 @@ struct MapRegionCounters : public ASTWalker {
295295
return LazyInitializerWalking::InAccessor;
296296
}
297297

298+
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
299+
// Don't walk into PropertyWrapperValuePlaceholderExprs, these should be
300+
// mapped as part of the wrapped value initialization.
301+
return false;
302+
}
303+
298304
void mapRegion(ASTNode N) {
299305
mapRegion(ProfileCounterRef::node(N));
300306
}
@@ -683,6 +689,12 @@ struct PGOMapping : public ASTWalker {
683689
return LazyInitializerWalking::InAccessor;
684690
}
685691

692+
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
693+
// Don't walk into PropertyWrapperValuePlaceholderExprs, these should be
694+
// mapped as part of the wrapped value initialization.
695+
return false;
696+
}
697+
686698
MacroWalking getMacroWalkingBehavior() const override {
687699
return MacroWalking::Expansion;
688700
}
@@ -1102,6 +1114,12 @@ struct CoverageMapping : public ASTWalker {
11021114
return LazyInitializerWalking::InAccessor;
11031115
}
11041116

1117+
bool shouldWalkIntoPropertyWrapperPlaceholderValue() override {
1118+
// Don't walk into PropertyWrapperValuePlaceholderExprs, these should be
1119+
// mapped as part of the wrapped value initialization.
1120+
return false;
1121+
}
1122+
11051123
MacroWalking getMacroWalkingBehavior() const override {
11061124
return MacroWalking::Expansion;
11071125
}

‎test/Profiler/coverage_errors.swift

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ func test6(
248248
} // CHECK-NEXT: }
249249

250250
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors5test7s5Int32VyF"
251-
func test7() -> Int32 { // CHECK-NEXT: [[@LINE]]:23 -> [[@LINE+31]]:2 : 0
251+
func test7() -> Int32 { // CHECK-NEXT: [[@LINE]]:23 -> [[@LINE+29]]:2 : 0
252252
var x : Int32 = 0
253253

254254
do { // CHECK-NEXT: [[@LINE]]:6 -> [[@LINE+3]]:4 : 0
@@ -271,8 +271,6 @@ func test7() -> Int32 { // CHECK-NEXT: [[@LINE]]:23 -> [[@LINE+31]]:2 : 0
271271
} catch _ {} // CHECK-NEXT: [[@LINE]]:13 -> [[@LINE]]:15 : 6
272272
// CHECK-NEXT: [[@LINE-1]]:15 -> {{[0-9:]+}} : (((((1 + 2) + 4) + 6) - 3) - 5)
273273

274-
// TODO: We ought to realize that everything after try! is unreachable
275-
// This is similar issue to rdar://100896177
276274
try! test6 {
277275
() throws -> () in
278276
return
@@ -817,8 +815,8 @@ func test65() throws { // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE+5]]:2 : 0
817815
// CHECK-NEXT: [[@LINE-1]]:29 -> [[@LINE+1]]:2 : (0 - 2)
818816
} // CHECK-NEXT: }
819817

820-
struct TestInit {
821-
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_errors.TestInit.init() -> coverage_errors.TestInit
818+
struct TestType1 {
819+
// CHECK-LABEL: sil_coverage_map {{.*}}// coverage_errors.TestType1.init() -> coverage_errors.TestType1
822820
init() { // CHECK-NEXT: [[@LINE]]:10 -> [[@LINE+5]]:4 : 0
823821
do { // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+2]]:6 : 0
824822
throw SomeErr.Err1
@@ -827,31 +825,105 @@ struct TestInit {
827825
} // CHECK-NEXT: }
828826
}
829827

830-
struct TestProp {
828+
@propertyWrapper
829+
struct Wrapper<T> {
830+
var wrappedValue: T
831+
832+
init(wrappedValue: T) {
833+
self.wrappedValue = wrappedValue
834+
}
835+
836+
init(wrappedValue: T, x: T, y: T? = nil) {
837+
self.wrappedValue = wrappedValue
838+
}
839+
}
840+
841+
struct TestType2 {
831842
let a = try? throwingFn()
832-
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1aSiSgvpfi"
843+
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1aSiSgvpfi"
833844
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:28 : 0
834845
// CHECK-NEXT: }
835846

836847
let b = try? (throwingFn(), 0)
837-
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1bSi_SitSgvpfi"
848+
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1bSi_SitSgvpfi"
838849
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:33 : 0
839850
// CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:33 : (0 - 1)
840851
// CHECK-NEXT: }
841852

842853
let c = try? (throwingFn(), .random() ? 0 : 1)
843-
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1cSi_SitSgvpfi"
854+
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1cSi_SitSgvpfi"
844855
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:49 : 0
845856
// CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:49 : (0 - 1)
846857
// CHECK-NEXT: [[@LINE-4]]:43 -> [[@LINE-4]]:44 : 2
847858
// CHECK-NEXT: [[@LINE-5]]:47 -> [[@LINE-5]]:48 : ((0 - 1) - 2)
848859
// CHECK-NEXT: }
849860

850861
let d = (try? (throwingFn(), .random() ? 0 : 1), 0)
851-
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors8TestPropV1dSi_SitSg_Sitvpfi"
862+
// CHECK-LABEL: sil_coverage_map {{.*}} "$s15coverage_errors9TestType2V1dSi_SitSg_Sitvpfi"
852863
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:54 : 0
853864
// CHECK-NEXT: [[@LINE-3]]:30 -> [[@LINE-3]]:50 : (0 - 1)
854865
// CHECK-NEXT: [[@LINE-4]]:44 -> [[@LINE-4]]:45 : 2
855866
// CHECK-NEXT: [[@LINE-5]]:48 -> [[@LINE-5]]:49 : ((0 - 1) - 2)
856867
// CHECK-NEXT: }
857868
}
869+
870+
// rdar://118939162 - Make sure we don't crash when generating coverage for 'try!' in a property wrapper init.
871+
struct TestType3 {
872+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType3.x
873+
// CHECK-NEXT: [[@LINE+2]]:4 -> [[@LINE+2]]:13 : 0
874+
// CHECK-NEXT: }
875+
@Wrapper()
876+
var x = try! throwingFn()
877+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType3.(_x
878+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:28 : 0
879+
// CHECK-NEXT: }
880+
}
881+
882+
struct TestType4 {
883+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType4.x
884+
// CHECK-NEXT: [[@LINE+2]]:4 -> [[@LINE+2]]:13 : 0
885+
// CHECK-NEXT: }
886+
@Wrapper()
887+
var x = try! (throwingFn(), 0)
888+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType4.(_x
889+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:33 : 0
890+
// CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:33 : (0 - 1)
891+
// CHECK-NEXT: }
892+
}
893+
894+
struct TestType5 {
895+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType5.x
896+
// CHECK-NEXT: [[@LINE+3]]:4 -> [[@LINE+3]]:33 : 0
897+
// CHECK-NEXT: [[@LINE+2]]:32 -> [[@LINE+2]]:33 : (0 - 1)
898+
// CHECK-NEXT: }
899+
@Wrapper(x: try! throwingFn())
900+
var x = 0
901+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType5.(_x
902+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:12 : 0
903+
// CHECK-NEXT: }
904+
}
905+
906+
struct TestType6 {
907+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType6.x
908+
// CHECK-NEXT: [[@LINE+3]]:4 -> [[@LINE+3]]:39 : 0
909+
// CHECK-NEXT: [[@LINE+2]]:32 -> [[@LINE+2]]:39 : (0 - 1)
910+
// CHECK-NEXT: }
911+
@Wrapper(x: try! throwingFn(), y: 0)
912+
var x = 0
913+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType6.(_x
914+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:12 : 0
915+
// CHECK-NEXT: }
916+
}
917+
918+
struct TestType7 {
919+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_errors.TestType7.x
920+
// CHECK-NEXT: [[@LINE+3]]:4 -> [[@LINE+3]]:49 : 0
921+
// CHECK-NEXT: [[@LINE+2]]:33 -> [[@LINE+2]]:49 : (0 - 1)
922+
// CHECK-NEXT: }
923+
@Wrapper(x: try! (throwingFn(), 0), y: (0, 0))
924+
var x = try! (throwingFn(), 0)
925+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_errors.TestType7.(_x
926+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:33 : 0
927+
// CHECK-NEXT: [[@LINE-3]]:29 -> [[@LINE-3]]:33 : (0 - 1)
928+
// CHECK-NEXT: }
929+
}

‎test/Profiler/coverage_property_wrapper_backing.swift

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,29 @@ struct PassThroughWrapper<T> {
2525
// CHECK: [[BB]]:
2626
// CHECK-NEXT: increment_profiler_counter 1
2727

28+
// CHECK-LABEL: sil hidden @$s33coverage_property_wrapper_backing1UV1xSivpfP : $@convention(thin) (Int) -> Wrapper<Int>
29+
// CHECK: function_ref @$sSb6randomSbyFZ : $@convention(method) (@thin Bool.Type) -> Bool
30+
// CHECK: cond_br {{%[0-9]+}}, [[BB_TRUE:bb[0-9]+]], [[BB_FALSE:bb[0-9]+]]
31+
//
32+
// CHECK: [[BB_FALSE]]:
33+
// CHECK: integer_literal {{.*}}, 2
34+
//
35+
// CHECK: [[BB_TRUE]]:
36+
// CHECK: increment_profiler_counter 1
37+
// CHECK: integer_literal {{.*}}, 1
38+
39+
// CHECK-LABEL: sil hidden [transparent] @$s33coverage_property_wrapper_backing1UV2_{{.*}}7WrapperVySiGvpfi : $@convention(thin) () -> Int
40+
// CHECK: increment_profiler_counter 0
41+
// CHECK: function_ref @$sSb6randomSbyFZ : $@convention(method) (@thin Bool.Type) -> Bool
42+
// CHECK: cond_br {{%[0-9]+}}, [[BB_TRUE:bb[0-9]+]], [[BB_FALSE:bb[0-9]+]]
43+
44+
// CHECK: [[BB_FALSE]]:
45+
// CHECK: integer_literal {{.*}}, 4
46+
//
47+
// CHECK: [[BB_TRUE]]:
48+
// CHECK: increment_profiler_counter 1
49+
// CHECK: integer_literal {{.*}}, 3
50+
2851
struct S {
2952
// CHECK-LABEL: sil_coverage_map {{.*}} "$s33coverage_property_wrapper_backing1SV1iSivpfP" {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.S.i
3053
// CHECK-NEXT: [[@LINE+4]]:4 -> [[@LINE+4]]:30 : 0
@@ -55,3 +78,23 @@ struct T {
5578
@Wrapper(.random() ? 1 : 2)
5679
var k = 3
5780
}
81+
82+
// rdar://118939162 - Make sure we don't include the initialization expression
83+
// in the backing initializer.
84+
struct U {
85+
// CHECK-LABEL: sil_coverage_map {{.*}} // property wrapper backing initializer of coverage_property_wrapper_backing.U.x
86+
// CHECK-NEXT: [[@LINE+4]]:4 -> [[@LINE+4]]:30 : 0
87+
// CHECK-NEXT: [[@LINE+3]]:24 -> [[@LINE+3]]:25 : 1
88+
// CHECK-NEXT: [[@LINE+2]]:28 -> [[@LINE+2]]:29 : (0 - 1)
89+
// CHECK-NEXT: }
90+
@Wrapper(.random() ? 1 : 2)
91+
var x = if .random() { 3 } else { 4 }
92+
// CHECK-LABEL: sil_coverage_map {{.*}} // variable initialization expression of coverage_property_wrapper_backing.U.(_x
93+
// CHECK-NEXT: [[@LINE-2]]:11 -> [[@LINE-2]]:40 : 0
94+
// CHECK-NEXT: [[@LINE-3]]:14 -> [[@LINE-3]]:23 : 0
95+
// CHECK-NEXT: [[@LINE-4]]:24 -> [[@LINE-4]]:29 : 1
96+
// CHECK-NEXT: [[@LINE-5]]:29 -> [[@LINE-5]]:40 : 0
97+
// CHECK-NEXT: [[@LINE-6]]:35 -> [[@LINE-6]]:40 : (0 - 1)
98+
// CHECK-NEXT: }
99+
}
100+

0 commit comments

Comments
 (0)