From 0dc387bd3c2a0ce7e924eaa7c54d68d044e7910a Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 7 May 2026 20:30:50 +0300 Subject: [PATCH] Move the return var out of liveLocalsIntervals When generating an async call, in EmitSuspend, we obtain the set of vars that are alive at this point in time and we store this information inside the suspenData. If they async call needs to suspend it will store these vars into the ContinuationObject, with their values being restored when we resume the continuation. Previous code would store the result value as well into the ContinuationObject. The problem is that this would lead to storing uninitialized data into the object, which can result in random GC crashes. This commit removes the result var from the set of live vars, stores associated information specifically for the return var inside the suspend data and we make use of this information to write the result only on the suspend resume path. --- src/coreclr/interpreter/compiler.cpp | 28 ++++++++++++++++++- .../interpreter/inc/interpretershared.h | 2 ++ src/coreclr/vm/interpexec.cpp | 17 +++++++++-- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 6f10b84a6ea6f2..a465525bf5bb3e 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -5789,8 +5789,8 @@ void InterpCompiler::EmitSuspend(const CORINFO_CALL_INFO &callInfo, Continuation int32_t returnValueVar = -1; if (stackDepth > 0 && callInfo.sig.retType != CORINFO_TYPE_VOID) { + // The return value var is written explicitly during resume, it is not included in the set of liveVars returnValueVar = m_pStackPointer[-1].var; - liveVars.Add(returnValueVar); } // Step 1: Collect live IL vars @@ -6015,6 +6015,21 @@ void InterpCompiler::EmitSuspend(const CORINFO_CALL_INFO &callInfo, Continuation suspendData->resumeInfo.DiagnosticIP = (size_t)NULL; suspendData->methodStartIP = 0; // This is filled in by logic later in emission once we know the final address of the method suspendData->continuationArgOffset = m_pVars[m_continuationArgIndex].offset; + + if (returnValueVar != -1) + { + int32_t alignDummy; + int32_t size = GetInterpTypeStackSize(m_pVars[returnValueVar].clsHnd, m_pVars[returnValueVar].interpType, &alignDummy); + suspendData->returnValueContinuationDataSize = ALIGN_UP_TO(size, INTERP_STACK_SLOT_SIZE); + } + else + { + suspendData->returnValueContinuationDataSize = 0; + } + + // Patched up in UpdateLocalIntervalMaps to hold the actual offset of the var + suspendData->returnValueVarStackOffset = returnValueVar; + suspendData->asyncMethodReturnType = NULL; switch (m_methodInfo->args.retType) { @@ -6323,6 +6338,17 @@ void InterpCompiler::UpdateLocalIntervalMaps() { ConvertToIntervalMapData_ForOffsets(m_varIntervalMaps.Get(i)); } + + // Fix up return value var stack offsets for async suspend data + for (int32_t i = 0; i < m_asyncSuspendDataItems.GetSize(); i++) + { + InterpAsyncSuspendData* suspendData = m_asyncSuspendDataItems.Get(i); + int32_t varIndex = suspendData->returnValueVarStackOffset; + if (varIndex != -1) + { + suspendData->returnValueVarStackOffset = m_pVars[varIndex].offset; + } + } } static int32_t GetStindForType(InterpType interpType) diff --git a/src/coreclr/interpreter/inc/interpretershared.h b/src/coreclr/interpreter/inc/interpretershared.h index c696140b97b073..299c769b9735c3 100644 --- a/src/coreclr/interpreter/inc/interpretershared.h +++ b/src/coreclr/interpreter/inc/interpretershared.h @@ -214,6 +214,8 @@ struct InterpAsyncSuspendData COMPILER_SHARED_TYPE(CORINFO_CLASS_HANDLE, DPTR(MethodTable), asyncMethodReturnType); int32_t asyncMethodReturnTypePrimitiveSize; // 0 if not primitive, otherwise size in bytes int32_t continuationArgOffset; + int32_t returnValueContinuationDataSize; // Aligned size of the return value in continuation data (0 if void). Live locals start after this offset. + int32_t returnValueVarStackOffset; // Interpreter stack offset of the return value var (valid only when returnValueContinuationDataSize > 0) COMPILER_SHARED_TYPE(CORINFO_METHOD_HANDLE, DPTR(MethodDesc), captureSyncContextMethod); COMPILER_SHARED_TYPE(CORINFO_METHOD_HANDLE, DPTR(MethodDesc), restoreExecutionContextMethod); diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index e5d99b4c501b04..fabdb34336df9f 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -4491,9 +4491,11 @@ do \ } ip += 3; - // copy locals that need to move to the continuation object + // Copy locals that need to move to the continuation object + // The copied continuation data begins immediately after the + // continuation's result storage. size_t continuationOffset = OFFSETOF__CORINFO_Continuation__data; - uint8_t *pContinuationDataStart = continuation->GetResultStorage(); + uint8_t *pContinuationDataStart = continuation->GetResultStorage() + pAsyncSuspendData->returnValueContinuationDataSize; uint8_t *pContinuationData = pContinuationDataStart; size_t bytesTotal = 0; InterpIntervalMapEntry *pCopyEntry = pAsyncSuspendData->liveLocalsIntervals; @@ -4561,7 +4563,7 @@ do \ // Now copy the locals // copy locals that need to move from the continuation object - uint8_t *pContinuationData = continuation->GetResultStorage(); + uint8_t *pContinuationData = continuation->GetResultStorage() + pAsyncSuspendData->returnValueContinuationDataSize; InterpIntervalMapEntry *pCopyEntry = pAsyncSuspendData->liveLocalsIntervals; while (pCopyEntry->countBytes != 0) { @@ -4570,6 +4572,15 @@ do \ pCopyEntry++; } + // Explicitly copy the return value from the continuation's result storage + // to the interpreter stack. + if (pAsyncSuspendData->returnValueContinuationDataSize > 0) + { + memcpy(LOCAL_VAR_ADDR(pAsyncSuspendData->returnValueVarStackOffset, uint8_t), + continuation->GetResultStorage(), + pAsyncSuspendData->returnValueContinuationDataSize); + } + PTR_OBJECTREF pException = continuation->GetExceptionObjectStorageOrNull(); if (pException != NULL) {