R2R + Jit issue when inlining intrinsic methods

[steveharter]

Based on a discussion in the open PR, when an intrinsic is JIT'd an AV occurs later in CEEInfo::getArgType when it calls GetModule(sig->scope) which calls GetDynamicResolver(scope)->GetDynamicMethod().

This is a random exception, and only occurs in a release (or checked) build of the runtime when R2R is used on S.P.Corelib. Setting "dotnet_readytorun" to 0 prevents the exception. So when the JIT compiler decides to JIT the intrinsic method that was compiled for R2R, it causes the AV.

Repro:

1. Clone the PR referenced above
2. Change the class InstanceCalliHelper in file src\coreclr\System.Private.CoreLib\src\System\Reflection\InstanceCalliHelper.cs so that every [MethodImpl(MethodImplOptions.NoInlining)] is replaced with [MethodImpl(MethodImplOptions.AggressiveInlining)]
3. Build runtime in release or checked.
4. Run the test System.Reflection.Tests.MethodInfoTests.TestProperties
5. You may want to put a breakpoint in MethodDesc::GenerateFunctionPointerCall(DynamicResolver** resolver, COR_ILMETHOD_DECODER** methodILDecoder) (file instancecaii.cpp) to encourage the timing necessary to repro.
6. Run this a few times until the failure occurs. If the breakpoint is hit in the step above, the exception should occur afterwards.

Sample repro:

cd C:\git\YOURREPO\artifacts\bin\System.Reflection.Tests\Release\net10.0

C:\git\YOURREPO\artifacts\bin\testhost\net10.0-windows-Release-x64\dotnet.exe exec --runtimeconfig System.Reflection.Tests.runtimeconfig.json c:\Users\YOURALIAS\.nuget\packages\microsoft.dotnet.xunitconsolerunner\2.9.0-beta.24475.5\tools\net9.0\xunit.console.dll System.Reflection.Tests.dll -method System.Reflection.Tests.MethodInfoTests.TestProperties

[steveharter]

The AV is occurring because sig->scope is pointing to memory that has been freed. The scope (which contains the new signature containing the function pointer signature with (HasThis|ExplicitThis) was freed because the intrinsic has been jitted and the corresponding 'scope" was freed in TransientMethodDetails::~TransientMethodDetails().

@AaronRobinsonMSFT the scope containing the dynamic method also contains its own definition of a function pointer, but after being jitted, the scope\context containing that function pointer signature is deleted. Do we have any other cases of an intrinsic that defines its own signatures? Is it OK to tell the jitter not to inline these intrinsics?

Full call stack:

coreclr.dll!ILStubResolver::`scalar deleting destructor'(unsigned int)
coreclr.dll!TransientMethodDetails::~TransientMethodDetails() Line 214
	at C:\git\rfp2\src\coreclr\vm\jitinterface.cpp(214)
coreclr.dll!SArray<TransientMethodDetails,0>::~SArray<TransientMethodDetails,0>() Line 72
	at C:\git\rfp2\src\coreclr\inc\sarray.inl(72)
coreclr.dll!CEEInfo::~CEEInfo() Line 439
	at C:\git\rfp2\src\coreclr\vm\jitinterface.h(439)
coreclr.dll!UnsafeJitFunction(PrepareCodeConfig * config=0x000000228a57bb70, COR_ILMETHOD_DECODER * ILHeader=0x000000228a57b410, CORJIT_FLAGS * pJitFlags=0x000000228a57b270, unsigned long * pSizeOfCode=0x000000228a57b300) Line 13499
	at C:\git\rfp2\src\coreclr\vm\jitinterface.cpp(13499)
coreclr.dll!MethodDesc::JitCompileCodeLocked(PrepareCodeConfig * pConfig=0x000000228a57bb70, COR_ILMETHOD_DECODER * pilHeader=0x000000228a57b410, ListLockEntryBase<NativeCodeVersion> * pEntry=0x000001c4aed17450, unsigned long * pSizeOfCode=0x000000228a57b300) Line 927
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(927)
coreclr.dll!MethodDesc::JitCompileCodeLockedEventWrapper(PrepareCodeConfig * pConfig=0x000000228a57bb70, ListLockEntryBase<NativeCodeVersion> * pEntry=0x000001c4aed17450) Line 851
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(851)
coreclr.dll!MethodDesc::JitCompileCode(PrepareCodeConfig * pConfig=0x000000228a57bb70) Line 715
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(715)
coreclr.dll!MethodDesc::PrepareILBasedCode(PrepareCodeConfig * pConfig=0x000000228a57bb70) Line 431
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(431)
coreclr.dll!MethodDesc::PrepareCode(PrepareCodeConfig * pConfig=0x000000228a57bb70) Line 319
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(319)
coreclr.dll!CodeVersionManager::PublishVersionableCodeIfNecessary(MethodDesc * pMethodDesc=0x00007ffdddd3cbb0, CallerGCMode callerGCMode=Coop, bool * doBackpatchRef=0x000000228a57bca0, bool * doFullBackpatchRef=0x000000228a57bfa8) Line 1738
	at C:\git\rfp2\src\coreclr\vm\codeversion.cpp(1738)
coreclr.dll!MethodDesc::DoPrestub(MethodTable * pDispatchingMT=0x0000000000000000, CallerGCMode callerGCMode=Coop) Line 2174
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(2174)
coreclr.dll!PreStubWorker(TransitionBlock * pTransitionBlock=0x000000000000019a, MethodDesc * pMD=0x00007ffdddd3cbb0) Line 1964
	at C:\git\rfp2\src\coreclr\vm\prestub.cpp(1964)
coreclr.dll!ThePreStub() Line 20
	at C:\git\rfp2\src\coreclr\vm\amd64\ThePreStubAMD64.asm(20)

[dotnet-policy-service]

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

[AaronRobinsonMSFT]

Do we have any other cases of an intrinsic that defines its own signatures?

Not that I am aware of.

Is it OK to tell the jitter not to inline these intrinsics?

That would be less than ideal. I could be missing something here, but my assumption is that the generated code and signatures exists only during the JIT and then can be freed. The transient function system assumes that the functions are small enough that we can just regenerate them on demand. Since R2R is inovlved, perhaps there might be something we need to add to R2R?

the scope\context containing that function pointer signature is deleted.

Why is this needed after jitting?

[steveharter]

After a further look, this needs to be fixed.

The callstack shared earlier with TransientMethodDetails::~TransientMethodDetails() is a few frames after MethodDesc::GenerateFunctionPointerCall() returns (which creates the stub intrinsic).

[AaronRobinsonMSFT]

I also think that fILIntrinsic might need to be set to true.