dotnet / jitutils

MIT License
150 stars 60 forks source link

Assertions are no longer logged or detected in the default scenario #326

Open tannergooding opened 3 years ago

tannergooding commented 3 years ago

Issue

The assertion detecting logic in PMIDriver (https://github.com/dotnet/jitutils/blob/945715f07ce9c4381b3ab199f2cd52b586a89415/src/pmi/PMIDriver.cs#L214-L267) is no longer correct for what the JIT currently outputs.

Additionally, due to other changes in the JIT, assertions are never logged in the default scenario.

Repro

  1. Clone dotnet/runtime
  2. Build the repo in all required configurations
  3. Clone dotnet/jitutils
  4. Build the repo using .\bootstrap.cmd
  5. In the dotnet/runtime root, run jit-diff.bat diff --diff --pmi

Observe that the command fails due to https://github.com/dotnet/runtime/issues/51728. However, System.Private.Corelib.err reports: General error, no assert seen.

Analysis

assertAbort (https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/error.cpp#L277-L323) currently fails to log anything because COMPlus_JitFuncInfoLogFile is not set and because the default CoreCLR.dll (VM) for the PMI diffs is release which means CEEInfo::doAssert does not actually print anything: https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/jitinterface.cpp#L10897-L10925

Additionally, even if the assertions are printed, they currently resemble:

Encoding:GetCharsWithFallback(long,int,long,int,int,int):int:this - Assertion failed (C:\Users\tagoo\Source\repos\runtime_base\src\coreclr\jit\emitxarch.cpp:11739 - ((regMask & emitThisGCrefRegs) && (ins == INS_add)) || ((regMask & emitThisByrefRegs) && (ins == INS_add || ins == INS_sub))) during Emit code

While the PMIDriver is currently looking for ]): Assertion failed ' and Assert failure(PID, neither of which are used by this failure path.

tannergooding commented 3 years ago

CC. @dotnet/jit-contrib, @BruceForstall