@AndyAyersMS PTAL

Also review #13245 for more info

I will try to obtain a test case:
The method must be compiled for Debug,
Have GC args in the outgoing stack area, (i.e as the 5 or 6 argument)
The first four args should be side-effect free (of calls and assignments) and one of the first four args should use an array element (with a bounds check operation).

@dotnet-bot test Windows_NT x64 Checked gcstress0xc

Kind of scary that this has been in the code so long without causing trouble.

Do we commonly run GC stress on debug codegen? If not, perhaps we should.

Change LGTM but let's see what Carol / Bruce have to say.

I have asked vendors to try running CoreFX tests with GCStress=3 on chk builds as well as the usual ret (at least chk of CoreFX, we don't test chk of CoreCLR over in CoreFX)

Do you guys run GCStress using CoreFX tests at all? Eg in CI?

Yes we can/do run gc stress, but not AFAIK on CoreFX tests.
I ran GCStress on this PR with the text: (see above)
AtSign test Windows_NT x64 Checked gcstress0xc

Note that you need to run gcstress 4 or C to get coverange on every instruction in the JIT code.
GCStress 1 or 3 isn't really going to catch this kind of a GC issue.

Note that using gcstress 4 or C apparently also requires access to a Disassembler library for x64 instructions which wasn't available automatically for me when I tried running it by hand.

I have asked vendors to try running CoreFX tests with GCStress=3 on chk builds as well as the usual ret (at least chk of CoreFX, we don't test chk of CoreCLR over in CoreFX)

@danmosemsft I think it is a lot more valuable to run GCStress on CoreCLR checked builds. Checked CoreCLR builds have a lot of important asserts that catch problems sooner, and even in situations that do not lead to crashes.

I have traced down this GC hole using CoreCLR checked build. It resulted into assert on checked CoreCLR build that was hit pretty close to the place where the problem happened. If I was trying to trace it down using CoreCLR release build, I would be probably still scratching my head where it might be coming from.

@jkotas is that just better for diagnosing or also for finding? If former then I would rather test on retail so it doesn't take a life age to complete then repeat the run using Chk on whatever set of tests failed. Right now is really slow on ret and found nothing so far.

I think it is also better for finding. I recommend you chat with Scott Wadsworth about it. He has a lot more insights and opinions on how this kind of testing should be done.

OK. Also will talk to @valenis. Probably a few random vendor VMs is not going to cut it.

Right now is really slow on ret and found nothing so far.

It is slow because of the default xunit driver is not suitable for runtime testing like this. The default xunit driver is using a ton of reflection, and so you are basically GC stress testing the heck out of the one reflection path again and again, and a little bit of the actual tests.

Runtime testing like this needs a different xunit driver that does the reflection at "build time" and transforms the test into a bunch of static calls without reflection. (ProjectN tests that live in TFS use driver like that.)

I get that you can run gc stress in CI like you did above. Just wondering how much gc stress testing we get for the case where the jit is generating debug code.

I know some JIT tests have _d variants so we get a some coverage that way, but the core library and framework assemblies we use in CI are probably not requesting debug codegen.

Looks like we can sort of simulate this via COMPlus_JitDebuggable (like we do for minopts) and then running stress on a CHK build. It won't quite have the same IL though.

Shall I check this in now?
@AndyAyersMS @CarolEidt @BruceForstall

FWIW, CoreCLR runs lots of CoreFX test jobs, and runs lots of GCStress test jobs, but has no CoreFX + GCStress jobs (e.g., see https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/ for all "stress" jobs). Partially this is because CoreFX jobs are not clean, and GCStress jobs are not clean, and the combination is probably even worse (less likely to run clean).

@AndyAyersMS Looks like we currently set debuggable via:

    opts.compDbgCode = jitFlags->IsSet(JitFlags::JIT_FLAG_DEBUG_CODE);

Maybe we need to add a debuggable stress mode, e.g.

STRESS_MODE(DEBUGGABLE)

and then use:

    opts.compDbgCode = jitFlags->IsSet(JitFlags::JIT_FLAG_DEBUG_CODE) || compStressCompile(STRESS_DEBUGGABLE, 10));

I added a test case that fails without this fix, when run under GCStress=4

@dotnet-bot test Windows_NT x64 Checked gcstress0xc

@dotnet-bot test Windows_NT x64 Checked gcstress0xc