diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index ffd41afb34570a..32259558699ac8 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -12850,6 +12850,25 @@ 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: 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); // We only place the patches if we have jit info for this diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index a95ffe073329dc..aa4387805e3ce3 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -560,20 +560,137 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** } #endif // _DEBUG + // 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. + // + // 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; + ULONG cAsyncVariantSig = 0; + + { + 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 suppress GC_NOTRIGGER here because + // we're only resolving well-known system types (Task/ValueTask) in practice. + CONTRACT_VIOLATION(GCViolation); + ULONG elementTypeLength = 0; + returnKind = ClassifyMethodReturnKind( + SigPointer(pMemberSignature, sigLen), pModule, &offsetOfAsyncDetails, &elementTypeLength, &returnsValueTask); + } + + 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; + + // 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)); + + 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)) + { + // 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; + } + } + + // 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, &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)); + // Create the async variant eagerly alongside the primary. + 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()) { @@ -606,14 +723,30 @@ HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** continue; } + // Create a primary MethodDesc on this instantiation. 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, 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) + { + MethodDesc* pInstVariant = NULL; + if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, + 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, + W("Failed to add async variant to existing instantiated type instance")); + return E_FAIL; + } + } } } } @@ -634,6 +767,9 @@ HRESULT EEClass::AddMethodDesc( mdMethodDef methodDef, DWORD dwImplFlags, DWORD dwMemberAttrs, + AsyncMethodFlags asyncFlags, + PCCOR_SIGNATURE pAsyncSig, + DWORD cbAsyncSig, MethodDesc** ppNewMD) { CONTRACTL @@ -660,17 +796,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 +816,7 @@ HRESULT EEClass::AddMethodDesc( classification, TRUE, // fNonVtableSlot TRUE, // fNativeCodeSlot - FALSE, /* HasAsyncMethodData */ + hasAsyncData, /* HasAsyncMethodData */ pMT, &dummyAmTracker); @@ -733,8 +865,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) @@ -2988,7 +3120,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 { @@ -2997,7 +3131,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/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/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/method.hpp b/src/coreclr/vm/method.hpp index 9188d081cd5ee4..ce4ee0e51a6b8a 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2673,6 +2673,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; @@ -2684,7 +2690,11 @@ class MethodDescChunk PTR_MethodDescChunk GetNextChunk() { LIMITED_METHOD_CONTRACT; +#ifdef DACCESS_COMPILE return m_next; +#else + return VolatileLoad(&m_next); +#endif } UINT32 GetCount()