Fix GC hole with collectible assemblies and tailcalls (#51315)

We must take special care to keep the tailcall dispatcher targets alive
while tailcalls are in-flight. In particular, given the following
callstack:

[B]M2()
[SPC]DispatchTailCalls
[A]M()

it could happen that [B]M2() queued a tail call to a function [B]M3().
Since there is a live dispatcher on the call stack, this would result in
[B]M2() storing a function pointer pointing to [B]M3() and returning to
this dispatcher to let it take care of the tailcall.

If B was loaded in a collectible ALC, it would then be possible for
there to be nothing keeping this ALC alive, and for the assembly to be
unloaded before the dispatcher invoked the function pointer.

I was unable to come up with a test case where this happened without
making changes to the dispatcher; the window otherwise seems to be too
small. To reproduce the problem I thus had to add a Thread.Sleep(50)
into the dispatcher, which quickly resulted in an
AccessViolationException in the scenario above. With the changes in this
commit I was then no logner able to reproduce the problem.

Fix #41314

Author: jakobbotsch

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

@@ -316,7 +316,7 @@ public static IntPtr AllocateTypeAssociatedMemory(Type type, int size)
         [StackTraceHidden]
         private static unsafe void DispatchTailCalls(
             IntPtr callersRetAddrSlot,
-            delegate*<IntPtr, IntPtr, IntPtr*, void> callTarget,
+            delegate*<IntPtr, IntPtr, PortableTailCallFrame*, void> callTarget,
             IntPtr retVal)
         {
             IntPtr callersRetAddr;
@@ -330,15 +330,17 @@ private static unsafe void DispatchTailCalls(
 
             PortableTailCallFrame newFrame;
             newFrame.Prev = prevFrame;
+            // GC uses NextCall to keep LoaderAllocator alive after we link it below,
+            // so we must null it out before that.
+            newFrame.NextCall = null;
 
             try
             {
                 tls->Frame = &newFrame;
 
                 do
                 {
-                    newFrame.NextCall = null;
-                    callTarget(tls->ArgBuffer, retVal, &newFrame.TailCallAwareReturnAddress);
+                    callTarget(tls->ArgBuffer, retVal, &newFrame);
                     callTarget = newFrame.NextCall;
                 } while (callTarget != null);
             }
@@ -501,7 +503,7 @@ internal unsafe struct PortableTailCallFrame
     {
         public PortableTailCallFrame* Prev;
         public IntPtr TailCallAwareReturnAddress;
-        public delegate*<IntPtr, IntPtr, IntPtr*, void> NextCall;
+        public delegate*<IntPtr, IntPtr, PortableTailCallFrame*, void> NextCall;
     }
 
     [StructLayout(LayoutKind.Sequential)]

src/coreclr/vm/gcenv.ee.cpp

@@ -157,7 +157,22 @@ static void ScanStackRoots(Thread * pThread, promote_func* fn, ScanContext* sc)
 
 static void ScanTailCallArgBufferRoots(Thread* pThread, promote_func* fn, ScanContext* sc)
 {
-    TailCallArgBuffer* argBuffer = pThread->GetTailCallTls()->GetArgBuffer();
+    TailCallTls* tls = pThread->GetTailCallTls();
+    // Keep loader associated with CallTailCallTarget alive.
+    if (sc->promotion)
+    {
+#ifndef DACCESS_COMPILE
+        const PortableTailCallFrame* frame = tls->GetFrame();
+        if (frame->NextCall != NULL)
+        {
+            MethodDesc* pMD = Entry2MethodDesc((PCODE)frame->NextCall, NULL);
+            if (pMD != NULL)
+                GcReportLoaderAllocator(fn, sc, pMD->GetLoaderAllocator());
+        }
+#endif
+    }
+
+    TailCallArgBuffer* argBuffer = tls->GetArgBuffer();
     if (argBuffer == NULL || argBuffer->GCDesc == NULL)
         return;
 

src/coreclr/vm/tailcallhelp.cpp

@@ -465,10 +465,10 @@ MethodDesc* TailCallHelp::CreateCallTargetStub(const TailCallInfo& info)
 
     ILCodeStream* pCode = sl.NewCodeStream(ILStubLinker::kDispatch);
 
-    // void CallTarget(void* argBuffer, void* retVal, void** pTailCallAwareRetAddress)
+    // void CallTarget(void* argBuffer, void* retVal, PortableTailCallFrame* pFrame)
     const int ARG_ARG_BUFFER = 0;
     const int ARG_RET_VAL = 1;
-    const int ARG_PTR_TAILCALL_AWARE_RET_ADDR = 2;
+    const int ARG_PTR_FRAME = 2;
 
     auto emitOffs = [&](UINT offs)
     {
@@ -477,10 +477,16 @@ MethodDesc* TailCallHelp::CreateCallTargetStub(const TailCallInfo& info)
         pCode->EmitADD();
     };
 
-    // *pTailCallAwareRetAddr = NextCallReturnAddress();
-    pCode->EmitLDARG(ARG_PTR_TAILCALL_AWARE_RET_ADDR);
+    // pFrame->NextCall = 0;
+    pCode->EmitLDARG(ARG_PTR_FRAME);
+    pCode->EmitLDC(0);
+    pCode->EmitCONV_U();
+    pCode->EmitSTFLD(FIELD__PORTABLE_TAIL_CALL_FRAME__NEXT_CALL);
+
+    // pFrame->TailCallAwareReturnAddress = NextCallReturnAddress();
+    pCode->EmitLDARG(ARG_PTR_FRAME);
     pCode->EmitCALL(METHOD__STUBHELPERS__NEXT_CALL_RETURN_ADDRESS, 0, 1);
-    pCode->EmitSTIND_I();
+    pCode->EmitSTFLD(FIELD__PORTABLE_TAIL_CALL_FRAME__TAILCALL_AWARE_RETURN_ADDRESS);
 
     for (COUNT_T i = 0; i < info.ArgBufLayout.Values.GetCount(); i++)
     {
@@ -613,7 +619,7 @@ void TailCallHelp::CreateCallTargetStubSig(const TailCallInfo& info, SigBuilder*
     // Return value
     sig->AppendElementType(ELEMENT_TYPE_I);
 
-    // Pointer to tail call aware return address
+    // Pointer to tail call frame
     sig->AppendElementType(ELEMENT_TYPE_I);
 
 #ifdef _DEBUG