[release/8.0-staging] Fix ILLink/ILC hang when tracking too many hoisted local values #95302
Conversation
…et#95041) dotnet#87634 introduced a limit on the number of values tracked in dataflow analysis, but this approach was incorrect because resetting the set of tracked values was effectively moving up the dataflow lattice, breaking the invariant and causing potential hangs. The hang was observed in dotnet#94831, where (as found by @vitek-karas) the hoisted local `state` field of an async method with many await points was getting a large number of tracked values, hitting the limit, being reset to "empty", but then growing again, causing the analysis not to converge. The fix is to instead introduce a special value `ValueSet<TValue>.Unknown` that is used for this case. It acts like "bottom" in the lattice, so that it destroys any other inputs on meet ("bottom" meet anything else is "bottom"). In this testcase the `state` field isn't involved in dataflow warnings, so this actually allows the method to be analyzed correctly, producing the expected warning for the `Type` local that flows across all of the await points. Fixes dotnet#94831
sbomer
requested review from
marek-safar and
MichalStrehovsky
as code owners
dotnet-issue-labeler
bot
added
the
needs-area-label
ghost
assigned
sbomer
added
the
area-Tools-ILLink
|
Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas Issue DetailsBackport of #95041 to release/8.0-staging Customer ImpactFixes a customer-reported hang when using TestingValidated fix with a repro testcase. RiskLow. The fix can lead to new warnings in some cases (as seen in the analyzer testcase), but these represent potential correctness issues, can be easily silenced if necessary, and should be very rare (only affect very large methods with particular patterns that cause tracking of a large number of dataflow values where we give up on the tracking).
|
sbomer
added
area-NativeAOT-coreclr
and removed
needs-area-label
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsBackport of #95041 to release/8.0-staging Customer ImpactFixes a customer-reported hang when using TestingValidated fix with a repro testcase. RiskLow. The fix can lead to new warnings in some cases (as seen in the analyzer testcase), but these represent potential correctness issues, can be easily silenced if necessary, and should be very rare (only affect very large methods with particular patterns that cause tracking of a large number of dataflow values where we give up on the tracking).
|
rbhanda
added
Servicing-approved
jeffschwMSFT
merged commit 3001c75
into
dotnet:release/8.0-staging
Backport of #95041 to release/8.0-staging
Customer Impact
Fixes a customer-reported hang when using
PublishTrimmedin an app that happened to have a large method with a lot ofawaits. The same issue is also present withPublishAot.Testing
Validated fix with a repro testcase.
Risk
Low. The fix can lead to new warnings in some cases (as seen in the analyzer testcase), but these represent potential correctness issues, can be easily silenced if necessary, and should be very rare (only affect very large methods with particular patterns that cause tracking of a large number of dataflow values where we give up on the tracking).