Assertion failed 'node->DefinesLocalAddr(this, size, &lcl, nullptr) && lvaGetDesc(lcl)->lvHiddenBufferStructArg'

[BruceForstall]

pipeline: runtime-coreclr libraries-jitstress
Test: System.Reflection.MetadataLoadContext.Tests
JitStress=1
Failed for both win-x64 and linux-arm

https://dev.azure.com/dnceng/public/_build/results?buildId=1741924&view=ms.vss-test-web.build-test-results-tab&runId=47086172&resultId=193772&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab

+ printenv
+ grep COMPlus
COMPlus_TieredCompilation=0
COMPlus_JitStress=1
COMPlus_DbgMiniDumpName=/home/helixbot/dotnetbuild/dumps/coredump.%d.dmp
COMPlus_DbgEnableMiniDump=1
+ ./RunTests.sh --runtime-path /root/helix/work/correlation
----- start Thu Apr 28 07:47:43 UTC 2022 =============== To repro directly: =====================================================
pushd .
/root/helix/work/correlation/dotnet exec --runtimeconfig System.Reflection.MetadataLoadContext.Tests.runtimeconfig.json --depsfile System.Reflection.MetadataLoadContext.Tests.deps.json xunit.console.dll System.Reflection.MetadataLoadContext.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing 
popd
===========================================================================================================
/root/helix/work/workitem/e /root/helix/work/workitem/e
  Discovering: System.Reflection.MetadataLoadContext.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Reflection.MetadataLoadContext.Tests (found 493 of 494 test cases)
  Starting:    System.Reflection.MetadataLoadContext.Tests (parallel test collections = on, max threads = 4)

Assert failure(PID 61 [0x0000003d], Thread: 75 [0x004b]): Assertion failed 'node->DefinesLocalAddr(this, size, &lcl, nullptr) && lvaGetDesc(lcl)->lvHiddenBufferStructArg' in 'System.Reflection.TypeLoading.Ecma.EcmaEvent:ComputeEventRaiseMethod():System.Reflection.TypeLoading.RoMethod:this' during 'Morph - Global' (IL size 37; hash 0x941ee672; FullOpts)

    File: /__w/1/s/src/coreclr/jit/gentree.cpp Line: 17913
    Image: /root/helix/work/correlation/dotnet

@dotnet/jit-contrib

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

Issue Details

---

pipeline: runtime-coreclr libraries-jitstress
Test: System.Reflection.MetadataLoadContext.Tests
JitStress=1
Failed for both win-x64 and linux-arm

https://dev.azure.com/dnceng/public/_build/results?buildId=1741924&view=ms.vss-test-web.build-test-results-tab&runId=47086172&resultId=193772&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab

+ printenv
+ grep COMPlus
COMPlus_TieredCompilation=0
COMPlus_JitStress=1
COMPlus_DbgMiniDumpName=/home/helixbot/dotnetbuild/dumps/coredump.%d.dmp
COMPlus_DbgEnableMiniDump=1
+ ./RunTests.sh --runtime-path /root/helix/work/correlation
----- start Thu Apr 28 07:47:43 UTC 2022 =============== To repro directly: =====================================================
pushd .
/root/helix/work/correlation/dotnet exec --runtimeconfig System.Reflection.MetadataLoadContext.Tests.runtimeconfig.json --depsfile System.Reflection.MetadataLoadContext.Tests.deps.json xunit.console.dll System.Reflection.MetadataLoadContext.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing 
popd
===========================================================================================================
/root/helix/work/workitem/e /root/helix/work/workitem/e
  Discovering: System.Reflection.MetadataLoadContext.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Reflection.MetadataLoadContext.Tests (found 493 of 494 test cases)
  Starting:    System.Reflection.MetadataLoadContext.Tests (parallel test collections = on, max threads = 4)

Assert failure(PID 61 [0x0000003d], Thread: 75 [0x004b]): Assertion failed 'node->DefinesLocalAddr(this, size, &lcl, nullptr) && lvaGetDesc(lcl)->lvHiddenBufferStructArg' in 'System.Reflection.TypeLoading.Ecma.EcmaEvent:ComputeEventRaiseMethod():System.Reflection.TypeLoading.RoMethod:this' during 'Morph - Global' (IL size 37; hash 0x941ee672; FullOpts)

    File: /__w/1/s/src/coreclr/jit/gentree.cpp Line: 17913
    Image: /root/helix/work/correlation/dotnet

@dotnet/jit-contrib

Author:	BruceForstall
Assignees:	-
Labels:	JitStress, area-CodeGen-coreclr
Milestone:	7.0.0

[jakobbotsch]

One part of the problem is that we end up with a GTF_GLOB_REF on a GT_ADDR node because the following does not handle locals generally:

runtime/src/coreclr/jit/morph.cpp

Lines 11190 to 11193 in 11bff1a

	if (oper == GT_ADDR && (op1->gtOper == GT_LCL_VAR))
	{
	tree->gtFlags &= ~GTF_GLOB_REF;
	}

It means we end up evaluating the retbuf into a temp and stop being able to recognize it.
This seems to end up happening because in this case we have an isTmp local for the retbuf we are optimizing, and this local is address exposed besides as the ret buf:

[000198] S-C-G------                         ▌  CALL      void   System.Reflection.TypeLoading.Ecma.MetadataExtensions.GetMethodDefinition
[000203] ----------- retbuf                  ├──▌  ADDR      byref 
[000202] -------N---                         │  └──▌  FIELD     struct _neverAccessThisExceptThroughMethodDefinitionProperty
[000191] -----------                         │     └──▌  ADDR      byref 
[000192] -------N---                         │        └──▌  LCL_VAR   struct<System.Reflection.TypeLoading.Ecma.EcmaMethodDecoder, 32>(AX)(RB) V16 tmp14        
[000200] n---------- arg1                    ├──▌  OBJ       struct<System.Reflection.Metadata.MethodDefinitionHandle, 4>
[000199] -----------                         │  └──▌  ADDR      byref 
[000193] -------N---                         │     └──▌  LCL_VAR   struct<System.Reflection.Metadata.MethodDefinitionHandle, 4> V18 tmp16        
[000196] --C-G------ arg2                    └──▌  CALL      ref    System.Reflection.TypeLoading.Ecma.EcmaMethodDecoder.get_Reader
[000194] ----------- this                       └──▌  LCL_VAR_ADDR byref  V16 tmp14

@kunalspathak said he would take a closer look.

[jakobbotsch]

#68741 at least fixes introducing the unnecessary temp which resolves the assert. However, I think we should leave this open just to investigate whether it is in general safe to do the ret buf optimization here. I suppose yes, since the local does end up marked as address exposed and will not actually be tracked.

I have attached a SPMI context for the compile where it happens which I collected on git hash 5c2a738 (with the fix from #68741 applied). The above tree comes from around line 4334 in the JIT dump.

context.zip