Assert debugging System.Private.CoreLib in DEBUG build

[AaronRobinsonMSFT]

Looks like this was introduced in #108809

Assert failure(PID 38768 [0x00009770], Thread: 42112 [0xa480]): m_instrAttrib.m_fIsCall

CORECLR! DebuggerPatchSkip::IsInPlaceSingleStep + 0x2F (0x00007ffe`91e9447f)
CORECLR! DebuggerPatchSkip::DebuggerPatchSkip + 0x606 (0x00007ffe`91e827e6)
CORECLR! DebuggerController::ActivatePatchSkip + 0x164 (0x00007ffe`91e84584)
CORECLR! DebuggerController::DispatchPatchOrSingleStep + 0xF74 (0x00007ffe`91e8e3d4)
CORECLR! DebuggerController::DispatchNativeException + 0xAD0 (0x00007ffe`91e8d1f0)
CORECLR! Debugger::FirstChanceNativeException + 0x508 (0x00007ffe`91ebf618)
CORECLR! IsDebuggerFault + 0x8D (0x00007ffe`9152254d)
CORECLR! CLRVectoredExceptionHandlerPhase2 + 0x18D (0x00007ffe`9150df3d)
CORECLR! CLRVectoredExceptionHandler + 0x3DA (0x00007ffe`9150dd8a)
CORECLR! CLRVectoredExceptionHandlerShim + 0x248 (0x00007ffe`9150e8a8)
    File: ...\runtime\src\coreclr\debug\ee\controller.h:1543
    Image: ...\runtime\artifacts\tests\coreclr\windows.x64.Debug\Tests\Core_Root\corerun.exe

Running with the following:

DOTNET_TieredCompilation=0
DOTNET_ReadyToRun=0

/cc @tommcdon

[dotnet-policy-service]

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

[AaronRobinsonMSFT]

/cc @dotnet/dotnet-diag

[noahfalk]

Thanks Aaron!

@jeffschwMSFT - heads up, it looks like there was a bug in #108809 which was recently merged to .NET 9 release. Its not known yet whether the issue would be ultimately problematic for customers, but its possible.

The fix in 108809 is intended to only activate for breakpoints on CALL opcodes when CET is enabled or the user opted-in via environment variable. However it appears that in cases where the fix was not intended to activate the variable which controls activation remains uninitialized rather than being explicitly set FALSE. This means the fix might activate in scenarios that we didn't consider or explicitly test for. Thinking about it for a few minutes I'm not aware of any obvious problem this would create, but at minimum there is a new source of risk that wasn't initially included in the decision to take the fix.

I think the lower risk option is to make a quick 2nd fix to deterministically initialize the variable so I'm going to prioritize that route. The alternative is to leave the code as-is but increase the testing to prove that randomly enabling fix outside of its intended scope didn't create a problem.

[noahfalk]

Exploratory testing with a private build where the m_fInPlaceSS variable is always initialized to true caused numerous failures (ExecutionEngineExceptions) in debugging tests. I haven't had time to investigate the exact sequence of events that leads to the failure and user error is of course possible but the conservative interpretation is that as-is we've got a non-deterministic crash bug in most debugging scenarios.

[noahfalk]

Fixed by #109008