runtime: JIT reads elements from the wrong ImmutableArray when enumerating ImmutableArray multiple times

Tagging @DavidWrighton @jcouv

Roslyn has encountered a crash dotnet/roslyn#46575 which we can only reproduce when building the runtime in fairly specific circumstances. You can see stack trace info, etc. in the linked bug in Roslyn. It requires building the runtime using a release mode compiler from later than Aug 4th or so, and building using msbuild, not calling the compiler directly.

Analysis

Looking at the crash dump, @davidwrighton identified that the JIT produced incorrect assembly. In the body of ExplicitInterfaceImplementation (source), we have two locals of the same type (ImmutableArray<MethodSymbol>) and we loop through one of them twice. In the second loop, the assembly uses the wrong local to index into the array. As a result, we get a crash/NRE with method being null on this line.

Repro

To reproduce this crash in dotnet/runtime:

Checkout the triage-jit-crash branch and clean the artifacts out of your local repo.
- There are currently several compiler breaking changes in flight at the same time as this crash. This repro branch is based on release/5.0, but hacking around some unrelated compiler issues in order to get us to the point where we can repro the bug.
Run .\Build.cmd -restore, then .\Build.cmd libs -bl (having a binlog with -bl will probably be handy).
- You should see crashes in the projects for ref\System.Diagnostics.Process.csproj and ref\System.Data.Common.csproj.

Details

We have found differences in the native code produced for the PEMethodSymbol.get_ExplicitInterfaceImplementations method when running from msbuild and when running csc using the args produced by msbuild. I have attached the output from the Disassembly window for these methods. The interesting bit seems to be around line 210 where in the broken version, we read the length from the explicitlyOverriddenMethods array, but seem to read elements from the explicitInterfaceImplementations array. (I’m not skilled in reading disassembly, so I could have mangled the description of this, sorry.)

It could be illustrative to pop these two disassembled methods into a diff tool and compare what they are doing.

PEMethodSymbol.ExplicitInterfaceImplementations-broken.txt PEMethodSymbol.ExplicitInterfaceImplementations-working.txt

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 23 (23 by maintainers)

Commits related to this issue

Prospective fix Fixes #41100. — committed to AndyAyersMS/runtime by AndyAyersMS 4 years ago

Most upvoted comments

Have verified the prototype fix leads to successful libraries build.

C:\repos\runtime1\.dotnet\shared\Microsoft.NETCore.App\5.0.0-preview.8.20361.2 >
copy \repos\runtime4\artifacts\bin\coreclr\Windows_NT.x64.Checked\clrjit.dll .

C:\repos\runtime1 (triage-jit-crash) > .\Build.cmd libs -bl
... 
Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:15:58.89

AndyAyersMS on Aug 21, 2020

Psychic debugging would say this is an issue with CSE, but that’s just a guess.

I can repro the failure via msbuild, but can’t yet get the extracted command line to repro the issue. @davidwrighton any tips on how you did the latter?

AndyAyersMS on Aug 20, 2020

Fixed in 5.0 by #41243.

AndyAyersMS on Aug 25, 2020