From 31d59eb6f38de09f25fc61e3b1a7cdd2f921086f Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 7 Mar 2026 10:32:34 -0500 Subject: [PATCH 01/29] Implement async method variant handling in AddMethod and UpdateMethod --- src/coreclr/vm/class.cpp | 139 +++++++++++++++++++++++++++++++++++---- src/coreclr/vm/class.h | 4 ++ src/coreclr/vm/encee.cpp | 12 ++++ 3 files changed, 143 insertions(+), 12 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index a95ffe073329dc..e41777c36599ae 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -560,20 +560,135 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } #endif // _DEBUG + // Classify the method's return type to determine if it needs async variant handling. + // For runtime-async methods (IsMiAsync) that return Task/ValueTask, we need to create + // two MethodDescs: the task-returning variant and the async call variant. + ULONG sigLen; + PCCOR_SIGNATURE pMemberSignature; + if (FAILED(pImport->GetSigOfMethodDef(methodDef, &sigLen, &pMemberSignature))) + return COR_E_BADIMAGEFORMAT; + + ULONG offsetOfAsyncDetails = 0; + bool returnsValueTask = false; + MethodReturnKind returnKind; + { + // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which + // does type resolution that may trigger GC. We're in a GC_NOTRIGGER scope because + // the CLR is suspended for debugging, but this resolution is safe since we're only + // resolving well-known system types (Task/ValueTask). This mirrors the existing + // CONTRACT_VIOLATION(GCViolation) in ApplyEditAndContinue. + CONTRACT_VIOLATION(GCViolation); + returnKind = ClassifyMethodReturnKind( + SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); + } + + bool isAsyncTaskReturning = IsMiAsync(dwImplFlags) && IsTaskReturning(returnKind); + + // Create the primary MethodDesc (task-returning variant for async, or the only variant for non-async) + AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; + if (isAsyncTaskReturning) + { + // For IsMiAsync + task-returning: the task-returning variant is a thunk, + // the real IL body belongs to the async variant. + primaryAsyncFlags = AsyncMethodFlags::ReturnsTaskOrValueTask | AsyncMethodFlags::Thunk; + } + else if (IsMiAsync(dwImplFlags)) + { + // IsMiAsync but not task-returning: infrastructure async method (e.g. Await helpers) + primaryAsyncFlags = AsyncMethodFlags::AsyncCall; + } + MethodDesc* pNewMD; - if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, &pNewMD))) + if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, + primaryAsyncFlags, NULL, 0, &pNewMD))) { LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed: 0x%08x\n", hr)); return hr; } - // Store the new MethodDesc into the collection for this class + // Store the task-returning (or only) variant in the module's method lookup. + // The async variant is found via IntroducedMethodIterator, not stored here. pModule->EnsureMethodDefCanBeStored(methodDef); pModule->EnsuredStoreMethodDef(methodDef, pNewMD); LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added pMD:%p for token 0x%08x\n", pNewMD, methodDef)); + // For runtime-async task-returning methods, create the async call variant. + // This mirrors what MethodTableBuilder does in the second pass of its method enumeration loop. + if (isAsyncTaskReturning) + { + AsyncMethodFlags asyncFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; + if (returnsValueTask) + asyncFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; + + // For IsMiAsync methods, the async variant has the real IL body (not a thunk). + // The task-returning variant above is the thunk. + + // Construct the async variant signature by stripping the Task/ValueTask wrapper + // from the return type. This follows the same logic as MethodTableBuilder. + ULONG cAsyncSig; + ULONG taskTokenOffsetFromAsyncDetailsOffset; + ULONG taskTypePrefixSize; + ULONG taskTypePrefixReplacementSize; + ULONG tokenLen = 0; + + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + { + // "... Task ... Method(args);" → "... void ... Method(args);" + taskTokenOffsetFromAsyncDetailsOffset = 1; + tokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE + taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID + cAsyncSig = sigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; + } + else if (returnKind == MethodReturnKind::GenericTaskReturningMethod) + { + // "... Task ... Method(args);" → "... tk ... Method(args);" + taskTokenOffsetFromAsyncDetailsOffset = 2; + tokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + taskTypePrefixReplacementSize = 0; + cAsyncSig = sigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; + } + else + { + _ASSERTE(!"Unexpected returnKind for async task-returning method"); + return E_UNEXPECTED; + } + + LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); + BYTE* pNewSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncSig)); + + ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; + ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; + + // Copy bytes before the async prefix + memcpy(pNewSig, pMemberSignature, offsetOfAsyncDetails); + + // Copy bytes after the async prefix + _ASSERTE((sigLen - originalRemainingSigOffset) == (cAsyncSig - newRemainingSigOffset)); + memcpy(pNewSig + newRemainingSigOffset, + pMemberSignature + originalRemainingSigOffset, + sigLen - originalRemainingSigOffset); + + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + { + pNewSig[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; + } + + MethodDesc* pAsyncVariantMD; + if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, + asyncFlags, pNewSig, cAsyncSig, &pAsyncVariantMD))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant: 0x%08x\n", hr)); + return hr; + } + + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added async variant pMD:%p for token 0x%08x\n", + pAsyncVariantMD, methodDef)); + } + // If the type is generic, then we need to update all existing instantiated types if (pMT->IsGenericTypeDefinition()) { @@ -607,7 +722,8 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } MethodDesc* pNewMDUnused; - if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, &pNewMDUnused))) + if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, + primaryAsyncFlags, NULL, 0, &pNewMDUnused))) { LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed: 0x%08x\n", hr)); EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, @@ -634,6 +750,9 @@ HRESULT EEClass::AddMethodDesc( mdMethodDef methodDef, DWORD dwImplFlags, DWORD dwMemberAttrs, + AsyncMethodFlags asyncFlags, + PCCOR_SIGNATURE pAsyncSig, + DWORD cbAsyncSig, MethodDesc** ppNewMD) { CONTRACTL @@ -660,17 +779,13 @@ HRESULT EEClass::AddMethodDesc( if (FAILED(hr = pImport->GetSigOfMethodDef(methodDef, &sigLen, &sig))) return hr; - if (IsMiAsync(dwImplFlags)) - { - LOG((LF_ENC, LL_INFO100, "**Error** EnC for Async methods is NYI")); - return E_FAIL; - } - uint32_t callConv = CorSigUncompressData(sig); DWORD classification = (callConv & IMAGE_CEE_CS_CALLCONV_GENERIC) ? mcInstantiated : mcIL; + bool hasAsyncData = (asyncFlags != AsyncMethodFlags::None); + LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); // [TODO] OOM: InitMethodDesc will allocate loaderheap memory but leak it @@ -684,7 +799,7 @@ HRESULT EEClass::AddMethodDesc( classification, TRUE, // fNonVtableSlot TRUE, // fNativeCodeSlot - FALSE, /* HasAsyncMethodData */ + hasAsyncData, /* HasAsyncMethodData */ pMT, &dummyAmTracker); @@ -733,8 +848,8 @@ HRESULT EEClass::AddMethodDesc( 0, // RVA - non-zero only for PInvoke pImport, NULL, - Signature(), - AsyncMethodFlags::None + Signature(pAsyncSig, cbAsyncSig), + asyncFlags COMMA_INDEBUG(debug_szMethodName) COMMA_INDEBUG(pMT->GetDebugClassName()) COMMA_INDEBUG(NULL) diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 13d1ee37a8d3cd..bc5f2df4753b5a 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -78,6 +78,7 @@ class MethodTable; class Module; class Object; class Stub; +enum class AsyncMethodFlags; class Substitution; class SystemDomain; class TypeHandle; @@ -792,6 +793,9 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! mdMethodDef methodDef, DWORD dwImplFlags, DWORD dwMemberAttrs, + AsyncMethodFlags asyncFlags, + PCCOR_SIGNATURE pAsyncSig, + DWORD cbAsyncSig, MethodDesc** ppNewMD); public: // Add a new field to an already loaded type for EnC diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index 025df8ac2aa77f..b4b557e3639868 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -370,6 +370,18 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) { // Not a method impacted by generics, so this is the MethodDesc to use. pMethod->ResetCodeEntryPointForEnC(); + + // For runtime-async methods, also reset the async variant's entry point. + // The task-returning variant is stored in the module's method lookup, but + // its async counterpart also needs to be re-JITted with the new IL. + if (pMethod->HasAsyncOtherVariant()) + { + MethodDesc* pAsyncOther = pMethod->GetAsyncOtherVariantNoCreate(); + if (pAsyncOther != NULL) + { + pAsyncOther->ResetCodeEntryPointForEnC(); + } + } } else { From 0e5fc4d0c95736807ac1433e06e2e2642af2d6be Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 14 Mar 2026 21:29:52 -0400 Subject: [PATCH 02/29] Use async variant in EditAndContinueModule::UpdateMethod Use GetAsyncOtherVariantNoCreate in UpdateMethod --- src/coreclr/debug/ee/debugger.cpp | 41 +++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index ffd41afb34570a..94afb1cb3e59a7 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -12909,6 +12909,47 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) pJitInfo->m_encBreakpointsApplied = true; + // For runtime-async methods, also plant EnC remap breakpoints on the async variant. + // The JIT creates two MethodDescs: an entry variant (runs pre-await code) and an + // async variant (runs on resumption after await). Each has its own JIT info with + // different sequence points. Without planting breakpoints on both, remap during + // async resumption won't fire because the breakpoints only cover the entry variant. + if (pMD->HasAsyncOtherVariant()) + { + MethodDesc* pAsyncOther = pMD->GetAsyncOtherVariantNoCreate(); + if (pAsyncOther != NULL) + { + DebuggerJitInfo *pAsyncJitInfo = GetLatestJitInfoFromMethodDesc(pAsyncOther); + if (pAsyncJitInfo != NULL && !pAsyncJitInfo->m_encBreakpointsApplied) + { + LOG((LF_CORDB, LL_INFO10000, "D::UF: Also applying breakpoints to async variant\n")); + + EnCSequencePointHelper asyncSeqHelper(pAsyncJitInfo); + PTR_DebuggerILToNativeMap asyncSeqMap = pAsyncJitInfo->GetSequenceMap(); + for (unsigned int j = 0; j < pAsyncJitInfo->GetSequenceMapCount(); j++) + { + if (!asyncSeqHelper.ShouldSetRemapBreakpoint(j)) + { + continue; + } + + SIZE_T asyncOffset = asyncSeqMap[j].ilOffset; + LOG((LF_CORDB, LL_INFO10000, "D::UF: placing async variant EnC breakpoint at offset 0x%x (IL: 0x%x)\n", + asyncSeqMap[j].nativeStartOffset, asyncSeqMap[j].ilOffset)); + + DebuggerEnCBreakpoint *asyncBp; + asyncBp = new (interopsafe) DebuggerEnCBreakpoint(asyncOffset, + pAsyncJitInfo, + DebuggerEnCBreakpoint::REMAP_PENDING, + AppDomain::GetCurrentDomain()); + _ASSERTE(asyncBp != NULL); + } + + pAsyncJitInfo->m_encBreakpointsApplied = true; + } + } + } + return S_OK; } From 7c114b4c18a1e3bab496613b19988e1a6f7c67dc Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Fri, 20 Mar 2026 14:26:13 -0400 Subject: [PATCH 03/29] Address PR feedback Extract BuildAsyncVariantSignature to share async signature construction The async variant signature construction logic (stripping Task/ValueTask wrapper from the return type) was duplicated between MethodTableBuilder (initial type load) and EEClass::AddMethod (EnC add method). Extract it into a shared BuildAsyncVariantSignature helper in method.cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 52 +++--------------------- src/coreclr/vm/method.cpp | 58 +++++++++++++++++++++++++++ src/coreclr/vm/method.hpp | 14 +++++++ src/coreclr/vm/methodtablebuilder.cpp | 5 --- 4 files changed, 77 insertions(+), 52 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index e41777c36599ae..fa629bd05851bf 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -625,57 +625,15 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // For IsMiAsync methods, the async variant has the real IL body (not a thunk). // The task-returning variant above is the thunk. - // Construct the async variant signature by stripping the Task/ValueTask wrapper - // from the return type. This follows the same logic as MethodTableBuilder. + // Construct the async variant signature by stripping the Task/ValueTask wrapper. ULONG cAsyncSig; - ULONG taskTokenOffsetFromAsyncDetailsOffset; - ULONG taskTypePrefixSize; - ULONG taskTypePrefixReplacementSize; - ULONG tokenLen = 0; - - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) - { - // "... Task ... Method(args);" → "... void ... Method(args);" - taskTokenOffsetFromAsyncDetailsOffset = 1; - tokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE - taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID - cAsyncSig = sigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; - } - else if (returnKind == MethodReturnKind::GenericTaskReturningMethod) - { - // "... Task ... Method(args);" → "... tk ... Method(args);" - taskTokenOffsetFromAsyncDetailsOffset = 2; - tokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 - taskTypePrefixReplacementSize = 0; - cAsyncSig = sigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; - } - else - { - _ASSERTE(!"Unexpected returnKind for async task-returning method"); - return E_UNEXPECTED; - } + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + nullptr, &cAsyncSig); LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); BYTE* pNewSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncSig)); - - ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; - ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; - - // Copy bytes before the async prefix - memcpy(pNewSig, pMemberSignature, offsetOfAsyncDetails); - - // Copy bytes after the async prefix - _ASSERTE((sigLen - originalRemainingSigOffset) == (cAsyncSig - newRemainingSigOffset)); - memcpy(pNewSig + newRemainingSigOffset, - pMemberSignature + originalRemainingSigOffset, - sigLen - originalRemainingSigOffset); - - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) - { - pNewSig[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; - } + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + pNewSig, &cAsyncSig); MethodDesc* pAsyncVariantMD; if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 6e4772c539db5c..e55abf811656b7 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2458,6 +2458,64 @@ MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG return MethodReturnKind::NormalMethod; } +void BuildAsyncVariantSignature( + MethodReturnKind returnKind, + const BYTE* pOrigSig, + ULONG origSigLen, + ULONG offsetOfAsyncDetails, + BYTE* pDestSig, + ULONG* pAsyncSigLen) +{ + _ASSERTE(returnKind == MethodReturnKind::NonGenericTaskReturningMethod || + returnKind == MethodReturnKind::GenericTaskReturningMethod); + _ASSERTE(pAsyncSigLen != nullptr); + + ULONG taskTokenOffsetFromAsyncDetailsOffset; + ULONG taskTypePrefixSize; + ULONG taskTypePrefixReplacementSize; + + ULONG tokenLen = CorSigUncompressedDataSize( + &pOrigSig[offsetOfAsyncDetails + + (returnKind == MethodReturnKind::NonGenericTaskReturningMethod ? 1 : 2)]); + + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + { + // "... Task ... Method(args);" → "... void ... Method(args);" + taskTokenOffsetFromAsyncDetailsOffset = 1; + taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE + taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID + } + else + { + // "... Task ... Method(args);" → "... T ... Method(args);" + taskTokenOffsetFromAsyncDetailsOffset = 2; + taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + taskTypePrefixReplacementSize = 0; + } + + *pAsyncSigLen = origSigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; + + if (pDestSig != nullptr) + { + ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; + ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; + + // Copy bytes before the async prefix + memcpy(pDestSig, pOrigSig, offsetOfAsyncDetails); + + // Copy bytes after the async prefix + _ASSERTE((origSigLen - originalRemainingSigOffset) == (*pAsyncSigLen - newRemainingSigOffset)); + memcpy(pDestSig + newRemainingSigOffset, + pOrigSig + originalRemainingSigOffset, + origSigLen - originalRemainingSigOffset); + + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + { + pDestSig[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; + } + } +} + //******************************************************************************* BOOL MethodDesc::ShouldCallPrestub() { diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 9188d081cd5ee4..c985bf2739f71d 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -281,6 +281,20 @@ enum class MethodReturnKind bool IsTypeDefOrRefImplementedInSystemModule(Module* pModule, mdToken tk); MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG* offsetOfAsyncDetails, ULONG* elementTypeLength, bool *isValueTask); +// Given a task-returning method signature, compute the async variant signature +// by stripping the Task/ValueTask wrapper from the return type. +// For NonGenericTaskReturningMethod: "... Task ... Method(args)" → "... void ... Method(args)" +// For GenericTaskReturningMethod: "... Task ... Method(args)" → "... T ... Method(args)" +// If pDestSig is non-null, the new signature is written there (caller must allocate at least *pAsyncSigLen bytes). +// *pAsyncSigLen is always set to the required output size. +void BuildAsyncVariantSignature( + MethodReturnKind returnKind, + const BYTE* pOrigSig, + ULONG origSigLen, + ULONG offsetOfAsyncDetails, + BYTE* pDestSig, + ULONG* pAsyncSigLen); + inline bool IsTaskReturning(MethodReturnKind input) { return (input == MethodReturnKind::GenericTaskReturningMethod) || diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 038fa7a8d61417..788c42a6fedce2 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3397,11 +3397,6 @@ MethodTableBuilder::EnumerateClassMethods() { // Extra pass, add an async variant. - ULONG cAsyncThunkMemberSignature; - ULONG taskTokenOffsetFromAsyncDetailsOffset; - ULONG taskTypePrefixSize; - ULONG taskTypePrefixReplacementSize; - AsyncMethodFlags asyncFlags = (AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant); if (returnsValueTask) { From 40bd1052061c170da46af803cb38f60decb0f497 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 2 Apr 2026 19:49:47 -0400 Subject: [PATCH 04/29] EnC: Create async variant for generic instantiations in AddMethod When adding a new async method to a generic type via EnC, the async variant MethodDesc was only created for the type definition but not for existing canonical instantiations. This caused crashes when the newly added async method was called on value type instantiations (e.g. G) because the thunk could not find its async variant. Hoist the async variant signature and flags computation above the generic instantiation loop, then create the async variant for each canonical MethodTable alongside the primary thunk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 46 +++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index fa629bd05851bf..2e51b60e48d4ae 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -614,30 +614,32 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added pMD:%p for token 0x%08x\n", pNewMD, methodDef)); - // For runtime-async task-returning methods, create the async call variant. - // This mirrors what MethodTableBuilder does in the second pass of its method enumeration loop. + // For runtime-async task-returning methods, compute the async variant signature and flags. + // These are used for both the type definition and any existing generic instantiations. + AsyncMethodFlags asyncVariantFlags = AsyncMethodFlags::None; + BYTE* pAsyncVariantSig = NULL; + ULONG cAsyncVariantSig = 0; + if (isAsyncTaskReturning) { - AsyncMethodFlags asyncFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; + asyncVariantFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; if (returnsValueTask) - asyncFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; - - // For IsMiAsync methods, the async variant has the real IL body (not a thunk). - // The task-returning variant above is the thunk. + asyncVariantFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; // Construct the async variant signature by stripping the Task/ValueTask wrapper. - ULONG cAsyncSig; BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - nullptr, &cAsyncSig); + nullptr, &cAsyncVariantSig); LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); - BYTE* pNewSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncSig)); + pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - pNewSig, &cAsyncSig); + pAsyncVariantSig, &cAsyncVariantSig); + // Create the async variant for the type definition. + // For IsMiAsync methods, the async variant has the real IL body (not a thunk). MethodDesc* pAsyncVariantMD; if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, - asyncFlags, pNewSig, cAsyncSig, &pAsyncVariantMD))) + asyncVariantFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariantMD))) { LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant: 0x%08x\n", hr)); return hr; @@ -688,6 +690,26 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** W("Failed to add method to existing instantiated type instance")); return E_FAIL; } + + // For async task-returning methods, also create the async variant for + // this instantiation. Without this, the thunk on the instantiation's + // MethodTable cannot find its async variant, causing crashes when the + // newly added async method is called on value type instantiations. + if (isAsyncTaskReturning) + { + MethodDesc* pAsyncVariantMDUnused; + if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, + asyncVariantFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariantMDUnused))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant for instantiation: 0x%08x\n", hr)); + EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, + W("Failed to add async variant to existing instantiated type instance")); + return E_FAIL; + } + + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added async variant pMD:%p for instantiation pMT:%p\n", + pAsyncVariantMDUnused, pMTMaybe)); + } } } } From 750bab8d00b7661c0d839d1162855cd28142c71a Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 2 Apr 2026 20:48:29 -0400 Subject: [PATCH 05/29] Address PR #125397 feedback: GC comment, infrastructure async rejection, generic UpdateMethod comment - class.cpp:579: Update GC violation comment to acknowledge the extern-alias edge case as Won't Fix per reviewer feedback - class.cpp:597: Reject infrastructure async methods (IsMiAsync but not task-returning) on non-system modules with COR_E_BADIMAGEFORMAT, mirroring MethodTableBuilder validation - encee.cpp:388: Add comment explaining that ResetCodeEntryPointForEnC cascades from thunk to async variant automatically, so generic UpdateMethod path does not need explicit async variant handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 21 ++++++++++++++++----- src/coreclr/vm/encee.cpp | 3 +++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 2e51b60e48d4ae..06cc10cee1a938 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -573,10 +573,12 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** MethodReturnKind returnKind; { // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which - // does type resolution that may trigger GC. We're in a GC_NOTRIGGER scope because - // the CLR is suspended for debugging, but this resolution is safe since we're only - // resolving well-known system types (Task/ValueTask). This mirrors the existing - // CONTRACT_VIOLATION(GCViolation) in ApplyEditAndContinue. + // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because + // we're only resolving well-known system types (Task/ValueTask) in practice. + // Note: ClassifyMethodReturnKind checks type names without verifying the assembly + // reference, so user-defined types matching these names (e.g. via extern alias) + // could theoretically induce GC-triggering paths. Accepted as Won't Fix given + // the extreme unlikelihood and consistency with ApplyEditAndContinue. CONTRACT_VIOLATION(GCViolation); returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); @@ -594,7 +596,16 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } else if (IsMiAsync(dwImplFlags)) { - // IsMiAsync but not task-returning: infrastructure async method (e.g. Await helpers) + // IsMiAsync but not task-returning: infrastructure async method (e.g. Await helpers). + // These are only permitted on types in the system module, mirroring the validation + // in MethodTableBuilder during normal type loading. + if (!pModule->IsSystem()) + { + LOG((LF_ENC, LL_INFO100, + "EEClass::AddMethod rejecting infrastructure async method (methodDef: 0x%08x) on non-system module\n", + methodDef)); + return COR_E_BADIMAGEFORMAT; + } primaryAsyncFlags = AsyncMethodFlags::AsyncCall; } diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index b4b557e3639868..bdecf6f91a125e 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -386,6 +386,9 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) else { // Generics are involved so we need to search for all related MethodDescs. + // Note: ResetCodeEntryPointForEnC() handles async variants automatically — + // when called on a thunk MethodDesc, it detects IsAsyncThunkMethod() and + // cascades the reset to the async variant (see method.cpp). Module* module = pMethod->GetLoaderModule(); mdMethodDef tkMethod = pMethod->GetMemberDef(); From e15eebd363bb542627374c18262fc0a387a14823 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 2 Apr 2026 21:55:47 -0400 Subject: [PATCH 06/29] Remove dead store taskTokenOffsetFromAsyncDetailsOffset in BuildAsyncVariantSignature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/method.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index e55abf811656b7..394971e6f5ac28 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2470,7 +2470,6 @@ void BuildAsyncVariantSignature( returnKind == MethodReturnKind::GenericTaskReturningMethod); _ASSERTE(pAsyncSigLen != nullptr); - ULONG taskTokenOffsetFromAsyncDetailsOffset; ULONG taskTypePrefixSize; ULONG taskTypePrefixReplacementSize; @@ -2481,14 +2480,12 @@ void BuildAsyncVariantSignature( if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) { // "... Task ... Method(args);" → "... void ... Method(args);" - taskTokenOffsetFromAsyncDetailsOffset = 1; taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID } else { // "... Task ... Method(args);" → "... T ... Method(args);" - taskTokenOffsetFromAsyncDetailsOffset = 2; taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 taskTypePrefixReplacementSize = 0; } From 4d317a6d395b37c9168ffc5eec5cc133423d72ba Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 4 Apr 2026 19:19:50 -0400 Subject: [PATCH 07/29] Address noahfalk feedback: clarify GC comment, reject all infrastructure async in EnC - Clarify GC violation comment to explain the specific unlikely scenario (naming collision + extern alias + hot reload while debugging) - Reject infrastructure async methods unconditionally in EnC rather than only on non-system modules, since no scenario benefits from allowing them Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 06cc10cee1a938..0bfed4439f4995 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -575,10 +575,11 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because // we're only resolving well-known system types (Task/ValueTask) in practice. - // Note: ClassifyMethodReturnKind checks type names without verifying the assembly - // reference, so user-defined types matching these names (e.g. via extern alias) - // could theoretically induce GC-triggering paths. Accepted as Won't Fix given - // the extreme unlikelihood and consistency with ApplyEditAndContinue. + // Note: ClassifyMethodReturnKind matches by type name without verifying the assembly + // reference, so a user-defined type named "Task" or "ValueTask" (e.g. via extern + // alias) could be misidentified and induce GC-triggering resolution paths. Accepted + // as Won't Fix given this requires an extremely unlikely combination of naming + // collision, extern alias usage, and hot reload of such a method while debugging. CONTRACT_VIOLATION(GCViolation); returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); @@ -597,16 +598,12 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** else if (IsMiAsync(dwImplFlags)) { // IsMiAsync but not task-returning: infrastructure async method (e.g. Await helpers). - // These are only permitted on types in the system module, mirroring the validation - // in MethodTableBuilder during normal type loading. - if (!pModule->IsSystem()) - { - LOG((LF_ENC, LL_INFO100, - "EEClass::AddMethod rejecting infrastructure async method (methodDef: 0x%08x) on non-system module\n", - methodDef)); - return COR_E_BADIMAGEFORMAT; - } - primaryAsyncFlags = AsyncMethodFlags::AsyncCall; + // There is no known scenario that benefits from adding these via EnC, so reject + // unconditionally rather than allowing them even on the system module. + LOG((LF_ENC, LL_INFO100, + "EEClass::AddMethod rejecting infrastructure async method (methodDef: 0x%08x)\n", + methodDef)); + return COR_E_BADIMAGEFORMAT; } MethodDesc* pNewMD; From d0d2855daa4eb7b743f14eb111b4bcf812046ba7 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 4 Apr 2026 22:03:05 -0400 Subject: [PATCH 08/29] Address jkotas/copilot feedback: fix GC comment, use CORDBG_E_ENC_EDIT_NOT_SUPPORTED - Rewrite GC violation comment per jkotas: the issue is unresolved TypeRef/AssemblyRef for Task/ValueTask, not type misidentification. System.Runtime is typically already resolved so this is unlikely. - Use CORDBG_E_ENC_EDIT_NOT_SUPPORTED instead of COR_E_BADIMAGEFORMAT for infrastructure async rejection, since this is an unsupported edit rather than malformed metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 0bfed4439f4995..d7b5c595e65b2e 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -574,12 +574,10 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** { // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because - // we're only resolving well-known system types (Task/ValueTask) in practice. - // Note: ClassifyMethodReturnKind matches by type name without verifying the assembly - // reference, so a user-defined type named "Task" or "ValueTask" (e.g. via extern - // alias) could be misidentified and induce GC-triggering resolution paths. Accepted - // as Won't Fix given this requires an extremely unlikely combination of naming - // collision, extern alias usage, and hot reload of such a method while debugging. + // these types are typically referenced via System.Runtime which is likely already + // resolved at this point. However, if the TypeRef/AssemblyRef for Task or ValueTask + // has not been resolved yet, the resolution path could trigger GC. Accepted as + // Won't Fix given this is unlikely in practice. CONTRACT_VIOLATION(GCViolation); returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); @@ -603,7 +601,7 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod rejecting infrastructure async method (methodDef: 0x%08x)\n", methodDef)); - return COR_E_BADIMAGEFORMAT; + return CORDBG_E_ENC_EDIT_NOT_SUPPORTED; } MethodDesc* pNewMD; From 1d62c62e2ce861dcfbd8a4d328a416fdfbf0780a Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 4 Apr 2026 22:41:23 -0400 Subject: [PATCH 09/29] Fix cross-allocator async variant signature for generic instantiations Allocate the async variant signature from each instantiation's own LoaderAllocator instead of reusing the type definition's allocation. AddMethodDesc stores the signature as a raw pointer in AsyncMethodData, so it must be owned by the same allocator as the MethodDesc to avoid dangling pointers on collectible ALC unload. Also fix GC violation comment per jkotas feedback and use CORDBG_E_ENC_EDIT_NOT_SUPPORTED for infrastructure async rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index d7b5c595e65b2e..42eb0509c40ad4 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -703,9 +703,18 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // newly added async method is called on value type instantiations. if (isAsyncTaskReturning) { + // Allocate the async variant signature from this instantiation's own + // LoaderAllocator. We cannot reuse pAsyncVariantSig (allocated from pMT's + // allocator) because pMTMaybe may belong to a different LoaderAllocator + // (e.g. collectible ALC), and the signature pointer is stored as a raw + // pointer in AsyncMethodData. + LoaderAllocator* pInstAllocator = pMTMaybe->GetLoaderAllocator(); + BYTE* pInstAsyncSig = (BYTE*)(void*)pInstAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); + memcpy(pInstAsyncSig, pAsyncVariantSig, cAsyncVariantSig); + MethodDesc* pAsyncVariantMDUnused; if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, - asyncVariantFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariantMDUnused))) + asyncVariantFlags, pInstAsyncSig, cAsyncVariantSig, &pAsyncVariantMDUnused))) { LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant for instantiation: 0x%08x\n", hr)); EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, From d587af711846c42080ff5887f1826c1ecc3ca777 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 4 Apr 2026 23:32:56 -0400 Subject: [PATCH 10/29] Replace Unicode arrows with ASCII in C++ comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/method.cpp | 4 ++-- src/coreclr/vm/method.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 394971e6f5ac28..bff67c86b83109 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2479,13 +2479,13 @@ void BuildAsyncVariantSignature( if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) { - // "... Task ... Method(args);" → "... void ... Method(args);" + // "... Task ... Method(args);" -> "... void ... Method(args);" taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID } else { - // "... Task ... Method(args);" → "... T ... Method(args);" + // "... Task ... Method(args);" -> "... T ... Method(args);" taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 taskTypePrefixReplacementSize = 0; } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index c985bf2739f71d..ac27caea690abf 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -283,8 +283,8 @@ MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG // Given a task-returning method signature, compute the async variant signature // by stripping the Task/ValueTask wrapper from the return type. -// For NonGenericTaskReturningMethod: "... Task ... Method(args)" → "... void ... Method(args)" -// For GenericTaskReturningMethod: "... Task ... Method(args)" → "... T ... Method(args)" +// For NonGenericTaskReturningMethod: "... Task ... Method(args)" -> "... void ... Method(args)" +// For GenericTaskReturningMethod: "... Task ... Method(args)" -> "... T ... Method(args)" // If pDestSig is non-null, the new signature is written there (caller must allocate at least *pAsyncSigLen bytes). // *pAsyncSigLen is always set to the required output size. void BuildAsyncVariantSignature( From 89509f8d307743de594632e0a037a684acbeeec8 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sun, 5 Apr 2026 21:39:38 -0400 Subject: [PATCH 11/29] Revert per-instantiation signature allocation per jkotas feedback Generic instantiations are guaranteed to have same or shorter lifetime than the method definition, so reusing the type definition's signature allocation is safe. Remove the unnecessary per-allocator copy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 42eb0509c40ad4..d7b5c595e65b2e 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -703,18 +703,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // newly added async method is called on value type instantiations. if (isAsyncTaskReturning) { - // Allocate the async variant signature from this instantiation's own - // LoaderAllocator. We cannot reuse pAsyncVariantSig (allocated from pMT's - // allocator) because pMTMaybe may belong to a different LoaderAllocator - // (e.g. collectible ALC), and the signature pointer is stored as a raw - // pointer in AsyncMethodData. - LoaderAllocator* pInstAllocator = pMTMaybe->GetLoaderAllocator(); - BYTE* pInstAsyncSig = (BYTE*)(void*)pInstAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); - memcpy(pInstAsyncSig, pAsyncVariantSig, cAsyncVariantSig); - MethodDesc* pAsyncVariantMDUnused; if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, - asyncVariantFlags, pInstAsyncSig, cAsyncVariantSig, &pAsyncVariantMDUnused))) + asyncVariantFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariantMDUnused))) { LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant for instantiation: 0x%08x\n", hr)); EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, From 0ee4ab2245bb067b720c2f933a2a79381f17be51 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 11 Apr 2026 21:48:22 -0400 Subject: [PATCH 12/29] EnC: Create async variant MethodDescs lazily on first use Previously, AddMethod eagerly created async variant MethodDescs for all runtime-async methods added via EnC (on the type-def and every existing generic instantiation). This commit defers variant creation to the point of first use: - FindOrCreateAssociatedMethodDesc (genmeth.cpp): lazily creates the variant using double-check locking with m_InstMethodHashTableCrst, matching the existing NewInstantiatedMethodDesc pattern. - LoadTypicalMethodDefinition (method.cpp): triggers lazy creation when navigating from an instantiation variant back to the type-def variant. - AddAsyncVariant (class.cpp): new helper that computes the variant signature on-demand from metadata (ClassifyMethodReturnKind + BuildAsyncVariantSignature) rather than storing it on primary thunks. - ResetCodeEntryPointForEnC (method.cpp): gracefully handles the case where the variant has not been created yet (no-op). - InitMethodDesc (methodtablebuilder.cpp): restored original contract where only IsAsyncVariant methods store a signature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 154 ++++++++++++++++---------- src/coreclr/vm/class.h | 14 ++- src/coreclr/vm/genmeth.cpp | 56 +++++++++- src/coreclr/vm/method.cpp | 25 ++++- src/coreclr/vm/method.hpp | 3 +- src/coreclr/vm/methodtablebuilder.cpp | 5 +- 6 files changed, 190 insertions(+), 67 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index d7b5c595e65b2e..b017855993b8dd 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -585,12 +585,13 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** bool isAsyncTaskReturning = IsMiAsync(dwImplFlags) && IsTaskReturning(returnKind); - // Create the primary MethodDesc (task-returning variant for async, or the only variant for non-async) + // For runtime-async task-returning methods, the primary MethodDesc is a thunk. + // The async variant is created lazily by FindOrCreateAssociatedMethodDesc when + // first needed (see "EnC async variants" in genmeth.cpp). AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; + if (isAsyncTaskReturning) { - // For IsMiAsync + task-returning: the task-returning variant is a thunk, - // the real IL body belongs to the async variant. primaryAsyncFlags = AsyncMethodFlags::ReturnsTaskOrValueTask | AsyncMethodFlags::Thunk; } else if (IsMiAsync(dwImplFlags)) @@ -604,6 +605,7 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** return CORDBG_E_ENC_EDIT_NOT_SUPPORTED; } + // Create the primary MethodDesc (task-returning thunk for async, or the only MethodDesc for non-async). MethodDesc* pNewMD; if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, primaryAsyncFlags, NULL, 0, &pNewMD))) @@ -620,40 +622,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added pMD:%p for token 0x%08x\n", pNewMD, methodDef)); - // For runtime-async task-returning methods, compute the async variant signature and flags. - // These are used for both the type definition and any existing generic instantiations. - AsyncMethodFlags asyncVariantFlags = AsyncMethodFlags::None; - BYTE* pAsyncVariantSig = NULL; - ULONG cAsyncVariantSig = 0; - - if (isAsyncTaskReturning) - { - asyncVariantFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; - if (returnsValueTask) - asyncVariantFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; - - // Construct the async variant signature by stripping the Task/ValueTask wrapper. - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - nullptr, &cAsyncVariantSig); - - LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); - pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - pAsyncVariantSig, &cAsyncVariantSig); - - // Create the async variant for the type definition. - // For IsMiAsync methods, the async variant has the real IL body (not a thunk). - MethodDesc* pAsyncVariantMD; - if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, - asyncVariantFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariantMD))) - { - LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant: 0x%08x\n", hr)); - return hr; - } - - LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added async variant pMD:%p for token 0x%08x\n", - pAsyncVariantMD, methodDef)); - } + // For runtime-async task-returning methods, the async variant MethodDesc is created + // lazily by FindOrCreateAssociatedMethodDesc (or LoadTypicalMethodDefinition for + // generic type definitions) when first needed. Only the primary thunk is created here. // If the type is generic, then we need to update all existing instantiated types if (pMT->IsGenericTypeDefinition()) @@ -687,6 +658,8 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** continue; } + // Create a primary thunk on this instantiation. The async variant + // will be created lazily by FindOrCreateAssociatedMethodDesc. MethodDesc* pNewMDUnused; if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, primaryAsyncFlags, NULL, 0, &pNewMDUnused))) @@ -696,26 +669,6 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** W("Failed to add method to existing instantiated type instance")); return E_FAIL; } - - // For async task-returning methods, also create the async variant for - // this instantiation. Without this, the thunk on the instantiation's - // MethodTable cannot find its async variant, causing crashes when the - // newly added async method is called on value type instantiations. - if (isAsyncTaskReturning) - { - MethodDesc* pAsyncVariantMDUnused; - if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, - asyncVariantFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariantMDUnused))) - { - LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed to add async variant for instantiation: 0x%08x\n", hr)); - EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, - W("Failed to add async variant to existing instantiated type instance")); - return E_FAIL; - } - - LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added async variant pMD:%p for instantiation pMT:%p\n", - pAsyncVariantMDUnused, pMTMaybe)); - } } } } @@ -727,6 +680,93 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** return S_OK; } +//--------------------------------------------------------------------------------------- +// +// AddAsyncVariant - lazily creates an async variant MethodDesc from a primary thunk. +// Called from FindOrCreateAssociatedMethodDesc when GetAsyncVariant() is first invoked +// on any EnC-added async primary thunk (both generic and non-generic types). +// +// The variant signature is computed on-demand from the method's metadata by stripping +// the Task/ValueTask wrapper from the return type (via ClassifyMethodReturnKind + +// BuildAsyncVariantSignature). The variant flags are derived from the return kind. +// +HRESULT EEClass::AddAsyncVariant( + MethodDesc* pPrimaryThunk, + MethodDesc** ppNewVariant) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + PRECONDITION(pPrimaryThunk != NULL); + PRECONDITION(pPrimaryThunk->IsEnCAddedMethod()); + PRECONDITION(ppNewVariant != NULL); + } + CONTRACTL_END; + + *ppNewVariant = NULL; + + mdMethodDef methodDef = pPrimaryThunk->GetMemberDef(); + MethodTable* pMT = pPrimaryThunk->GetMethodTable(); + + Module* pModule = pMT->GetModule(); + IMDInternalImport* pImport = pModule->GetMDImport(); + + DWORD dwDescrOffset, dwImplFlags, dwMemberAttrs; + HRESULT hr; + if (FAILED(hr = pImport->GetMethodImplProps(methodDef, &dwDescrOffset, &dwImplFlags))) + return hr; + if (FAILED(hr = pImport->GetMethodDefProps(methodDef, &dwMemberAttrs))) + return hr; + + // Fetch the method signature from metadata and compute the async variant signature + // by stripping the Task/ValueTask wrapper from the return type. + ULONG sigLen; + PCCOR_SIGNATURE pMemberSignature; + if (FAILED(hr = pImport->GetSigOfMethodDef(methodDef, &sigLen, &pMemberSignature))) + return hr; + + ULONG offsetOfAsyncDetails = 0; + bool returnsValueTask = false; + MethodReturnKind returnKind = ClassifyMethodReturnKind( + SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); + + _ASSERTE(IsTaskReturning(returnKind)); + + ULONG cAsyncVariantSig = 0; + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + nullptr, &cAsyncVariantSig); + + LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); + BYTE* pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + pAsyncVariantSig, &cAsyncVariantSig); + + AsyncMethodFlags variantFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; + if (returnsValueTask) + variantFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; + + hr = AddMethodDesc( + pMT, + methodDef, + dwImplFlags, + dwMemberAttrs, + variantFlags, + pAsyncVariantSig, + cAsyncVariantSig, + ppNewVariant); + + if (SUCCEEDED(hr) && *ppNewVariant != NULL) + { + LOG((LF_ENC, LL_INFO100, + "EEClass::AddAsyncVariant: created async variant pMD:%p for pMT:%p from primary:%p\n", + *ppNewVariant, pMT, pPrimaryThunk)); + } + + return hr; +} + //--------------------------------------------------------------------------------------- // // AddMethodDesc - called when a new MethodDesc needs to be created and added for EnC diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index bc5f2df4753b5a..bcc860c6cbc39e 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -787,6 +787,16 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! #ifdef FEATURE_METADATA_UPDATER // Add a new method to an already loaded type for EnC static HRESULT AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** ppMethod); + + // Lazily create an async variant MethodDesc from an EnC-added primary thunk. + // Called from FindOrCreateAssociatedMethodDesc when an async variant is first + // needed. Computes the variant signature on-demand from the method's metadata. + static HRESULT AddAsyncVariant( + MethodDesc* pPrimaryThunk, + MethodDesc** ppNewVariant); + + // Add a new field to an already loaded type for EnC + static HRESULT AddField(MethodTable* pMT, mdFieldDef fieldDesc, FieldDesc** pAddedField); private: static HRESULT AddMethodDesc( MethodTable* pMT, @@ -797,10 +807,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! PCCOR_SIGNATURE pAsyncSig, DWORD cbAsyncSig, MethodDesc** ppNewMD); -public: - // Add a new field to an already loaded type for EnC - static HRESULT AddField(MethodTable* pMT, mdFieldDef fieldDesc, FieldDesc** pAddedField); -private: static HRESULT AddFieldDesc( MethodTable* pMT, mdMethodDef fieldDef, diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index 62f413ada24e67..cfc402efd814ed 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -741,6 +741,17 @@ InstantiatedMethodDesc::FindLoadedInstantiatedMethodDesc(MethodTable *pExactOrRe // allowCreate may be set to FALSE to enforce that the method searched // should already be in existence - thus preventing creation and GCs during // inappropriate times. +// +// EnC async variants +// ------------------ +// +// When EEClass::AddMethod adds a runtime-async method via EnC, it only creates +// primary thunks. The actual async variant MethodDescs are created lazily here +// when first needed: the variant signature is computed on-demand from metadata +// using the same double-check locking pattern (m_InstMethodHashTableCrst) as +// NewInstantiatedMethodDesc. LoadTypicalMethodDefinition also triggers this +// lazy creation when navigating from an instantiation variant to the type-def. +// /* static */ MethodDesc* MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, @@ -831,8 +842,51 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, pMDescInCanonMT = pExactMT->GetCanonicalMethodTable()->GetParallelMethodDesc(pDefMD, asyncVariantLookup); - if (!allowCreate && !pMDescInCanonMT->GetMethodTable()->IsFullyLoaded()) +#ifdef FEATURE_METADATA_UPDATER +#ifndef DACCESS_COMPILE + // Lazy async variant creation for EnC-added async methods + // (see "EnC async variants" in the block comment above). + if (asyncVariantLookup == AsyncVariantLookup::AsyncOtherVariant + && allowCreate + && pDefMD->IsEnCAddedMethod() + && (pMDescInCanonMT == NULL || + (pMDescInCanonMT->IsEnCAddedMethod() + && pMDescInCanonMT->GetMethodTable() != pExactMT->GetCanonicalMethodTable()))) + { + MethodTable* pCanonMT = pExactMT->GetCanonicalMethodTable(); + MethodDesc* pPrimaryOnCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, AsyncVariantLookup::MatchingAsyncVariant); + + if (pPrimaryOnCanonMT != NULL && pPrimaryOnCanonMT->IsEnCAddedMethod()) + { + Module* pModule = pDefMD->GetModule(); + GCX_COOP(); + CrstHolder ch(&pModule->m_InstMethodHashTableCrst); + + // Double-check: another thread may have created the variant while we waited for the lock. + pMDescInCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, asyncVariantLookup); + if (pMDescInCanonMT == NULL || + (pMDescInCanonMT->IsEnCAddedMethod() + && pMDescInCanonMT->GetMethodTable() != pCanonMT)) + { + MethodDesc* pNewAsyncVariant = NULL; + HRESULT hr = EEClass::AddAsyncVariant(pPrimaryOnCanonMT, &pNewAsyncVariant); + + if (FAILED(hr)) + { + COMPlusThrowHR(hr); + } + + pMDescInCanonMT = pNewAsyncVariant; + } + } + } +#endif // !DACCESS_COMPILE +#endif // FEATURE_METADATA_UPDATER + + if (pMDescInCanonMT == NULL + || (!allowCreate && !pMDescInCanonMT->GetMethodTable()->IsFullyLoaded())) { + _ASSERTE(pMDescInCanonMT != NULL || !allowCreate); RETURN(NULL); } diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index bff67c86b83109..43078889b8a4de 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1712,6 +1712,23 @@ MethodDesc* MethodDesc::LoadTypicalMethodDefinition() } CONSISTENCY_CHECK(TypeHandle(pMT).CheckFullyLoaded()); MethodDesc *resultMD = pMT->GetParallelMethodDesc(this); + +#ifdef FEATURE_METADATA_UPDATER + // For EnC-added async variants, the type-def's variant may not exist yet + // (all async variants are created lazily). Find the primary thunk on the + // type-def and use FCAMD to trigger lazy creation. + if (resultMD == NULL && IsEnCAddedMethod() && IsAsyncVariantMethod()) + { + MethodDesc* pPrimaryOnTypeDef = pMT->GetParallelMethodDesc(this, AsyncVariantLookup::AsyncOtherVariant); + if (pPrimaryOnTypeDef != NULL) + { + resultMD = FindOrCreateAssociatedMethodDesc( + pPrimaryOnTypeDef, pMT, FALSE, Instantiation(), TRUE, FALSE, TRUE, + AsyncVariantLookup::AsyncOtherVariant); + } + } +#endif // FEATURE_METADATA_UPDATER + _ASSERTE(resultMD != NULL); resultMD->CheckRestore(); RETURN (resultMD); @@ -3401,11 +3418,15 @@ void MethodDesc::ResetCodeEntryPointForEnC() // Updates are expressed via metadata diff and a methoddef of a runtime async method // would be resolved to the task-returning thunk. // If we see a thunk here, fetch the async variant that owns the IL and reset that. + // For EnC-added methods, the variant may not exist yet (created lazily on first call); + // if so, there is no compiled code to reset. if (IsAsyncThunkMethod()) { MethodDesc *otherVariant = GetAsyncVariantNoCreate(); - _ASSERTE(otherVariant != NULL); - otherVariant->ResetCodeEntryPointForEnC(); + if (otherVariant != NULL) + { + otherVariant->ResetCodeEntryPointForEnC(); + } return; } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index ac27caea690abf..0a2c3c672d6ca4 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1776,7 +1776,8 @@ class MethodDesc } // same as above, but with allowCreate = FALSE - // for rare cases where we cannot allow GC, but we know that the other variant is already created. + // For EnC-added async methods, the variant may not exist yet (created lazily), + // so callers must handle a NULL return. MethodDesc* GetAsyncVariantNoCreate(BOOL allowInstParam = TRUE) { MethodTable* mt = GetMethodTable(); diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 788c42a6fedce2..a1817232f7f3a3 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -6405,8 +6405,9 @@ MethodTableBuilder::InitMethodDesc( AsyncMethodData* pAsyncMethodData = pNewMD->GetAddrOfAsyncMethodData(); pAsyncMethodData->flags = asyncFlags; - // async variants have a signature different from their task-returning - // definitions, so we store the signature together with the flags + // Store the async variant signature if provided. Only async variants + // (IsAsyncVariant) have a modified signature; primary thunks use the + // original metadata signature and do not store one here. if (hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant)) { pAsyncMethodData->sig = sig; From 9eba84162f2a4b4bb62228e1658aa1a4b481ef64 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Sat, 11 Apr 2026 23:11:15 -0400 Subject: [PATCH 13/29] Use VolatileStore in AddChunk for ARM64 store ordering AddChunk publishes a new MethodDescChunk to a lock-free linked list that concurrent readers iterate without holding a lock. On weakly ordered architectures (ARM64), plain stores can be reordered, allowing a reader to see the new chunk pointer before the chunk's internal data (MethodDescs, flags, etc.) is fully visible. Use VolatileStore at both publish points (head and tail) to act as a release barrier, matching the pattern used elsewhere in the VM (e.g. dacenumerablehash.inl, codeversion.cpp, eehash.inl). This is a no-op on x86/x64 (strong memory model). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 6 ++++-- src/coreclr/vm/method.hpp | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index b017855993b8dd..24eedf67f57124 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -3129,7 +3129,9 @@ void EEClass::AddChunk (MethodDescChunk* pNewChunk) if (head == NULL) { - SetChunks(pNewChunk); + // Use VolatileStore to ensure the chunk's internal data (MethodDescs, flags, etc.) + // is fully visible to concurrent readers before the chunk becomes reachable. + VolatileStore(&m_pChunks, pNewChunk); } else { @@ -3138,7 +3140,7 @@ void EEClass::AddChunk (MethodDescChunk* pNewChunk) while (head->GetNextChunk() != NULL) head = head->GetNextChunk(); - head->SetNextChunk(pNewChunk); + head->SetNextChunkVolatile(pNewChunk); } } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 0a2c3c672d6ca4..a07704162d7f96 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2688,6 +2688,12 @@ class MethodDescChunk m_next = chunk; } + void SetNextChunkVolatile(MethodDescChunk *chunk) + { + LIMITED_METHOD_CONTRACT; + VolatileStore(&m_next, dac_cast(chunk)); + } + void SetLoaderModuleAttachedToChunk(Module* pModule) { m_flagsAndTokenRange |= enum_flag_LoaderModuleAttachedToChunk; From c5b12bcffa661ee3f44a5c51e65ce9eccdd4d6ec Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Tue, 14 Apr 2026 21:24:38 -0400 Subject: [PATCH 14/29] Address review feedback: narrow GC violation scope, fix redundant reset - Gate ClassifyMethodReturnKind on IsMiAsync in AddMethod so non-async methods never hit the GC-triggering type resolution path. Updated comment to note EnC runs with threads suspended. - Fix redundant double-reset in UpdateMethod: ResetCodeEntryPointForEnC already cascades from thunk to IL-owning variant, so skip the explicit variant reset when pMethod is the thunk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 44 ++++++++++++++++++++++------------------ src/coreclr/vm/encee.cpp | 9 ++++---- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 24eedf67f57124..a98f7e6c1fb4d5 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -560,30 +560,34 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } #endif // _DEBUG - // Classify the method's return type to determine if it needs async variant handling. // For runtime-async methods (IsMiAsync) that return Task/ValueTask, we need to create - // two MethodDescs: the task-returning variant and the async call variant. - ULONG sigLen; - PCCOR_SIGNATURE pMemberSignature; - if (FAILED(pImport->GetSigOfMethodDef(methodDef, &sigLen, &pMemberSignature))) - return COR_E_BADIMAGEFORMAT; + // two MethodDescs: the task-returning variant and the async call variant. Only perform + // return-type classification when IsMiAsync is set to avoid the GC-triggering type + // resolution path for non-async methods. + bool isAsyncTaskReturning = false; - ULONG offsetOfAsyncDetails = 0; - bool returnsValueTask = false; - MethodReturnKind returnKind; + if (IsMiAsync(dwImplFlags)) { - // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which - // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because - // these types are typically referenced via System.Runtime which is likely already - // resolved at this point. However, if the TypeRef/AssemblyRef for Task or ValueTask - // has not been resolved yet, the resolution path could trigger GC. Accepted as - // Won't Fix given this is unlikely in practice. - CONTRACT_VIOLATION(GCViolation); - returnKind = ClassifyMethodReturnKind( - SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); - } + ULONG sigLen; + PCCOR_SIGNATURE pMemberSignature; + if (FAILED(pImport->GetSigOfMethodDef(methodDef, &sigLen, &pMemberSignature))) + return COR_E_BADIMAGEFORMAT; - bool isAsyncTaskReturning = IsMiAsync(dwImplFlags) && IsTaskReturning(returnKind); + ULONG offsetOfAsyncDetails = 0; + bool returnsValueTask = false; + MethodReturnKind returnKind; + { + // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which + // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because + // AddMethod runs during EnC with all managed threads suspended. Task and ValueTask + // are fundamental types that are virtually always already resolved at this point. + CONTRACT_VIOLATION(GCViolation); + returnKind = ClassifyMethodReturnKind( + SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); + } + + isAsyncTaskReturning = IsTaskReturning(returnKind); + } // For runtime-async task-returning methods, the primary MethodDesc is a thunk. // The async variant is created lazily by FindOrCreateAssociatedMethodDesc when diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index bdecf6f91a125e..30c5ae4a11e5e5 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -371,10 +371,11 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) // Not a method impacted by generics, so this is the MethodDesc to use. pMethod->ResetCodeEntryPointForEnC(); - // For runtime-async methods, also reset the async variant's entry point. - // The task-returning variant is stored in the module's method lookup, but - // its async counterpart also needs to be re-JITted with the new IL. - if (pMethod->HasAsyncOtherVariant()) + // For runtime-async methods, ResetCodeEntryPointForEnC cascades from the + // thunk to the IL-owning variant automatically. Only do an explicit reset + // of the paired variant when pMethod is the IL-owning variant (not the thunk), + // since in that case the cascade doesn't reach the thunk. + if (!pMethod->IsAsyncThunkMethod() && pMethod->HasAsyncOtherVariant()) { MethodDesc* pAsyncOther = pMethod->GetAsyncOtherVariantNoCreate(); if (pAsyncOther != NULL) From 271c359e7c297ce9712f75f5a44098f5283435b7 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Wed, 15 Apr 2026 12:54:02 -0400 Subject: [PATCH 15/29] Fix misleading comment: classification is for async variants, not GC avoidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index a98f7e6c1fb4d5..90274384da048b 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -560,10 +560,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } #endif // _DEBUG - // For runtime-async methods (IsMiAsync) that return Task/ValueTask, we need to create - // two MethodDescs: the task-returning variant and the async call variant. Only perform - // return-type classification when IsMiAsync is set to avoid the GC-triggering type - // resolution path for non-async methods. + // Runtime-async methods (IsMiAsync) that return Task/ValueTask need two MethodDescs: + // the task-returning variant and the async call variant. Return-type classification + // is only needed for these methods to determine whether to create async variants. bool isAsyncTaskReturning = false; if (IsMiAsync(dwImplFlags)) @@ -577,10 +576,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** bool returnsValueTask = false; MethodReturnKind returnKind; { - // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which - // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because - // AddMethod runs during EnC with all managed threads suspended. Task and ValueTask - // are fundamental types that are virtually always already resolved at this point. + // ClassifyMethodReturnKind resolves TypeRefs which may trigger GC. + // AddMethod runs during EnC with all managed threads suspended, so + // GC is safe in practice even though the contract says GC_NOTRIGGER. CONTRACT_VIOLATION(GCViolation); returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); From a0ded99c20dd5a129f22358fde6297594ba8b1a1 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Wed, 15 Apr 2026 16:12:05 -0400 Subject: [PATCH 16/29] Remove redundant async variant reset in UpdateMethod The explicit reset of the paired async variant in UpdateMethod is dead code: UpdateMethod always receives the thunk (from the delta metadata token), so !IsAsyncThunkMethod() is always false and the block never fires. Even if it did, ResetCodeEntryPointForEnC already cascades from the thunk to the IL-owning variant automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/encee.cpp | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index 30c5ae4a11e5e5..5946a72b719b8c 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -369,27 +369,15 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) if (!pMethod->HasClassOrMethodInstantiation()) { // Not a method impacted by generics, so this is the MethodDesc to use. + // For runtime-async methods, ResetCodeEntryPointForEnC detects the thunk + // and cascades the reset to the async variant (see method.cpp). pMethod->ResetCodeEntryPointForEnC(); - - // For runtime-async methods, ResetCodeEntryPointForEnC cascades from the - // thunk to the IL-owning variant automatically. Only do an explicit reset - // of the paired variant when pMethod is the IL-owning variant (not the thunk), - // since in that case the cascade doesn't reach the thunk. - if (!pMethod->IsAsyncThunkMethod() && pMethod->HasAsyncOtherVariant()) - { - MethodDesc* pAsyncOther = pMethod->GetAsyncOtherVariantNoCreate(); - if (pAsyncOther != NULL) - { - pAsyncOther->ResetCodeEntryPointForEnC(); - } - } } else { // Generics are involved so we need to search for all related MethodDescs. - // Note: ResetCodeEntryPointForEnC() handles async variants automatically — - // when called on a thunk MethodDesc, it detects IsAsyncThunkMethod() and - // cascades the reset to the async variant (see method.cpp). + // For runtime-async methods, ResetCodeEntryPointForEnC detects the thunk + // and cascades the reset to the async variant (see method.cpp). Module* module = pMethod->GetLoaderModule(); mdMethodDef tkMethod = pMethod->GetMemberDef(); From e8ebe059eca4f622297b2a9edc0cf2c9c1a71043 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Wed, 15 Apr 2026 16:38:09 -0400 Subject: [PATCH 17/29] Simplify async classification in AddMethod Consolidate the IsMiAsync handling into a single block with early return for non-task-returning methods, removing the intermediate isAsyncTaskReturning variable and separate if/else-if chain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 90274384da048b..bc9a55a36979cb 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -561,9 +561,10 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** #endif // _DEBUG // Runtime-async methods (IsMiAsync) that return Task/ValueTask need two MethodDescs: - // the task-returning variant and the async call variant. Return-type classification - // is only needed for these methods to determine whether to create async variants. - bool isAsyncTaskReturning = false; + // the task-returning thunk and the async variant that owns the IL. The async variant + // is created lazily by FindOrCreateAssociatedMethodDesc when first needed + // (see "EnC async variants" in genmeth.cpp). + AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; if (IsMiAsync(dwImplFlags)) { @@ -584,28 +585,18 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); } - isAsyncTaskReturning = IsTaskReturning(returnKind); - } - - // For runtime-async task-returning methods, the primary MethodDesc is a thunk. - // The async variant is created lazily by FindOrCreateAssociatedMethodDesc when - // first needed (see "EnC async variants" in genmeth.cpp). - AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; + if (!IsTaskReturning(returnKind)) + { + // IsMiAsync but not task-returning (e.g. infrastructure Await helpers). + // Not supported for EnC. + LOG((LF_ENC, LL_INFO100, + "EEClass::AddMethod rejecting non-task-returning async method (methodDef: 0x%08x)\n", + methodDef)); + return CORDBG_E_ENC_EDIT_NOT_SUPPORTED; + } - if (isAsyncTaskReturning) - { primaryAsyncFlags = AsyncMethodFlags::ReturnsTaskOrValueTask | AsyncMethodFlags::Thunk; } - else if (IsMiAsync(dwImplFlags)) - { - // IsMiAsync but not task-returning: infrastructure async method (e.g. Await helpers). - // There is no known scenario that benefits from adding these via EnC, so reject - // unconditionally rather than allowing them even on the system module. - LOG((LF_ENC, LL_INFO100, - "EEClass::AddMethod rejecting infrastructure async method (methodDef: 0x%08x)\n", - methodDef)); - return CORDBG_E_ENC_EDIT_NOT_SUPPORTED; - } // Create the primary MethodDesc (task-returning thunk for async, or the only MethodDesc for non-async). MethodDesc* pNewMD; From e51074e3e4974fa35405aace01cb68c48525b5a0 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Wed, 15 Apr 2026 21:27:56 -0400 Subject: [PATCH 18/29] Address PR review feedback: defensive checks and VolatileLoad pairing - AddAsyncVariant: replace standalone assert with runtime guard that returns COR_E_BADIMAGEFORMAT for non-task-returning methods in Release builds (assert inside for Debug). - FCAMD: gate lazy async variant creation on IsAsyncThunkMethod() to prevent calling AddAsyncVariant on non-async EnC methods. - GetChunks/GetNextChunk: pair VolatileStore in AddChunk with VolatileLoad on readers for proper acquire/release semantics on weak memory models. DAC paths use plain loads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 6 +++++- src/coreclr/vm/class.inl | 4 ++++ src/coreclr/vm/genmeth.cpp | 3 ++- src/coreclr/vm/method.hpp | 4 ++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index bc9a55a36979cb..f42cea00185914 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -725,7 +725,11 @@ HRESULT EEClass::AddAsyncVariant( MethodReturnKind returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); - _ASSERTE(IsTaskReturning(returnKind)); + if (!IsTaskReturning(returnKind)) + { + _ASSERTE(!"AddAsyncVariant called on non-task-returning method"); + return COR_E_BADIMAGEFORMAT; + } ULONG cAsyncVariantSig = 0; BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, diff --git a/src/coreclr/vm/class.inl b/src/coreclr/vm/class.inl index bfce7c4678fb46..758a142a0df08f 100644 --- a/src/coreclr/vm/class.inl +++ b/src/coreclr/vm/class.inl @@ -10,7 +10,11 @@ inline PTR_MethodDescChunk EEClass::GetChunks() { LIMITED_METHOD_DAC_CONTRACT; +#ifdef DACCESS_COMPILE return m_pChunks; +#else + return VolatileLoad(&m_pChunks); +#endif } //******************************************************************************* diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index cfc402efd814ed..74ad097bc215e0 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -856,7 +856,8 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, MethodTable* pCanonMT = pExactMT->GetCanonicalMethodTable(); MethodDesc* pPrimaryOnCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, AsyncVariantLookup::MatchingAsyncVariant); - if (pPrimaryOnCanonMT != NULL && pPrimaryOnCanonMT->IsEnCAddedMethod()) + if (pPrimaryOnCanonMT != NULL && pPrimaryOnCanonMT->IsEnCAddedMethod() + && pPrimaryOnCanonMT->IsAsyncThunkMethod()) { Module* pModule = pDefMD->GetModule(); GCX_COOP(); diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index a07704162d7f96..c66aef99f75423 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2705,7 +2705,11 @@ class MethodDescChunk PTR_MethodDescChunk GetNextChunk() { LIMITED_METHOD_CONTRACT; +#ifdef DACCESS_COMPILE return m_next; +#else + return VolatileLoad(&m_next); +#endif } UINT32 GetCount() From 02d50c5b2c05c1c7552d6b08697fe2d08fe6b0c1 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 16 Apr 2026 12:59:07 -0400 Subject: [PATCH 19/29] Update GC safety comment to acknowledge Won't Fix corner case Restore the comment explaining that ClassifyMethodReturnKind matching by type name without verifying the assembly reference is a legitimate but corner case bug (extern alias + naming collision + hot reload), accepted as Won't Fix rather than claiming GC is safe in practice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index f42cea00185914..ded0710318f4c7 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -577,9 +577,14 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** bool returnsValueTask = false; MethodReturnKind returnKind; { - // ClassifyMethodReturnKind resolves TypeRefs which may trigger GC. - // AddMethod runs during EnC with all managed threads suspended, so - // GC is safe in practice even though the contract says GC_NOTRIGGER. + // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which + // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because + // we're only resolving well-known system types (Task/ValueTask) in practice. + // Note: ClassifyMethodReturnKind matches by type name without verifying the assembly + // reference, so a user-defined type named "Task" or "ValueTask" (e.g. via extern + // alias) could be misidentified and induce GC-triggering resolution paths. Accepted + // as Won't Fix given this requires an extremely unlikely combination of naming + // collision, extern alias usage, and hot reload of such a method while debugging. CONTRACT_VIOLATION(GCViolation); returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); From 9b4e02f17ca5f9033200c3571a60b0148bfcf5ab Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 16 Apr 2026 14:32:16 -0400 Subject: [PATCH 20/29] Simplify FCAMD lazy creation check and update GC comment - Remove pMDescInCanonMT->GetMethodTable() != pExactMT->GetCanonicalMethodTable() condition (always false since GetParallelMethodDesc searches the canonical MT). Replace with asserts to verify the invariant. - Update GC safety comment to acknowledge the ClassifyMethodReturnKind extern alias corner case as a legitimate Won't Fix bug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/genmeth.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index 74ad097bc215e0..825b5e07fda3e7 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -849,10 +849,10 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, if (asyncVariantLookup == AsyncVariantLookup::AsyncOtherVariant && allowCreate && pDefMD->IsEnCAddedMethod() - && (pMDescInCanonMT == NULL || - (pMDescInCanonMT->IsEnCAddedMethod() - && pMDescInCanonMT->GetMethodTable() != pExactMT->GetCanonicalMethodTable()))) + && pMDescInCanonMT == NULL) { + _ASSERTE(pMDescInCanonMT == NULL || pMDescInCanonMT->GetMethodTable() == pExactMT->GetCanonicalMethodTable()); + MethodTable* pCanonMT = pExactMT->GetCanonicalMethodTable(); MethodDesc* pPrimaryOnCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, AsyncVariantLookup::MatchingAsyncVariant); @@ -865,9 +865,8 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, // Double-check: another thread may have created the variant while we waited for the lock. pMDescInCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, asyncVariantLookup); - if (pMDescInCanonMT == NULL || - (pMDescInCanonMT->IsEnCAddedMethod() - && pMDescInCanonMT->GetMethodTable() != pCanonMT)) + _ASSERTE(pMDescInCanonMT == NULL || pMDescInCanonMT->IsEnCAddedMethod()); + if (pMDescInCanonMT == NULL) { MethodDesc* pNewAsyncVariant = NULL; HRESULT hr = EEClass::AddAsyncVariant(pPrimaryOnCanonMT, &pNewAsyncVariant); From c639080f8fbdd3f6c399d5e4abe19c789b7009bf Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 16 Apr 2026 20:15:22 -0400 Subject: [PATCH 21/29] Fix rebase against Vlad's async variant API renames - Update AsyncVariantLookup enum values: AsyncOtherVariant -> Async, MatchingAsyncVariant -> Ordinary - Update ClassifyMethodReturnKind calls to pass new elementTypeLength param - Update FindOrCreateAssociatedMethodDesc call to match new parameter order (AsyncVariantLookup moved before forceRemotableMethod) - Update debugger.cpp: HasAsyncOtherVariant -> IsAsyncThunkMethod, GetAsyncOtherVariantNoCreate -> GetAsyncVariantNoCreate - Add missing variable declarations in methodtablebuilder.cpp - Remove tautological assert in FCAMD lazy creation block Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 4 ++-- src/coreclr/vm/class.cpp | 6 ++++-- src/coreclr/vm/genmeth.cpp | 6 ++---- src/coreclr/vm/method.cpp | 6 +++--- src/coreclr/vm/methodtablebuilder.cpp | 4 ++++ 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 94afb1cb3e59a7..49843da3f99f4e 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -12914,9 +12914,9 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) // async variant (runs on resumption after await). Each has its own JIT info with // different sequence points. Without planting breakpoints on both, remap during // async resumption won't fire because the breakpoints only cover the entry variant. - if (pMD->HasAsyncOtherVariant()) + if (pMD->IsAsyncThunkMethod()) { - MethodDesc* pAsyncOther = pMD->GetAsyncOtherVariantNoCreate(); + MethodDesc* pAsyncOther = pMD->GetAsyncVariantNoCreate(); if (pAsyncOther != NULL) { DebuggerJitInfo *pAsyncJitInfo = GetLatestJitInfoFromMethodDesc(pAsyncOther); diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index ded0710318f4c7..2a732add7f8bd9 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -586,8 +586,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // as Won't Fix given this requires an extremely unlikely combination of naming // collision, extern alias usage, and hot reload of such a method while debugging. CONTRACT_VIOLATION(GCViolation); + ULONG elementTypeLength = 0; returnKind = ClassifyMethodReturnKind( - SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); + SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &elementTypeLength, &returnsValueTask); } if (!IsTaskReturning(returnKind)) @@ -727,8 +728,9 @@ HRESULT EEClass::AddAsyncVariant( ULONG offsetOfAsyncDetails = 0; bool returnsValueTask = false; + ULONG elementTypeLength = 0; MethodReturnKind returnKind = ClassifyMethodReturnKind( - SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &returnsValueTask); + SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &elementTypeLength, &returnsValueTask); if (!IsTaskReturning(returnKind)) { diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index 825b5e07fda3e7..f7ad90d1970a1e 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -846,15 +846,13 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, #ifndef DACCESS_COMPILE // Lazy async variant creation for EnC-added async methods // (see "EnC async variants" in the block comment above). - if (asyncVariantLookup == AsyncVariantLookup::AsyncOtherVariant + if (asyncVariantLookup == AsyncVariantLookup::Async && allowCreate && pDefMD->IsEnCAddedMethod() && pMDescInCanonMT == NULL) { - _ASSERTE(pMDescInCanonMT == NULL || pMDescInCanonMT->GetMethodTable() == pExactMT->GetCanonicalMethodTable()); - MethodTable* pCanonMT = pExactMT->GetCanonicalMethodTable(); - MethodDesc* pPrimaryOnCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, AsyncVariantLookup::MatchingAsyncVariant); + MethodDesc* pPrimaryOnCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, AsyncVariantLookup::Ordinary); if (pPrimaryOnCanonMT != NULL && pPrimaryOnCanonMT->IsEnCAddedMethod() && pPrimaryOnCanonMT->IsAsyncThunkMethod()) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 43078889b8a4de..665c095e5e1fe9 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1719,12 +1719,12 @@ MethodDesc* MethodDesc::LoadTypicalMethodDefinition() // type-def and use FCAMD to trigger lazy creation. if (resultMD == NULL && IsEnCAddedMethod() && IsAsyncVariantMethod()) { - MethodDesc* pPrimaryOnTypeDef = pMT->GetParallelMethodDesc(this, AsyncVariantLookup::AsyncOtherVariant); + MethodDesc* pPrimaryOnTypeDef = pMT->GetParallelMethodDesc(this, AsyncVariantLookup::Ordinary); if (pPrimaryOnTypeDef != NULL) { resultMD = FindOrCreateAssociatedMethodDesc( - pPrimaryOnTypeDef, pMT, FALSE, Instantiation(), TRUE, FALSE, TRUE, - AsyncVariantLookup::AsyncOtherVariant); + pPrimaryOnTypeDef, pMT, FALSE, Instantiation(), TRUE, + AsyncVariantLookup::Async, FALSE, TRUE); } } #endif // FEATURE_METADATA_UPDATER diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index a1817232f7f3a3..ada32b2bca93a0 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3415,6 +3415,10 @@ MethodTableBuilder::EnumerateClassMethods() // the token for T or inserting void instead. // The rest of the signature stays exactly the same. ULONG taskTokenLen = 0; + ULONG cAsyncThunkMemberSignature; + ULONG taskTokenOffsetFromAsyncDetailsOffset; + ULONG taskTypePrefixSize; + ULONG taskTypePrefixReplacementSize; if (insertCount == 2) { From fb411359f5116a8d935fc7048011ce597e45f3e2 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 16 Apr 2026 20:40:56 -0400 Subject: [PATCH 22/29] Revert cosmetic declaration move; keep InitMethodDesc comment clarification Reverts the unnecessary variable declaration reordering in EnumerateClassMethods that fell out of the rebase conflict resolution. Keeps the improved comment in InitMethodDesc that clarifies primary thunks use the original metadata signature and do not store one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/methodtablebuilder.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index ada32b2bca93a0..05baee16279db8 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3397,6 +3397,11 @@ MethodTableBuilder::EnumerateClassMethods() { // Extra pass, add an async variant. + ULONG cAsyncThunkMemberSignature; + ULONG taskTokenOffsetFromAsyncDetailsOffset; + ULONG taskTypePrefixSize; + ULONG taskTypePrefixReplacementSize; + AsyncMethodFlags asyncFlags = (AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant); if (returnsValueTask) { @@ -3415,10 +3420,6 @@ MethodTableBuilder::EnumerateClassMethods() // the token for T or inserting void instead. // The rest of the signature stays exactly the same. ULONG taskTokenLen = 0; - ULONG cAsyncThunkMemberSignature; - ULONG taskTokenOffsetFromAsyncDetailsOffset; - ULONG taskTypePrefixSize; - ULONG taskTypePrefixReplacementSize; if (insertCount == 2) { From 3dea83b8b111175702ae627102748ef731de17b9 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Mon, 20 Apr 2026 10:34:49 -0400 Subject: [PATCH 23/29] Create EnC async variant MethodDescs eagerly in AddMethod Instead of lazily creating async variant MethodDescs in FindOrCreateAssociatedMethodDesc, create them eagerly alongside the primary thunk in EEClass::AddMethod. The variant signature is computed once from metadata and then AddMethodDesc (GC_NOTRIGGER) creates both MethodDescs. For generic types, each existing instantiation gets its own sig copy from its loader allocator. This eliminates: - Lazy creation block in FCAMD (double-check locking, GCX_COOP, lock) - LoadTypicalMethodDefinition fallback for async variants - EEClass::AddAsyncVariant helper (no longer needed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 154 +++++++++----------------- src/coreclr/vm/class.h | 14 +-- src/coreclr/vm/encee.cpp | 4 - src/coreclr/vm/genmeth.cpp | 50 +-------- src/coreclr/vm/method.cpp | 25 +---- src/coreclr/vm/method.hpp | 3 +- src/coreclr/vm/methodtablebuilder.cpp | 5 +- 7 files changed, 68 insertions(+), 187 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 2a732add7f8bd9..aa86ccfc1fc9ff 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -561,10 +561,12 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** #endif // _DEBUG // Runtime-async methods (IsMiAsync) that return Task/ValueTask need two MethodDescs: - // the task-returning thunk and the async variant that owns the IL. The async variant - // is created lazily by FindOrCreateAssociatedMethodDesc when first needed - // (see "EnC async variants" in genmeth.cpp). + // the task-returning thunk and the async variant that owns the IL. Both are created + // eagerly here to avoid lazy creation complexity in FindOrCreateAssociatedMethodDesc. AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; + AsyncMethodFlags variantAsyncFlags = AsyncMethodFlags::None; + BYTE* pAsyncVariantSig = NULL; + ULONG cAsyncVariantSig = 0; if (IsMiAsync(dwImplFlags)) { @@ -602,6 +604,21 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } primaryAsyncFlags = AsyncMethodFlags::ReturnsTaskOrValueTask | AsyncMethodFlags::Thunk; + + // Compute the async variant signature by stripping Task/ValueTask from the return type. + // This is done once here and then reused for all instantiations. + // Each instantiation gets its own copy allocated from its loader allocator. + variantAsyncFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; + if (returnsValueTask) + variantAsyncFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; + + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + nullptr, &cAsyncVariantSig); + + LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); + pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + pAsyncVariantSig, &cAsyncVariantSig); } // Create the primary MethodDesc (task-returning thunk for async, or the only MethodDesc for non-async). @@ -621,9 +638,20 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added pMD:%p for token 0x%08x\n", pNewMD, methodDef)); - // For runtime-async task-returning methods, the async variant MethodDesc is created - // lazily by FindOrCreateAssociatedMethodDesc (or LoadTypicalMethodDefinition for - // generic type definitions) when first needed. Only the primary thunk is created here. + // Create the async variant eagerly alongside the primary thunk. + if (pAsyncVariantSig != NULL) + { + MethodDesc* pAsyncVariant = NULL; + if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, + variantAsyncFlags, pAsyncVariantSig, cAsyncVariantSig, &pAsyncVariant))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod async variant failed: 0x%08x\n", hr)); + return hr; + } + + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added async variant pMD:%p for token 0x%08x\n", + pAsyncVariant, methodDef)); + } // If the type is generic, then we need to update all existing instantiated types if (pMT->IsGenericTypeDefinition()) @@ -657,8 +685,7 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** continue; } - // Create a primary thunk on this instantiation. The async variant - // will be created lazily by FindOrCreateAssociatedMethodDesc. + // Create a primary thunk on this instantiation. MethodDesc* pNewMDUnused; if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, primaryAsyncFlags, NULL, 0, &pNewMDUnused))) @@ -668,6 +695,25 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** W("Failed to add method to existing instantiated type instance")); return E_FAIL; } + + // Also create the async variant eagerly on this instantiation. + if (pAsyncVariantSig != NULL) + { + // Allocate a per-MT copy of the variant signature from the correct loader allocator. + LoaderAllocator* pInstAllocator = pMTMaybe->GetLoaderAllocator(); + BYTE* pInstVariantSig = (BYTE*)(void*)pInstAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); + memcpy(pInstVariantSig, pAsyncVariantSig, cAsyncVariantSig); + + MethodDesc* pInstVariant = NULL; + if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, + variantAsyncFlags, pInstVariantSig, cAsyncVariantSig, &pInstVariant))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod async variant failed on instantiation: 0x%08x\n", hr)); + EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, + W("Failed to add async variant to existing instantiated type instance")); + return E_FAIL; + } + } } } } @@ -679,98 +725,6 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** return S_OK; } -//--------------------------------------------------------------------------------------- -// -// AddAsyncVariant - lazily creates an async variant MethodDesc from a primary thunk. -// Called from FindOrCreateAssociatedMethodDesc when GetAsyncVariant() is first invoked -// on any EnC-added async primary thunk (both generic and non-generic types). -// -// The variant signature is computed on-demand from the method's metadata by stripping -// the Task/ValueTask wrapper from the return type (via ClassifyMethodReturnKind + -// BuildAsyncVariantSignature). The variant flags are derived from the return kind. -// -HRESULT EEClass::AddAsyncVariant( - MethodDesc* pPrimaryThunk, - MethodDesc** ppNewVariant) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - PRECONDITION(pPrimaryThunk != NULL); - PRECONDITION(pPrimaryThunk->IsEnCAddedMethod()); - PRECONDITION(ppNewVariant != NULL); - } - CONTRACTL_END; - - *ppNewVariant = NULL; - - mdMethodDef methodDef = pPrimaryThunk->GetMemberDef(); - MethodTable* pMT = pPrimaryThunk->GetMethodTable(); - - Module* pModule = pMT->GetModule(); - IMDInternalImport* pImport = pModule->GetMDImport(); - - DWORD dwDescrOffset, dwImplFlags, dwMemberAttrs; - HRESULT hr; - if (FAILED(hr = pImport->GetMethodImplProps(methodDef, &dwDescrOffset, &dwImplFlags))) - return hr; - if (FAILED(hr = pImport->GetMethodDefProps(methodDef, &dwMemberAttrs))) - return hr; - - // Fetch the method signature from metadata and compute the async variant signature - // by stripping the Task/ValueTask wrapper from the return type. - ULONG sigLen; - PCCOR_SIGNATURE pMemberSignature; - if (FAILED(hr = pImport->GetSigOfMethodDef(methodDef, &sigLen, &pMemberSignature))) - return hr; - - ULONG offsetOfAsyncDetails = 0; - bool returnsValueTask = false; - ULONG elementTypeLength = 0; - MethodReturnKind returnKind = ClassifyMethodReturnKind( - SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &elementTypeLength, &returnsValueTask); - - if (!IsTaskReturning(returnKind)) - { - _ASSERTE(!"AddAsyncVariant called on non-task-returning method"); - return COR_E_BADIMAGEFORMAT; - } - - ULONG cAsyncVariantSig = 0; - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - nullptr, &cAsyncVariantSig); - - LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); - BYTE* pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - pAsyncVariantSig, &cAsyncVariantSig); - - AsyncMethodFlags variantFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; - if (returnsValueTask) - variantFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; - - hr = AddMethodDesc( - pMT, - methodDef, - dwImplFlags, - dwMemberAttrs, - variantFlags, - pAsyncVariantSig, - cAsyncVariantSig, - ppNewVariant); - - if (SUCCEEDED(hr) && *ppNewVariant != NULL) - { - LOG((LF_ENC, LL_INFO100, - "EEClass::AddAsyncVariant: created async variant pMD:%p for pMT:%p from primary:%p\n", - *ppNewVariant, pMT, pPrimaryThunk)); - } - - return hr; -} - //--------------------------------------------------------------------------------------- // // AddMethodDesc - called when a new MethodDesc needs to be created and added for EnC diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index bcc860c6cbc39e..bc5f2df4753b5a 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -787,16 +787,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! #ifdef FEATURE_METADATA_UPDATER // Add a new method to an already loaded type for EnC static HRESULT AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** ppMethod); - - // Lazily create an async variant MethodDesc from an EnC-added primary thunk. - // Called from FindOrCreateAssociatedMethodDesc when an async variant is first - // needed. Computes the variant signature on-demand from the method's metadata. - static HRESULT AddAsyncVariant( - MethodDesc* pPrimaryThunk, - MethodDesc** ppNewVariant); - - // Add a new field to an already loaded type for EnC - static HRESULT AddField(MethodTable* pMT, mdFieldDef fieldDesc, FieldDesc** pAddedField); private: static HRESULT AddMethodDesc( MethodTable* pMT, @@ -807,6 +797,10 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! PCCOR_SIGNATURE pAsyncSig, DWORD cbAsyncSig, MethodDesc** ppNewMD); +public: + // Add a new field to an already loaded type for EnC + static HRESULT AddField(MethodTable* pMT, mdFieldDef fieldDesc, FieldDesc** pAddedField); +private: static HRESULT AddFieldDesc( MethodTable* pMT, mdMethodDef fieldDef, diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index 5946a72b719b8c..025df8ac2aa77f 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -369,15 +369,11 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) if (!pMethod->HasClassOrMethodInstantiation()) { // Not a method impacted by generics, so this is the MethodDesc to use. - // For runtime-async methods, ResetCodeEntryPointForEnC detects the thunk - // and cascades the reset to the async variant (see method.cpp). pMethod->ResetCodeEntryPointForEnC(); } else { // Generics are involved so we need to search for all related MethodDescs. - // For runtime-async methods, ResetCodeEntryPointForEnC detects the thunk - // and cascades the reset to the async variant (see method.cpp). Module* module = pMethod->GetLoaderModule(); mdMethodDef tkMethod = pMethod->GetMemberDef(); diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index f7ad90d1970a1e..9a1480a5d40836 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -745,12 +745,11 @@ InstantiatedMethodDesc::FindLoadedInstantiatedMethodDesc(MethodTable *pExactOrRe // EnC async variants // ------------------ // -// When EEClass::AddMethod adds a runtime-async method via EnC, it only creates -// primary thunks. The actual async variant MethodDescs are created lazily here -// when first needed: the variant signature is computed on-demand from metadata -// using the same double-check locking pattern (m_InstMethodHashTableCrst) as -// NewInstantiatedMethodDesc. LoadTypicalMethodDefinition also triggers this -// lazy creation when navigating from an instantiation variant to the type-def. +// When EEClass::AddMethod adds a runtime-async method via EnC, it eagerly creates +// both the primary thunk and the async variant MethodDescs. The variant signature +// is computed from metadata (via ClassifyMethodReturnKind + BuildAsyncVariantSignature) +// at AddMethod time, before any generic instantiation loop. For generic types, each +// existing instantiation also gets both MethodDescs created eagerly. // /* static */ MethodDesc* @@ -842,45 +841,6 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, pMDescInCanonMT = pExactMT->GetCanonicalMethodTable()->GetParallelMethodDesc(pDefMD, asyncVariantLookup); -#ifdef FEATURE_METADATA_UPDATER -#ifndef DACCESS_COMPILE - // Lazy async variant creation for EnC-added async methods - // (see "EnC async variants" in the block comment above). - if (asyncVariantLookup == AsyncVariantLookup::Async - && allowCreate - && pDefMD->IsEnCAddedMethod() - && pMDescInCanonMT == NULL) - { - MethodTable* pCanonMT = pExactMT->GetCanonicalMethodTable(); - MethodDesc* pPrimaryOnCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, AsyncVariantLookup::Ordinary); - - if (pPrimaryOnCanonMT != NULL && pPrimaryOnCanonMT->IsEnCAddedMethod() - && pPrimaryOnCanonMT->IsAsyncThunkMethod()) - { - Module* pModule = pDefMD->GetModule(); - GCX_COOP(); - CrstHolder ch(&pModule->m_InstMethodHashTableCrst); - - // Double-check: another thread may have created the variant while we waited for the lock. - pMDescInCanonMT = pCanonMT->GetParallelMethodDesc(pDefMD, asyncVariantLookup); - _ASSERTE(pMDescInCanonMT == NULL || pMDescInCanonMT->IsEnCAddedMethod()); - if (pMDescInCanonMT == NULL) - { - MethodDesc* pNewAsyncVariant = NULL; - HRESULT hr = EEClass::AddAsyncVariant(pPrimaryOnCanonMT, &pNewAsyncVariant); - - if (FAILED(hr)) - { - COMPlusThrowHR(hr); - } - - pMDescInCanonMT = pNewAsyncVariant; - } - } - } -#endif // !DACCESS_COMPILE -#endif // FEATURE_METADATA_UPDATER - if (pMDescInCanonMT == NULL || (!allowCreate && !pMDescInCanonMT->GetMethodTable()->IsFullyLoaded())) { diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 665c095e5e1fe9..bff67c86b83109 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1712,23 +1712,6 @@ MethodDesc* MethodDesc::LoadTypicalMethodDefinition() } CONSISTENCY_CHECK(TypeHandle(pMT).CheckFullyLoaded()); MethodDesc *resultMD = pMT->GetParallelMethodDesc(this); - -#ifdef FEATURE_METADATA_UPDATER - // For EnC-added async variants, the type-def's variant may not exist yet - // (all async variants are created lazily). Find the primary thunk on the - // type-def and use FCAMD to trigger lazy creation. - if (resultMD == NULL && IsEnCAddedMethod() && IsAsyncVariantMethod()) - { - MethodDesc* pPrimaryOnTypeDef = pMT->GetParallelMethodDesc(this, AsyncVariantLookup::Ordinary); - if (pPrimaryOnTypeDef != NULL) - { - resultMD = FindOrCreateAssociatedMethodDesc( - pPrimaryOnTypeDef, pMT, FALSE, Instantiation(), TRUE, - AsyncVariantLookup::Async, FALSE, TRUE); - } - } -#endif // FEATURE_METADATA_UPDATER - _ASSERTE(resultMD != NULL); resultMD->CheckRestore(); RETURN (resultMD); @@ -3418,15 +3401,11 @@ void MethodDesc::ResetCodeEntryPointForEnC() // Updates are expressed via metadata diff and a methoddef of a runtime async method // would be resolved to the task-returning thunk. // If we see a thunk here, fetch the async variant that owns the IL and reset that. - // For EnC-added methods, the variant may not exist yet (created lazily on first call); - // if so, there is no compiled code to reset. if (IsAsyncThunkMethod()) { MethodDesc *otherVariant = GetAsyncVariantNoCreate(); - if (otherVariant != NULL) - { - otherVariant->ResetCodeEntryPointForEnC(); - } + _ASSERTE(otherVariant != NULL); + otherVariant->ResetCodeEntryPointForEnC(); return; } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index c66aef99f75423..3a172a76f39907 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1776,8 +1776,7 @@ class MethodDesc } // same as above, but with allowCreate = FALSE - // For EnC-added async methods, the variant may not exist yet (created lazily), - // so callers must handle a NULL return. + // for rare cases where we cannot allow GC, but we know that the other variant is already created. MethodDesc* GetAsyncVariantNoCreate(BOOL allowInstParam = TRUE) { MethodTable* mt = GetMethodTable(); diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 05baee16279db8..038fa7a8d61417 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -6410,9 +6410,8 @@ MethodTableBuilder::InitMethodDesc( AsyncMethodData* pAsyncMethodData = pNewMD->GetAddrOfAsyncMethodData(); pAsyncMethodData->flags = asyncFlags; - // Store the async variant signature if provided. Only async variants - // (IsAsyncVariant) have a modified signature; primary thunks use the - // original metadata signature and do not store one here. + // async variants have a signature different from their task-returning + // definitions, so we store the signature together with the flags if (hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant)) { pAsyncMethodData->sig = sig; From 0d0fba222aa2e3fc5b98a9b0ea0d2ba93e7d193e Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Tue, 21 Apr 2026 11:15:53 -0400 Subject: [PATCH 24/29] Create async variant MethodDescs for all task-returning methods in EnC Previously AddMethod only created async variants for IsMiAsync methods. Normal type loading (MethodTableBuilder::EnumerateClassMethods) creates variants for all task-returning methods regardless of IsMiAsync. This change aligns EnC with that behavior: - IsMiAsync: primary is thunk, async variant owns the IL - Non-IsMiAsync: primary owns IL, async variant is a thunk Also removes per-instantiation signature copy for generic types since instantiations cannot have longer lifetime than the definition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 66 ++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index aa86ccfc1fc9ff..75869264e59c83 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -560,15 +560,16 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } #endif // _DEBUG - // Runtime-async methods (IsMiAsync) that return Task/ValueTask need two MethodDescs: - // the task-returning thunk and the async variant that owns the IL. Both are created - // eagerly here to avoid lazy creation complexity in FindOrCreateAssociatedMethodDesc. + // All task-returning methods need two MethodDescs: the task-returning variant and + // an async variant with Task/ValueTask stripped from the return type. This matches + // the normal type loading path in MethodTableBuilder::EnumerateClassMethods. + // For IsMiAsync methods the primary is a thunk and the async variant owns the IL; + // for non-IsMiAsync methods the primary owns the IL and the async variant is a thunk. AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; AsyncMethodFlags variantAsyncFlags = AsyncMethodFlags::None; BYTE* pAsyncVariantSig = NULL; ULONG cAsyncVariantSig = 0; - if (IsMiAsync(dwImplFlags)) { ULONG sigLen; PCCOR_SIGNATURE pMemberSignature; @@ -582,18 +583,33 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // ClassifyMethodReturnKind calls IsTypeDefOrRefImplementedInSystemModule which // does type resolution that may trigger GC. We suppress GC_NOTRIGGER here because // we're only resolving well-known system types (Task/ValueTask) in practice. - // Note: ClassifyMethodReturnKind matches by type name without verifying the assembly - // reference, so a user-defined type named "Task" or "ValueTask" (e.g. via extern - // alias) could be misidentified and induce GC-triggering resolution paths. Accepted - // as Won't Fix given this requires an extremely unlikely combination of naming - // collision, extern alias usage, and hot reload of such a method while debugging. CONTRACT_VIOLATION(GCViolation); ULONG elementTypeLength = 0; returnKind = ClassifyMethodReturnKind( SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &elementTypeLength, &returnsValueTask); } - if (!IsTaskReturning(returnKind)) + if (IsTaskReturning(returnKind)) + { + primaryAsyncFlags = AsyncMethodFlags::ReturnsTaskOrValueTask; + if (IsMiAsync(dwImplFlags)) + primaryAsyncFlags |= AsyncMethodFlags::Thunk; + + variantAsyncFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; + if (returnsValueTask) + variantAsyncFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; + if (!IsMiAsync(dwImplFlags)) + variantAsyncFlags |= AsyncMethodFlags::Thunk; + + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + nullptr, &cAsyncVariantSig); + + LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); + pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); + BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, + pAsyncVariantSig, &cAsyncVariantSig); + } + else if (IsMiAsync(dwImplFlags)) { // IsMiAsync but not task-returning (e.g. infrastructure Await helpers). // Not supported for EnC. @@ -602,26 +618,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** methodDef)); return CORDBG_E_ENC_EDIT_NOT_SUPPORTED; } - - primaryAsyncFlags = AsyncMethodFlags::ReturnsTaskOrValueTask | AsyncMethodFlags::Thunk; - - // Compute the async variant signature by stripping Task/ValueTask from the return type. - // This is done once here and then reused for all instantiations. - // Each instantiation gets its own copy allocated from its loader allocator. - variantAsyncFlags = AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant; - if (returnsValueTask) - variantAsyncFlags |= AsyncMethodFlags::IsAsyncVariantForValueTask; - - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - nullptr, &cAsyncVariantSig); - - LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); - pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - pAsyncVariantSig, &cAsyncVariantSig); } - // Create the primary MethodDesc (task-returning thunk for async, or the only MethodDesc for non-async). + // Create the primary MethodDesc (task-returning for async methods, or the only MethodDesc for non-async). MethodDesc* pNewMD; if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, primaryAsyncFlags, NULL, 0, &pNewMD))) @@ -638,7 +637,7 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added pMD:%p for token 0x%08x\n", pNewMD, methodDef)); - // Create the async variant eagerly alongside the primary thunk. + // Create the async variant eagerly alongside the primary. if (pAsyncVariantSig != NULL) { MethodDesc* pAsyncVariant = NULL; @@ -685,7 +684,7 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** continue; } - // Create a primary thunk on this instantiation. + // Create a primary MethodDesc on this instantiation. MethodDesc* pNewMDUnused; if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, primaryAsyncFlags, NULL, 0, &pNewMDUnused))) @@ -699,14 +698,9 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // Also create the async variant eagerly on this instantiation. if (pAsyncVariantSig != NULL) { - // Allocate a per-MT copy of the variant signature from the correct loader allocator. - LoaderAllocator* pInstAllocator = pMTMaybe->GetLoaderAllocator(); - BYTE* pInstVariantSig = (BYTE*)(void*)pInstAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); - memcpy(pInstVariantSig, pAsyncVariantSig, cAsyncVariantSig); - MethodDesc* pInstVariant = NULL; if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, - variantAsyncFlags, pInstVariantSig, cAsyncVariantSig, &pInstVariant))) + variantAsyncFlags, pAsyncVariantSig, cAsyncVariantSig, &pInstVariant))) { LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod async variant failed on instantiation: 0x%08x\n", hr)); EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, From c64dba832b1ea1a91017d3d38a60973e5ec2ac29 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Tue, 21 Apr 2026 11:33:49 -0400 Subject: [PATCH 25/29] De-duplicate async variant signature construction Replace inline NonGenericTask and GenericTask signature rewriting in MethodTableBuilder::EnumerateClassMethods with calls to the shared BuildAsyncVariantSignature function. The ReturnDroppingThunk case (rare covariant override scenario) remains inline since it has unique logic not applicable to the EnC path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/methodtablebuilder.cpp | 71 +++++++-------------------- 1 file changed, 17 insertions(+), 54 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 038fa7a8d61417..86a39c259dca53 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3397,11 +3397,6 @@ MethodTableBuilder::EnumerateClassMethods() { // Extra pass, add an async variant. - ULONG cAsyncThunkMemberSignature; - ULONG taskTokenOffsetFromAsyncDetailsOffset; - ULONG taskTypePrefixSize; - ULONG taskTypePrefixReplacementSize; - AsyncMethodFlags asyncFlags = (AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant); if (returnsValueTask) { @@ -3419,7 +3414,8 @@ MethodTableBuilder::EnumerateClassMethods() // It is basically just removing the Task/ValueTask part of the return type and keeping // the token for T or inserting void instead. // The rest of the signature stays exactly the same. - ULONG taskTokenLen = 0; + ULONG cAsyncThunkMemberSignature; + BYTE* pNewMemberSignature; if (insertCount == 2) { @@ -3431,60 +3427,27 @@ MethodTableBuilder::EnumerateClassMethods() // from ". . . Task . . . Method(args);" we construct // ". . . void . . . Method(args);" - taskTokenOffsetFromAsyncDetailsOffset = 2; - taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - - taskTypePrefixSize = 2 + taskTokenLen + 1 + elementTypeLength; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 - taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID - - cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; - } - else if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) - { - // from ". . . Task . . . Method(args);" we construct - // ". . . void . . . Method(args);" - - taskTokenOffsetFromAsyncDetailsOffset = 1; - taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - - taskTypePrefixSize = 1 + taskTokenLen; // E_T_CLASS/E_T_VALUETYPE - taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID + ULONG taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + 2]); + ULONG taskTypePrefixSize = 2 + taskTokenLen + 1 + elementTypeLength; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + ULONG taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; - } - else if (returnKind == MethodReturnKind::GenericTaskReturningMethod) - { - // from ". . . Task . . . Method(args);" we construct - // ". . . tk . . . Method(args);" - - taskTokenOffsetFromAsyncDetailsOffset = 2; - taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - - taskTypePrefixSize = 2 + taskTokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 - taskTypePrefixReplacementSize = 0; + pNewMemberSignature = AllocateFromHighFrequencyHeap(S_SIZE_T(cAsyncThunkMemberSignature)); - cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; + ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; + ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; + memcpy(pNewMemberSignature, pMemberSignature, offsetOfAsyncDetails); + _ASSERTE((cMemberSignature - originalRemainingSigOffset) == (cAsyncThunkMemberSignature - newRemainingSigOffset)); + memcpy(pNewMemberSignature + newRemainingSigOffset, pMemberSignature + originalRemainingSigOffset, cMemberSignature - originalRemainingSigOffset); + pNewMemberSignature[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; } else { - UNREACHABLE(); - } - - BYTE* pNewMemberSignature = AllocateFromHighFrequencyHeap(S_SIZE_T(cAsyncThunkMemberSignature)); - ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; - ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; - - ULONG initialCopyLen = offsetOfAsyncDetails; - // copy bytes before the original async prefix - memcpy(pNewMemberSignature, pMemberSignature, initialCopyLen); - - // copy bytes after the original async prefix - _ASSERTE((cMemberSignature - originalRemainingSigOffset) == (cAsyncThunkMemberSignature - newRemainingSigOffset)); - memcpy(pNewMemberSignature + newRemainingSigOffset, pMemberSignature + originalRemainingSigOffset, cMemberSignature - originalRemainingSigOffset); - - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod || insertCount == 2) - { - pNewMemberSignature[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; + BuildAsyncVariantSignature(returnKind, pMemberSignature, cMemberSignature, offsetOfAsyncDetails, + nullptr, &cAsyncThunkMemberSignature); + pNewMemberSignature = AllocateFromHighFrequencyHeap(S_SIZE_T(cAsyncThunkMemberSignature)); + BuildAsyncVariantSignature(returnKind, pMemberSignature, cMemberSignature, offsetOfAsyncDetails, + pNewMemberSignature, &cAsyncThunkMemberSignature); } MethodClassification asyncVariantType = type; From f3f6dd42eec849b4af0414206b038935a06d38ef Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Tue, 21 Apr 2026 12:11:38 -0400 Subject: [PATCH 26/29] Switch to user-code MethodDesc for EnC remap breakpoints For task-returning methods with async variants, detect when the primary MethodDesc is a thunk and switch to the async variant that owns the user code before planting remap breakpoints. This eliminates the duplicated breakpoint-planting loop that previously handled the async variant separately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 57 +++++++++---------------------- 1 file changed, 16 insertions(+), 41 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 49843da3f99f4e..2089af3ba10635 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -12850,6 +12850,22 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) // This is called before the MethodDesc is updated to point to the new function. // So this call will get the most recent old function. + // + // Task-returning methods have two MethodDescs: a primary and an async variant. + // If the primary is a thunk (i.e. the type loader created it as a wrapper that + // packages the result into a Task), the user code lives in the async variant. + // Switch to that variant so we plant remap breakpoints on the user code, not + // the thunk. + if (pMD->IsAsyncThunkMethod() && pMD->ReturnsTaskOrValueTask()) + { + MethodDesc* pAsyncVariant = pMD->GetAsyncVariantNoCreate(); + if (pAsyncVariant != NULL) + { + LOG((LF_CORDB, LL_INFO10000, "D::UF: switching from async thunk to user-code variant %p\n", pAsyncVariant)); + pMD = pAsyncVariant; + } + } + DebuggerJitInfo *pJitInfo = GetLatestJitInfoFromMethodDesc(pMD); // We only place the patches if we have jit info for this @@ -12909,47 +12925,6 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) pJitInfo->m_encBreakpointsApplied = true; - // For runtime-async methods, also plant EnC remap breakpoints on the async variant. - // The JIT creates two MethodDescs: an entry variant (runs pre-await code) and an - // async variant (runs on resumption after await). Each has its own JIT info with - // different sequence points. Without planting breakpoints on both, remap during - // async resumption won't fire because the breakpoints only cover the entry variant. - if (pMD->IsAsyncThunkMethod()) - { - MethodDesc* pAsyncOther = pMD->GetAsyncVariantNoCreate(); - if (pAsyncOther != NULL) - { - DebuggerJitInfo *pAsyncJitInfo = GetLatestJitInfoFromMethodDesc(pAsyncOther); - if (pAsyncJitInfo != NULL && !pAsyncJitInfo->m_encBreakpointsApplied) - { - LOG((LF_CORDB, LL_INFO10000, "D::UF: Also applying breakpoints to async variant\n")); - - EnCSequencePointHelper asyncSeqHelper(pAsyncJitInfo); - PTR_DebuggerILToNativeMap asyncSeqMap = pAsyncJitInfo->GetSequenceMap(); - for (unsigned int j = 0; j < pAsyncJitInfo->GetSequenceMapCount(); j++) - { - if (!asyncSeqHelper.ShouldSetRemapBreakpoint(j)) - { - continue; - } - - SIZE_T asyncOffset = asyncSeqMap[j].ilOffset; - LOG((LF_CORDB, LL_INFO10000, "D::UF: placing async variant EnC breakpoint at offset 0x%x (IL: 0x%x)\n", - asyncSeqMap[j].nativeStartOffset, asyncSeqMap[j].ilOffset)); - - DebuggerEnCBreakpoint *asyncBp; - asyncBp = new (interopsafe) DebuggerEnCBreakpoint(asyncOffset, - pAsyncJitInfo, - DebuggerEnCBreakpoint::REMAP_PENDING, - AppDomain::GetCurrentDomain()); - _ASSERTE(asyncBp != NULL); - } - - pAsyncJitInfo->m_encBreakpointsApplied = true; - } - } - } - return S_OK; } From 4505981432f2ede437297b2ac1e0daf4e2db0eb4 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 23 Apr 2026 14:34:20 -0400 Subject: [PATCH 27/29] Inline async variant sig construction and revert shared function Per reviewer feedback, inline the sig construction directly in EEClass::AddMethod instead of using a shared BuildAsyncVariantSignature function. Remove the function from method.cpp/method.hpp. Revert all changes to genmeth.cpp and methodtablebuilder.cpp as no modifications are needed in the normal type loading path for the EnC feature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 35 +++++++++++-- src/coreclr/vm/genmeth.cpp | 14 +----- src/coreclr/vm/method.cpp | 55 --------------------- src/coreclr/vm/method.hpp | 14 ------ src/coreclr/vm/methodtablebuilder.cpp | 71 ++++++++++++++++++++------- 5 files changed, 86 insertions(+), 103 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 75869264e59c83..58c0288f05ab95 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -601,13 +601,40 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** if (!IsMiAsync(dwImplFlags)) variantAsyncFlags |= AsyncMethodFlags::Thunk; - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - nullptr, &cAsyncVariantSig); + // Build the async variant signature by stripping Task/ValueTask from the return type. + // NonGenericTask: "Task Method(args)" -> "void Method(args)" + // GenericTask: "Task Method(args)" -> "T Method(args)" + ULONG tokenLen = CorSigUncompressedDataSize( + &pMemberSignature[offsetOfAsyncDetails + + (returnKind == MethodReturnKind::NonGenericTaskReturningMethod ? 1 : 2)]); + + ULONG taskTypePrefixSize; + ULONG taskTypePrefixReplacementSize; + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + { + taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE + taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID + } + else + { + taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + taskTypePrefixReplacementSize = 0; + } + cAsyncVariantSig = sigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); pAsyncVariantSig = (BYTE*)(void*)pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(cAsyncVariantSig)); - BuildAsyncVariantSignature(returnKind, pMemberSignature, sigLen, offsetOfAsyncDetails, - pAsyncVariantSig, &cAsyncVariantSig); + + ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; + ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; + + memcpy(pAsyncVariantSig, pMemberSignature, offsetOfAsyncDetails); + memcpy(pAsyncVariantSig + newRemainingSigOffset, + pMemberSignature + originalRemainingSigOffset, + sigLen - originalRemainingSigOffset); + + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + pAsyncVariantSig[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; } else if (IsMiAsync(dwImplFlags)) { diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index 9a1480a5d40836..62f413ada24e67 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -741,16 +741,6 @@ InstantiatedMethodDesc::FindLoadedInstantiatedMethodDesc(MethodTable *pExactOrRe // allowCreate may be set to FALSE to enforce that the method searched // should already be in existence - thus preventing creation and GCs during // inappropriate times. -// -// EnC async variants -// ------------------ -// -// When EEClass::AddMethod adds a runtime-async method via EnC, it eagerly creates -// both the primary thunk and the async variant MethodDescs. The variant signature -// is computed from metadata (via ClassifyMethodReturnKind + BuildAsyncVariantSignature) -// at AddMethod time, before any generic instantiation loop. For generic types, each -// existing instantiation also gets both MethodDescs created eagerly. -// /* static */ MethodDesc* MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, @@ -841,10 +831,8 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, pMDescInCanonMT = pExactMT->GetCanonicalMethodTable()->GetParallelMethodDesc(pDefMD, asyncVariantLookup); - if (pMDescInCanonMT == NULL - || (!allowCreate && !pMDescInCanonMT->GetMethodTable()->IsFullyLoaded())) + if (!allowCreate && !pMDescInCanonMT->GetMethodTable()->IsFullyLoaded()) { - _ASSERTE(pMDescInCanonMT != NULL || !allowCreate); RETURN(NULL); } diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index bff67c86b83109..6e4772c539db5c 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2458,61 +2458,6 @@ MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG return MethodReturnKind::NormalMethod; } -void BuildAsyncVariantSignature( - MethodReturnKind returnKind, - const BYTE* pOrigSig, - ULONG origSigLen, - ULONG offsetOfAsyncDetails, - BYTE* pDestSig, - ULONG* pAsyncSigLen) -{ - _ASSERTE(returnKind == MethodReturnKind::NonGenericTaskReturningMethod || - returnKind == MethodReturnKind::GenericTaskReturningMethod); - _ASSERTE(pAsyncSigLen != nullptr); - - ULONG taskTypePrefixSize; - ULONG taskTypePrefixReplacementSize; - - ULONG tokenLen = CorSigUncompressedDataSize( - &pOrigSig[offsetOfAsyncDetails + - (returnKind == MethodReturnKind::NonGenericTaskReturningMethod ? 1 : 2)]); - - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) - { - // "... Task ... Method(args);" -> "... void ... Method(args);" - taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE - taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID - } - else - { - // "... Task ... Method(args);" -> "... T ... Method(args);" - taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 - taskTypePrefixReplacementSize = 0; - } - - *pAsyncSigLen = origSigLen - taskTypePrefixSize + taskTypePrefixReplacementSize; - - if (pDestSig != nullptr) - { - ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; - ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; - - // Copy bytes before the async prefix - memcpy(pDestSig, pOrigSig, offsetOfAsyncDetails); - - // Copy bytes after the async prefix - _ASSERTE((origSigLen - originalRemainingSigOffset) == (*pAsyncSigLen - newRemainingSigOffset)); - memcpy(pDestSig + newRemainingSigOffset, - pOrigSig + originalRemainingSigOffset, - origSigLen - originalRemainingSigOffset); - - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) - { - pDestSig[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; - } - } -} - //******************************************************************************* BOOL MethodDesc::ShouldCallPrestub() { diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 3a172a76f39907..ce4ee0e51a6b8a 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -281,20 +281,6 @@ enum class MethodReturnKind bool IsTypeDefOrRefImplementedInSystemModule(Module* pModule, mdToken tk); MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG* offsetOfAsyncDetails, ULONG* elementTypeLength, bool *isValueTask); -// Given a task-returning method signature, compute the async variant signature -// by stripping the Task/ValueTask wrapper from the return type. -// For NonGenericTaskReturningMethod: "... Task ... Method(args)" -> "... void ... Method(args)" -// For GenericTaskReturningMethod: "... Task ... Method(args)" -> "... T ... Method(args)" -// If pDestSig is non-null, the new signature is written there (caller must allocate at least *pAsyncSigLen bytes). -// *pAsyncSigLen is always set to the required output size. -void BuildAsyncVariantSignature( - MethodReturnKind returnKind, - const BYTE* pOrigSig, - ULONG origSigLen, - ULONG offsetOfAsyncDetails, - BYTE* pDestSig, - ULONG* pAsyncSigLen); - inline bool IsTaskReturning(MethodReturnKind input) { return (input == MethodReturnKind::GenericTaskReturningMethod) || diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 86a39c259dca53..038fa7a8d61417 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3397,6 +3397,11 @@ MethodTableBuilder::EnumerateClassMethods() { // Extra pass, add an async variant. + ULONG cAsyncThunkMemberSignature; + ULONG taskTokenOffsetFromAsyncDetailsOffset; + ULONG taskTypePrefixSize; + ULONG taskTypePrefixReplacementSize; + AsyncMethodFlags asyncFlags = (AsyncMethodFlags::AsyncCall | AsyncMethodFlags::IsAsyncVariant); if (returnsValueTask) { @@ -3414,8 +3419,7 @@ MethodTableBuilder::EnumerateClassMethods() // It is basically just removing the Task/ValueTask part of the return type and keeping // the token for T or inserting void instead. // The rest of the signature stays exactly the same. - ULONG cAsyncThunkMemberSignature; - BYTE* pNewMemberSignature; + ULONG taskTokenLen = 0; if (insertCount == 2) { @@ -3427,27 +3431,60 @@ MethodTableBuilder::EnumerateClassMethods() // from ". . . Task . . . Method(args);" we construct // ". . . void . . . Method(args);" - ULONG taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + 2]); - ULONG taskTypePrefixSize = 2 + taskTokenLen + 1 + elementTypeLength; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 - ULONG taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID + taskTokenOffsetFromAsyncDetailsOffset = 2; + taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + + taskTypePrefixSize = 2 + taskTokenLen + 1 + elementTypeLength; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; - pNewMemberSignature = AllocateFromHighFrequencyHeap(S_SIZE_T(cAsyncThunkMemberSignature)); + } + else if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + { + // from ". . . Task . . . Method(args);" we construct + // ". . . void . . . Method(args);" - ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; - ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; - memcpy(pNewMemberSignature, pMemberSignature, offsetOfAsyncDetails); - _ASSERTE((cMemberSignature - originalRemainingSigOffset) == (cAsyncThunkMemberSignature - newRemainingSigOffset)); - memcpy(pNewMemberSignature + newRemainingSigOffset, pMemberSignature + originalRemainingSigOffset, cMemberSignature - originalRemainingSigOffset); - pNewMemberSignature[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; + taskTokenOffsetFromAsyncDetailsOffset = 1; + taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + + taskTypePrefixSize = 1 + taskTokenLen; // E_T_CLASS/E_T_VALUETYPE + taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID + + cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; + } + else if (returnKind == MethodReturnKind::GenericTaskReturningMethod) + { + // from ". . . Task . . . Method(args);" we construct + // ". . . tk . . . Method(args);" + + taskTokenOffsetFromAsyncDetailsOffset = 2; + taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + + taskTypePrefixSize = 2 + taskTokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + taskTypePrefixReplacementSize = 0; + + cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; } else { - BuildAsyncVariantSignature(returnKind, pMemberSignature, cMemberSignature, offsetOfAsyncDetails, - nullptr, &cAsyncThunkMemberSignature); - pNewMemberSignature = AllocateFromHighFrequencyHeap(S_SIZE_T(cAsyncThunkMemberSignature)); - BuildAsyncVariantSignature(returnKind, pMemberSignature, cMemberSignature, offsetOfAsyncDetails, - pNewMemberSignature, &cAsyncThunkMemberSignature); + UNREACHABLE(); + } + + BYTE* pNewMemberSignature = AllocateFromHighFrequencyHeap(S_SIZE_T(cAsyncThunkMemberSignature)); + ULONG originalRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixSize; + ULONG newRemainingSigOffset = offsetOfAsyncDetails + taskTypePrefixReplacementSize; + + ULONG initialCopyLen = offsetOfAsyncDetails; + // copy bytes before the original async prefix + memcpy(pNewMemberSignature, pMemberSignature, initialCopyLen); + + // copy bytes after the original async prefix + _ASSERTE((cMemberSignature - originalRemainingSigOffset) == (cAsyncThunkMemberSignature - newRemainingSigOffset)); + memcpy(pNewMemberSignature + newRemainingSigOffset, pMemberSignature + originalRemainingSigOffset, cMemberSignature - originalRemainingSigOffset); + + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod || insertCount == 2) + { + pNewMemberSignature[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; } MethodClassification asyncVariantType = type; From 43b264aac5117a3e830071d62bf1089372c4bcf9 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Wed, 22 Apr 2026 21:11:51 -0400 Subject: [PATCH 28/29] Return early from UpdateFunction when async variant is missing Per reviewer feedback, return S_OK instead of falling through when GetAsyncVariantNoCreate() returns NULL for an async thunk. There is no user code to plant remap breakpoints on in this case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 2089af3ba10635..32259558699ac8 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -12859,11 +12859,14 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) if (pMD->IsAsyncThunkMethod() && pMD->ReturnsTaskOrValueTask()) { MethodDesc* pAsyncVariant = pMD->GetAsyncVariantNoCreate(); - if (pAsyncVariant != NULL) + if (pAsyncVariant == NULL) { - LOG((LF_CORDB, LL_INFO10000, "D::UF: switching from async thunk to user-code variant %p\n", pAsyncVariant)); - pMD = pAsyncVariant; + LOG((LF_CORDB, LL_INFO10000, "D::UF: async variant not found for %s::%s encVersion %zx\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, encVersion)); + return S_OK; } + LOG((LF_CORDB, LL_INFO10000, "D::UF: switching from async thunk to user-code variant %p\n", pAsyncVariant)); + pMD = pAsyncVariant; } DebuggerJitInfo *pJitInfo = GetLatestJitInfoFromMethodDesc(pMD); From 58e4defffdf8d08731e24a3ba6470879cc553bb9 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 23 Apr 2026 14:38:16 -0400 Subject: [PATCH 29/29] Re-document known corner-case GC violation in AddMethod Re-add comment explaining the Won't Fix corner cases around the CONTRACT_VIOLATION(GCViolation) in EnC async variant creation: type identity may not match well-known types, and the call may trigger GC even for well-known types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/class.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 58c0288f05ab95..aa4387805e3ce3 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -565,6 +565,18 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** // the normal type loading path in MethodTableBuilder::EnumerateClassMethods. // For IsMiAsync methods the primary is a thunk and the async variant owns the IL; // for non-IsMiAsync methods the primary owns the IL and the async variant is a thunk. + // + // The normal type loading path also creates a void-returning ReturnDroppingThunk + // for covariant virtual overrides (base returns Task, derived returns Task). + // EnC-added methods cannot be METHOD_IMPL overrides, so that case does not apply here. + // Note: There are multiple corner-case bugs here we are choosing not to address: + // 1. The types might not be the well-known Task/ValueTask types from + // System.Private.CoreLib. We won't know the answer until after + // ClassifyMethodReturnKind returns. + // 2. Even if the types are the well-known types that alone doesn't guarantee this + // call won't trigger a GC. + // Accepted as Won't Fix given this requires an unlikely combination events during + // an EnC operation while debugging. AsyncMethodFlags primaryAsyncFlags = AsyncMethodFlags::None; AsyncMethodFlags variantAsyncFlags = AsyncMethodFlags::None; BYTE* pAsyncVariantSig = NULL;