JIT: fix filter liveness computation #23138
Conversation
Port of dotnet#23044 to release/2.1. When a filter is finished executing, control can logically pass to the associated handler, any enclosing handler or filter, or any finally or fault handler nested within the associated try. This is a consequence of two-pass EH. The jit was not propagating liveness from the nested handlers, which lead to a live object being collected inadvertently. This change updates `fgGetHandlerLiveVars` to find the nested handlers and merge their live-in into the filter block live sets. Because these implicit EH flow edges can create cycles in the liveness dataflow equations, the jit will also now always iterate liveness when it sees there is exception flow, to ensure livness reaches the appropriate fixed point. Added test case. Closes #22820.
DescriptionWhen running on x86, in code where a try/filter has a nested try/finally and a GC is triggered in first pass EH while the filter is executing, GC may collect objects that need to remain live until a handler is executed in second pass EH. Test case added with this PR shows one such example, derived from customer code; here the nested try/finally comes from a The fix is to incorporate liveness information from all reachable handlers into the filter liveness computation. The jit was already including liveness from enclosing try/finally handlers, but not from enclosed try/finally handlers. Customer ImpactThis bug is causing crashes in RavenDB running on x86 on .Net Core 2.1. Regression?Yes. Core 2.0 and earlier used Jit32 for x86, which is apparently less prone to this sort of issue, though it has quite similar liveness logic for filters. RiskModerate. The intersection of GC and EH is complex, and this issue also involves filters, which come with additional complications. But the risk is somewhat mitigated because filters are not commonly used, and this change increases the amount of local state reported live to GC. So it should be conseratively sound. One concern brought up in review is that this change increases the chances that a GC ref could be double-reported (in particular for funclet EH models). But the double-reporting issue was already a concern in filter reporting, and the jit is already set up to use pinning in filter scopes to make the double reporting tolerable. |
vivmishra
added
Servicing-approved
|
Pending perf run by ASP.NET. @Eilon |
vivmishra
added
Servicing-consider
|
Optimistically merging so a build can be created and an ASP.NET perf run can be done. |
vivmishra
added
Servicing-approved
Port of #23044 to release/2.1.
When a filter is finished executing, control can logically pass to the
associated handler, any enclosing handler or filter, or any finally or fault
handler nested within the associated try. This is a consequence of two-pass EH.
The jit was not propagating liveness from the nested handlers, which lead to a
live object being collected inadvertently.
This change updates
fgGetHandlerLiveVarsto find the nested handlers andmerge their live-in into the filter block live sets.
Because these implicit EH flow edges can create cycles in the liveness dataflow
equations, the jit will also now always iterate liveness when it sees there is
exception flow, to ensure livness reaches the appropriate fixed point.
Added test case.
Closes #22820.