dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.37k stars 4.75k forks source link

JIT: libraries jitstress AV failures #109730

Closed AndyAyersMS closed 5 days ago

AndyAyersMS commented 5 days ago

See eg https://dev.azure.com/dnceng-public/public/_build/results?buildId=867258&view=ms.vss-test-web.build-test-results-tab

windows x64 jitstress 2 (and many others)

DOTNET_JitStress=2
DOTNET_TieredCompilation=0

... 

C:\h\w\A30E091D\w\B1B8091D\e>"C:\h\w\A30E091D\p\dotnet.exe" exec --runtimeconfig System.Text.Json.Tests.runtimeconfig.json --depsfile System.Text.Json.Tests.deps.json xunit.console.dll System.Text.Json.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Text.Json.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Text.Json.Tests (found 8722 of 8789 test cases)
  Starting:    System.Text.Json.Tests (parallel test collections = on [4 threads], stop on fail = off)
Fatal error. 0xC0000005
dotnet-policy-service[bot] commented 5 days ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

AndyAyersMS commented 5 days ago

Looks like m_dfsTree is null when we got to build loop info in VN. Trying to track down how this happens.

Stress mode enables early prop to fold a null check under an explicit throw in a block that is BBJ_THROW. Remorphing during early prop sees the throw and unconditionally converts the block to BBJ_THROW and thinks it has changed control flow, and so invalidates the DFS. VN doesn't expect this and so crashes.

So one possible fix is to teach remorphing that changing a BBJ_THROW to a BBJ_THROW is a no-op, and the DFS is fine. Or we could have VN note if the DFS is invalid and rebuild it.

For now I'm going to do the detection as this seems like a rare case?

Going to add the fix to #109714

jakobbotsch commented 5 days ago

Note the DFS tree invalidation in morph was added by #106637, so that's what exposed this.

It would probably be a good idea to stop calling morph entirely from early prop, or to skip folding control flow entirely outside global morph.