Skip to content

Commit 6d467fd

Browse files
authored
[SILProfiler] Don't crash when coverage info for lazy getters is incomplete (#18744)
ASTWalker visits a lazy_initializer_expr once within its associated var_decl (by way of the parent nominal type). However, SILGen visits the lazy_initializer_expr while inside of the var_decl's getter. The result is that there is no coverage mapping information for the contents of the lazy init within the getter's SILProfiler. Fixing this will require reworking how profile counters are assigned to be more in line with what SILGen needs. As a stop-gap, this patch prevents SILGen from asserting that coverage mappings are complete with a defensive check which prevents a crash seen in SR-8429. rdar://42792053
1 parent 387c338 commit 6d467fd

File tree

3 files changed

+22
-2
lines changed

3 files changed

+22
-2
lines changed

lib/SIL/SILProfiler.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,16 @@ struct MapRegionCounters : public ASTWalker {
260260
} else if (auto *ACE = dyn_cast<AbstractClosureExpr>(E)) {
261261
return visitClosureExpr(*this, ACE, [&] { mapRegion(ACE); });
262262
}
263+
264+
// rdar://42792053
265+
// TODO: There's an outstanding issue here with LazyInitializerExpr. A LIE
266+
// is copied into the body of a property getter after type-checking (before
267+
// coverage). ASTWalker only visits this expression once via the property's
268+
// VarDecl, and does not visit it again within the getter. This results in
269+
// missing coverage. SILGen treats the init expr as part of the getter, but
270+
// its SILProfiler has no information about the init because the LIE isn't
271+
// visited here.
272+
263273
return {true, E};
264274
}
265275
};

lib/SILGen/SILGenFunction.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -653,8 +653,10 @@ void SILGenFunction::emitProfilerIncrement(ASTNode N) {
653653
auto &C = B.getASTContext();
654654
const auto &RegionCounterMap = SP->getRegionCounterMap();
655655
auto CounterIt = RegionCounterMap.find(N);
656-
assert(CounterIt != RegionCounterMap.end() &&
657-
"cannot increment non-existent counter");
656+
657+
// TODO: Assert that this cannot happen (rdar://42792053).
658+
if (CounterIt == RegionCounterMap.end())
659+
return;
658660

659661
auto Int32Ty = getLoweredType(BuiltinIntegerType::get(32, C));
660662
auto Int64Ty = getLoweredType(BuiltinIntegerType::get(64, C));

test/Profiler/coverage_class.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,11 @@ struct S2 {
3939
// CHECK-NEXT: [[@LINE+1]]:26 -> [[@LINE+1]]:27 : (1 - 0)
4040
var m1: Int = g1 ? 0 : 1
4141
}
42+
43+
// Test that the crash from SR-8429 is avoided. Follow-up work is
44+
// needed to generate the correct coverage mapping here. Coverage for
45+
// `offset` should be associated with its getter, not the class
46+
// constructor.
47+
class C2 {
48+
lazy var offset: Int = true ? 30 : 55
49+
}

0 commit comments

Comments
 (0)