[Copilot]

Thanks for the feedback on #135794. I've created this new PR, which merges into #135794, to address your comment. I will work on the changes and keep this PR's description up to date as I make progress.

Original PR: #135794
Triggering comment (#135794 (comment)):

"C:\h\w\A782094F\p\watchdog.exe" 299 "C:\h\w\A782094F\p\corerun.exe" -p "System.Reflection.Metadata.MetadataUpdater.IsSupported=false" -p "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization=true"  threading_group1.dll 

Assert failure(PID 3520 [0x00000dc0], Thread: 6500 [0x1964]): !th.IsNull()

CORECLR! SigPointer::PeekElemTypeNormalized + 0x15E (0x73a6bece)
CORECLR! MetaSig::GetReturnTypeNormalized + 0x32 (0x73a658a2)
CORECLR! MethodDesc::GetReturnKind + 0x20B (0x739e781b)
CORECLR! ReplaceInstrAfterCall + 0x10 (0x73b9fc80)
CORECLR! DoGcStress + 0x4CD (0x73b9f3bd)
CORECLR! OnGcCoverageInterrupt + 0x213 (0x73b9fa93)
CORECLR! CLRVectoredExceptionHandlerShim + 0x159 (0x73918289)
NTDLL! WinSqmEventWrite + 0x1B826 (0x779af4d6)
NTDLL! RtlUnwind + 0x1BA (0x7796ec4a)
NTDLL! KiUserExceptionDispatcher + 0xF (0x7798132f)
    File: D:\a\_work\1\s\src\coreclr\vm\siginfo.cpp:1231
    Image: C:\h\w\A782094F\p\corerun.exe

Exit Code: -1073740286
Expected: 100
Actual: -1073740286
END EXECUTION - FAILED
FAILED

Is it caused by the PR? The PR exposes it, but the root cause is already in main.

• Trigger (PR): GC stress now instruments R2R code. At an R2R call site, DoGcStress → ReplaceInstrAfterCall → x86-only MethodDesc::GetReturnKind() has to read the callee's return type. It runs under ENABLE_FORBID_GC_LOADER_USE and may not load types, so it asserts !th.IsNull() (siginfo.cpp:1231) if a value-type return isn't loaded yet.
• Root cause (main): Prepare ReadyToRun call signatures before publishing targets #135805, merged Oct 28, stopped R2R callers from force-loading callee value-type signatures. A call target that has a GCRefMap may skip loading them. Fix GC-time type loads in ReadyToRun transition frames after #135805 #136667 fixed only the GC-time paths that hit this on the stack-walk side.
• Baseline hits it too. The baseline [TEST ONLY] noop change #136001 failed DevDiv_461649 on x86 with the same assert, through VSD ResolveWorker → SizeOfArgStack. That is open issue Assert failure: !th.IsNull() #136674 and is unrelated to GC stress.
• Likely link to the x86 crashes: I suspect this is behind the many x86 gcstress0xc_* work-item crashes in the PR run, but I couldn't verify that.

Fix for the PR: in x86 MethodDesc::GetReturnKind, return RT_Illegal when the return value type isn't loaded. ReplaceInstrAfterCall already skips the post-call instruction for that value, so the only loss is coverage after that one call. The patch applies cleanly to PR head 1798c9d.

--- a/src/coreclr/vm/method.cpp
+++ b/src/coreclr/vm/method.cpp
@@ -1572,6 +1572,24 @@
     TypeHandle thValueType;
 
     MetaSig sig(this);
+
+    // Callers are not required to load the callee's value type return layout before the call
+    // (ReadyToRun call targets with a GCRefMap skip it) and we cannot load types here.
+    // Report an unknown return kind in that case.
+    if (sig.GetReturnType() == ELEMENT_TYPE_VALUETYPE)
+    {
+        TypeHandle thRet = sig.GetReturnProps().GetTypeHandleThrowing(
+            sig.GetModule(),
+            sig.GetSigTypeContext(),
+            ClassLoader::DontLoadTypes,
+            CLASS_LOAD_APPROXPARENTS,
+            TRUE /* dropGenericArgumentLevel */);
+        if (thRet.IsNull())
+        {
+            return RT_Illegal;
+        }
+    }
+
     CorElementType et = sig.GetReturnTypeNormalized(&thValueType);

This mirrors the non-loading check that PeekElemTypeNormalized already does in debug builds.

Not covered by this fix: the ResolveWorker → SizeOfArgStack path (#136674) needs its own fix in main. Preparing the target's signature before SizeOfArgStack is one option, but that is a guess I haven't checked.

@copilot fix the assert.

[azure-pipelines]

Azure Pipelines:
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[VSadov]

@copilot why is this a separate PR and not a change in the original PR?

[dotnet-policy-service]

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

[VSadov]

stray PR opened by copilot