From 61cf6367b338a0c3b0d932866844cffe2b8fcabe Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 13 Mar 2026 18:36:00 -0700 Subject: [PATCH 01/21] added a test --- .../covariant-return/covariant-returns.cs | 82 +++++++++++++++++++ .../covariant-return/covariant-returns.csproj | 9 ++ 2 files changed, 91 insertions(+) create mode 100644 src/tests/async/covariant-return/covariant-returns.cs create mode 100644 src/tests/async/covariant-return/covariant-returns.csproj diff --git a/src/tests/async/covariant-return/covariant-returns.cs b/src/tests/async/covariant-return/covariant-returns.cs new file mode 100644 index 00000000000000..4507f5cd5b7787 --- /dev/null +++ b/src/tests/async/covariant-return/covariant-returns.cs @@ -0,0 +1,82 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Xunit; + +public class CovariantReturns +{ + [Fact] + public static void Test1EntryPoint() + { + Test1().Wait(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task Test0() + { + Base b = new Base(); + await b.M1(); + Assert.Equal("Base.M1;", b.Trace); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task Test1() + { + Base b = DateTime.Now.Year > 0 ? new Derived() : new Base(); + await b.M1(); + Assert.Equal("Derived.M1;", b.Trace); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task Test2() + { + Base b = DateTime.Now.Year > 0 ? new Derived2() : new Base(); + await b.M1(); + Assert.Equal("Derived2.M1;Derived.M1;", b.Trace); + } + + struct S1 + { + public Guid guid; + public int num; + + public S1(int num) + { + this.guid = Guid.NewGuid(); + this.num = num; + } + } + + class Base + { + public string Trace; + public virtual Task M1() + { + Trace += "Base.M1;"; + return Task.CompletedTask; + } + } + + class Derived : Base + { + public override Task M1() + { + Trace += "Derived.M1;"; + return Task.FromResult(new S1(42)); + } + } + + class Derived2 : Derived + { + public override async Task M1() + { + Trace += "Derived2.M1;"; + await Task.Delay(1); + await base.M1(); + return new S1(4242); + } + } +} diff --git a/src/tests/async/covariant-return/covariant-returns.csproj b/src/tests/async/covariant-return/covariant-returns.csproj new file mode 100644 index 00000000000000..b0866d6e569808 --- /dev/null +++ b/src/tests/async/covariant-return/covariant-returns.csproj @@ -0,0 +1,9 @@ + + + + True + + + + + From acbec7c93eda7129ef40fde0c5fc6d69298ddec2 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 17 Mar 2026 21:47:03 -0700 Subject: [PATCH 02/21] Removed Matching/Other Lookup kinds. --- src/coreclr/vm/asyncthunks.cpp | 2 +- src/coreclr/vm/genmeth.cpp | 19 +++++--- src/coreclr/vm/jitinterface.cpp | 9 +++- src/coreclr/vm/memberload.cpp | 1 - src/coreclr/vm/method.hpp | 64 ++++++++++++++++++++------- src/coreclr/vm/methodtable.cpp | 44 +++++++++++------- src/coreclr/vm/methodtable.h | 5 ++- src/coreclr/vm/methodtablebuilder.cpp | 39 ++++++++-------- src/coreclr/vm/methodtablebuilder.h | 10 +---- src/coreclr/vm/runtimehandles.cpp | 2 +- src/coreclr/vm/stubmgr.cpp | 2 +- src/coreclr/vm/zapsig.cpp | 6 +-- 12 files changed, 124 insertions(+), 79 deletions(-) diff --git a/src/coreclr/vm/asyncthunks.cpp b/src/coreclr/vm/asyncthunks.cpp index 5d72e30f85ae41..8d67242d22c4bb 100644 --- a/src/coreclr/vm/asyncthunks.cpp +++ b/src/coreclr/vm/asyncthunks.cpp @@ -25,7 +25,7 @@ bool MethodDesc::TryGenerateAsyncThunk(DynamicResolver** resolver, COR_ILMETHOD_ return false; } - MethodDesc *pAsyncOtherVariant = this->GetAsyncOtherVariant(); + MethodDesc *pAsyncOtherVariant = IsAsyncVariantMethod() ? this->GetOrdinaryVariant() : this->GetAsyncVariant(); _ASSERTE(!IsWrapperStub() && !pAsyncOtherVariant->IsWrapperStub()); MetaSig msig(this); diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index 6dd9bdda0a1f32..88da8fcb0528fb 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -748,9 +748,9 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, BOOL forceBoxedEntryPoint, Instantiation methodInst, BOOL allowInstParam, + AsyncVariantLookup asyncVariantLookup, BOOL forceRemotableMethod, BOOL allowCreate, - AsyncVariantLookup asyncVariantLookup, ClassLoadLevel level) { CONTRACT(MethodDesc*) @@ -788,7 +788,7 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, methodInst.IsEmpty() && !forceBoxedEntryPoint && !pDefMD->IsUnboxingStub() && - asyncVariantLookup == AsyncVariantLookup::MatchingAsyncVariant) + pDefMD->AsyncVariantKind() == asyncVariantLookup) { // Make sure that pDefMD->GetMethodTable() and pExactMT are related types even // if we took the fast path. @@ -817,7 +817,9 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, COMPlusThrowHR(COR_E_TYPELOAD); } - if (pDefMD->HasClassOrMethodInstantiation() || !methodInst.IsEmpty() || asyncVariantLookup == AsyncVariantLookup::AsyncOtherVariant) + if (pDefMD->HasClassOrMethodInstantiation() || + !methodInst.IsEmpty() || + pDefMD->AsyncVariantKind() != asyncVariantLookup) { // General checks related to generics: arity (if any) must match and generic method // instantiation (if any) must be well-formed. @@ -845,8 +847,8 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, if ( methodInst.IsEmpty() && (allowInstParam || !pMDescInCanonMT->RequiresInstArg()) && (forceBoxedEntryPoint == pMDescInCanonMT->IsUnboxingStub()) - && (!forceRemotableMethod || !pMDescInCanonMT->IsInterface() - || !pMDescInCanonMT->GetMethodTable()->IsSharedByGenericInstantiations()) ) + && (!forceRemotableMethod || !pMDescInCanonMT->IsInterface() || !pMDescInCanonMT->GetMethodTable()->IsSharedByGenericInstantiations()) + && (pMDescInCanonMT->AsyncVariantKind() == asyncVariantLookup)) { RETURN(pMDescInCanonMT); } @@ -992,7 +994,10 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, pExactMT, FALSE /* not Unboxing */, methodInst, - FALSE, FALSE, TRUE, asyncVariantLookup); + FALSE, + asyncVariantLookup, + FALSE, + TRUE); _ASSERTE(pNonUnboxingStub->GetClassification() == mcInstantiated); _ASSERTE(!pNonUnboxingStub->RequiresInstArg()); @@ -1200,9 +1205,9 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, FALSE, Instantiation(repInst, methodInst.GetNumArgs()), /* allowInstParam */ TRUE, + asyncVariantLookup, /* forceRemotableMethod */ FALSE, /* allowCreate */ TRUE, - asyncVariantLookup, /* level */ level); _ASSERTE(pWrappedMD->IsSharedByGenericInstantiations()); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 0724c7ac359723..8041129865a390 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8997,10 +8997,15 @@ CORINFO_METHOD_HANDLE CEEInfo::getAsyncOtherVariant( MethodDesc* pMD = GetMethod(ftn); MethodDesc* pAsyncOtherVariant = NULL; - if (pMD->HasAsyncOtherVariant()) + if (pMD->ReturnsTaskOrValueTask()) { - pAsyncOtherVariant = pMD->GetAsyncOtherVariant(); + pAsyncOtherVariant = pMD->GetAsyncVariant(); } + else if (pMD->IsAsyncVariantMethod()) + { + pAsyncOtherVariant = pMD->GetOrdinaryVariant(); + } + result = (CORINFO_METHOD_HANDLE)pAsyncOtherVariant; *variantIsThunk = pAsyncOtherVariant != NULL && pAsyncOtherVariant->IsAsyncThunkMethod(); diff --git a/src/coreclr/vm/memberload.cpp b/src/coreclr/vm/memberload.cpp index ed734b8da3fac0..7f91a27ab4a490 100644 --- a/src/coreclr/vm/memberload.cpp +++ b/src/coreclr/vm/memberload.cpp @@ -781,7 +781,6 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec( allowInstParam, /* forceRemotableMethod */ FALSE, /* allowCreate */ TRUE, - AsyncVariantLookup::MatchingAsyncVariant, /* level */ owningTypeLoadLevel); } // MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index e196bf575bb9ce..5aae4c8c5076d0 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -95,7 +95,7 @@ enum class AsyncMethodFlags // Example: "Task Foo();" ===> "int Foo();" // Example: "ValueTask Bar();" ===> "void Bar();" // - // It is possible to get from one variant to another via GetAsyncOtherVariant. + // It is possible to get from one variant to another via GetAsyncVariant/GetOrdinaryVariant. // // NOTE: Not all AsyncCall methods are "variants" from a pair. // Methods that are explicitly declared as MethodImpl.Async in metadata while @@ -265,8 +265,8 @@ using PTR_MethodDescCodeData = DPTR(MethodDescCodeData); enum class AsyncVariantLookup { - MatchingAsyncVariant = 0, - AsyncOtherVariant + Ordinary = 0, + Async }; enum class MethodReturnKind @@ -1701,25 +1701,42 @@ class MethodDesc BOOL forceBoxedEntryPoint, Instantiation methodInst, BOOL allowInstParam, + AsyncVariantLookup variantLookup, BOOL forceRemotableMethod = FALSE, BOOL allowCreate = TRUE, - AsyncVariantLookup variantLookup = AsyncVariantLookup::MatchingAsyncVariant, ClassLoadLevel level = CLASS_LOADED); - // Normalize methoddesc for reflection - static MethodDesc* FindOrCreateAssociatedMethodDescForReflection(MethodDesc *pMethod, - TypeHandle instType, - Instantiation methodInst); - - inline bool HasAsyncOtherVariant() const + // Common Case: same async variant kind as pPrimaryMD + static MethodDesc* FindOrCreateAssociatedMethodDesc(MethodDesc* pPrimaryMD, + MethodTable* pExactMT, + BOOL forceBoxedEntryPoint, + Instantiation methodInst, + BOOL allowInstParam, + BOOL forceRemotableMethod = FALSE, + BOOL allowCreate = TRUE, + ClassLoadLevel level = CLASS_LOADED) { - return IsAsyncVariantMethod() || ReturnsTaskOrValueTask(); + return FindOrCreateAssociatedMethodDesc( + pPrimaryMD, + pExactMT, + forceBoxedEntryPoint, + methodInst, + allowInstParam, + pPrimaryMD->AsyncVariantKind(), + forceRemotableMethod, + allowCreate, + level); } - MethodDesc* GetAsyncOtherVariant(BOOL allowInstParam = TRUE) + // Normalize methoddesc for reflection + static MethodDesc* FindOrCreateAssociatedMethodDescForReflection(MethodDesc* pMethod, + TypeHandle instType, + Instantiation methodInst); + + MethodDesc* GetOrdinaryVariant(BOOL allowInstParam = TRUE) { - _ASSERTE(HasAsyncOtherVariant()); - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, FALSE, TRUE, AsyncVariantLookup::AsyncOtherVariant); + _ASSERT(IsAsyncVariantMethod()); + return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, TRUE); } MethodDesc* GetAsyncOtherVariantNoCreate(BOOL allowInstParam = TRUE) @@ -1731,7 +1748,7 @@ class MethodDesc MethodDesc* GetAsyncVariant(BOOL allowInstParam = TRUE) { _ASSERT(!IsAsyncVariantMethod()); - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, FALSE, TRUE, AsyncVariantLookup::AsyncOtherVariant); + return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, TRUE); } // same as above, but with allowCreate = FALSE @@ -1739,7 +1756,7 @@ class MethodDesc MethodDesc* GetAsyncVariantNoCreate(BOOL allowInstParam = TRUE) { _ASSERT(!IsAsyncVariantMethod()); - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, FALSE, FALSE, AsyncVariantLookup::AsyncOtherVariant); + return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, FALSE); } // True if a MD is an funny BoxedEntryPointStub (not from the method table) or @@ -2017,6 +2034,21 @@ class MethodDesc return hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant); } + inline AsyncVariantLookup AsyncVariantKind() const + { + LIMITED_METHOD_DAC_CONTRACT; + if (HasAsyncMethodData()) + { + AsyncMethodFlags asyncFlags = GetAddrOfAsyncMethodData()->flags; + if (hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant)) + { + return AsyncVariantLookup::Async; + } + } + + return AsyncVariantLookup::Ordinary; + } + // Is this an Async variant method for a method that // returns ValueTask or ValueTask ? inline bool IsAsyncVariantForValueTaskReturningMethod() const diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 0234a880c986fe..493d90e4a6ac96 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -5695,7 +5695,6 @@ namespace FALSE, // allowInstParam TRUE, // forceRemoteableMethod TRUE, // allowCreate - AsyncVariantLookup::MatchingAsyncVariant, level // level ); } @@ -7919,7 +7918,7 @@ namespace } } -MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantLookup asyncVariantLookup) +MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD) { CONTRACTL { @@ -7929,30 +7928,43 @@ MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantL } CONTRACTL_END; - if (asyncVariantLookup == AsyncVariantLookup::MatchingAsyncVariant) - { #ifdef FEATURE_METADATA_UPDATER - if (pDefMD->IsEnCAddedMethod()) - return GetParallelMethodDescForEnC(this, pDefMD); + if (pDefMD->IsEnCAddedMethod()) + { + return GetParallelMethodDescForEnC(this, pDefMD); + } #endif // FEATURE_METADATA_UPDATER - return GetMethodDescForSlot_NoThrow(pDefMD->GetSlot()); // TODO! We should probably use the throwing variant where possible + return GetMethodDescForSlot_NoThrow(pDefMD->GetSlot()); +} + +MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantLookup asyncVariantLookup) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + if (pDefMD->AsyncVariantKind() == asyncVariantLookup) + { + return GetParallelMethodDesc(pDefMD); } else { - // Slow path for finding the Async variant (or not-Async variant, if we start from Async one) + // Slow path for finding the matching async variant. // This could be optimized with some trickery around slot numbers, but doing so is ... confusing, so I'm not implementing this yet mdMethodDef tkMethod = pDefMD->GetMemberDef(); Module* mod = pDefMD->GetModule(); - bool isAsyncVariantMethod = pDefMD->IsAsyncVariantMethod(); - MethodTable::IntroducedMethodIterator it(this); for (; it.IsValid(); it.Next()) { MethodDesc* pMD = it.GetMethodDesc(); if (pMD->GetMemberDef() == tkMethod && pMD->GetModule() == mod - && pMD->IsAsyncVariantMethod() != isAsyncVariantMethod) + && (pMD->AsyncVariantKind() == asyncVariantLookup)) { return pMD; } @@ -8341,18 +8353,19 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType } bool differsByAsyncVariant = false; + _ASSERTE(!pMethodDecl->IsAsyncVariantMethod()); if (!pMethodDecl->HasSameMethodDefAs(pInterfaceMD)) { if (pMethodDecl->GetMemberDef() == pInterfaceMD->GetMemberDef() && pMethodDecl->GetModule() == pInterfaceMD->GetModule() && - pMethodDecl->IsAsyncVariantMethod() != pInterfaceMD->IsAsyncVariantMethod()) + pInterfaceMD->IsAsyncVariantMethod()) { differsByAsyncVariant = true; - pMethodDecl = pMethodDecl->GetAsyncOtherVariant(); + pMethodDecl = pMethodDecl->GetAsyncVariant(); if (verifyImplemented) { // if only asked to verify, return pMethodDecl as a success (not NULL) - // otherwise GetAsyncOtherVariant down below will trigger verifying again and we will keep coming here + // otherwise GetAsyncVariant down below will trigger verifying again and we will keep coming here _ASSERTE(pMethodDecl != NULL); return pMethodDecl; } @@ -8388,7 +8401,7 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType if (differsByAsyncVariant) { - pMethodImpl = pMethodImpl->GetAsyncOtherVariant(); + pMethodImpl = pMethodImpl->GetAsyncVariant(); } if (!verifyImplemented && instantiateMethodParameters) @@ -8401,7 +8414,6 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType /* allowInstParam */ FALSE, /* forceRemotableMethod */ FALSE, /* allowCreate */ TRUE, - AsyncVariantLookup::MatchingAsyncVariant, /* level */ level); } if (pMethodImpl != nullptr) diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 23cc148a09fe57..e5bb8869133220 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -1805,8 +1805,9 @@ class MethodTable // Returns MethodTable that GetRestoredSlot get its values from MethodTable * GetRestoredSlotMT(DWORD slot); - // Used to map methods on the same slot between instantiations. - MethodDesc * GetParallelMethodDesc(MethodDesc * pDefMD, AsyncVariantLookup asyncVariantLookup = (AsyncVariantLookup)0); + // Used to map methods between instantiations. + MethodDesc* GetParallelMethodDesc(MethodDesc* pDefMD); + MethodDesc* GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantLookup asyncVariantLookup); //------------------------------------------------------------------- // BoxedEntryPoint MethodDescs. diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index aba716ed8fabdc..9ae6f7197ddee0 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3338,7 +3338,6 @@ MethodTableBuilder::EnumerateClassMethods() // Create a new bmtMDMethod representing this method and add it to the // declared method list. // - bmtMDMethod *pDeclaredMethod = NULL; for (int insertCount = 0; insertCount < 2; insertCount++) { if (bmtMethod->m_cDeclaredMethods >= bmtMethod->m_cMaxDeclaredMethods) @@ -3391,8 +3390,6 @@ MethodTableBuilder::EnumerateClassMethods() pNewMethod->SetAsyncMethodFlags(AsyncMethodFlags::None); } } - - pDeclaredMethod = pNewMethod; } else { @@ -3487,9 +3484,6 @@ MethodTableBuilder::EnumerateClassMethods() asyncVariantType, implType); - pNewMethod->SetAsyncOtherVariant(pDeclaredMethod); - pDeclaredMethod->SetAsyncOtherVariant(pNewMethod); - #ifdef FEATURE_COMINTEROP // We only ever include one of the two async variants (whichever doesn't have the async calling convention) // Record an excluded method here in the COM VTable. @@ -5546,8 +5540,8 @@ MethodTableBuilder::PlaceVirtualMethods() } } -// Given an interface map entry, and a name+signature, compute the method on the interface -// that the name+signature corresponds to. Used by ProcessMethodImpls and ProcessInexactMethodImpls +// Given an interface map entry, and a name+signature+variantLookup, compute the method on the interface +// that the name+signature+variantLookup corresponds to. Used by ProcessMethodImpls and ProcessInexactMethodImpls // Always returns the first match that it finds. Affects the ambiguities in code:#ProcessInexactMethodImpls_Ambiguities MethodTableBuilder::bmtMethodHandle MethodTableBuilder::FindDeclMethodOnInterfaceEntry(bmtInterfaceEntry *pItfEntry, MethodSignature &declSig, AsyncVariantLookup variantLookup, bool searchForStaticMethods) @@ -5587,9 +5581,11 @@ MethodTableBuilder::FindDeclMethodOnInterfaceEntry(bmtInterfaceEntry *pItfEntry, } } - if (variantLookup == AsyncVariantLookup::AsyncOtherVariant && !declMethod.IsNull()) + if (variantLookup != AsyncVariantLookup::Ordinary && !declMethod.IsNull()) { bmtRTMethod* declRTMethod = declMethod.AsRTMethod(); + _ASSERTE(!declRTMethod->GetMethodDesc()->IsAsyncVariantMethod()); + // Other variant may not exist. For example we return Task and the base is generic and returns T. // Then we return Null. declMethod = {}; @@ -5600,7 +5596,7 @@ MethodTableBuilder::FindDeclMethodOnInterfaceEntry(bmtInterfaceEntry *pItfEntry, if ((slotDeclMethod->GetOwningType() == declRTMethod->GetOwningType()) && (slotDeclMethod->GetMethodDesc()->GetMethodTable() == declRTMethod->GetMethodDesc()->GetMethodTable()) && (slotDeclMethod->GetMethodDesc()->GetMemberDef() == declRTMethod->GetMethodDesc()->GetMemberDef()) && - (slotDeclMethod->GetMethodDesc()->IsAsyncVariantMethod() != declRTMethod->GetMethodDesc()->IsAsyncVariantMethod())) + (slotDeclMethod->GetMethodDesc()->AsyncVariantKind() == variantLookup)) { declMethod = slotIt->Decl(); break; @@ -5676,9 +5672,9 @@ MethodTableBuilder::ProcessInexactMethodImpls() continue; } - AsyncVariantLookup asyncVariantOfDeclToFind = !it->IsAsyncVariant() ? - AsyncVariantLookup::MatchingAsyncVariant : - AsyncVariantLookup::AsyncOtherVariant; + AsyncVariantLookup asyncVariantOfDeclToFind = it->IsAsyncVariant() ? + AsyncVariantLookup::Async : + AsyncVariantLookup::Ordinary; // If this method serves as the BODY of a MethodImpl specification, then // we should iterate all the MethodImpl's for this class and see just how many @@ -5821,9 +5817,9 @@ MethodTableBuilder::ProcessMethodImpls() continue; } - AsyncVariantLookup asyncVariantOfDeclToFind = !it->IsAsyncVariant() ? - AsyncVariantLookup::MatchingAsyncVariant : - AsyncVariantLookup::AsyncOtherVariant; + AsyncVariantLookup asyncVariantOfDeclToFind = it->IsAsyncVariant() ? + AsyncVariantLookup::Async : + AsyncVariantLookup::Ordinary; // If this method serves as the BODY of a MethodImpl specification, then // we should iterate all the MethodImpl's for this class and see just how many @@ -6008,7 +6004,7 @@ MethodTableBuilder::ProcessMethodImpls() declMethod = FindDeclMethodOnClassInHierarchy(it, pDeclMT, declSig, asyncVariantOfDeclToFind); } - if (declMethod.IsNull() && asyncVariantOfDeclToFind == AsyncVariantLookup::AsyncOtherVariant) + if (declMethod.IsNull() && asyncVariantOfDeclToFind == AsyncVariantLookup::Async) { // when implementing/overriding, we may see a Task-returning method // which matches a T-returning method in the interface/base, which would not have variants. @@ -6150,11 +6146,14 @@ MethodTableBuilder::bmtMethodHandle MethodTableBuilder::FindDeclMethodOnClassInH FALSE, iPass == 0 ? &newVisited : NULL)) { - if (variantLookup == AsyncVariantLookup::AsyncOtherVariant) + // We should find the ordinary variant first. + _ASSERTE(pCurMD->AsyncVariantKind() == AsyncVariantLookup::Ordinary); + + if (variantLookup != AsyncVariantLookup::Ordinary) { - if (pCurMD->HasAsyncOtherVariant()) + if (pCurMD->ReturnsTaskOrValueTask()) { - pCurMD = pCurMD->GetAsyncOtherVariant(); + pCurMD = pCurMD->GetAsyncVariant(); } else { diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h index 05d0d9c475fc8c..fece0cc7c6f83f 100644 --- a/src/coreclr/vm/methodtablebuilder.h +++ b/src/coreclr/vm/methodtablebuilder.h @@ -1112,9 +1112,6 @@ class MethodTableBuilder return m_asyncMethodFlags; } - bmtMDMethod * GetAsyncOtherVariant() const { return m_asyncOtherVariant; } - void SetAsyncOtherVariant(bmtMDMethod* pAsyncOtherVariant) { m_asyncOtherVariant = pAsyncOtherVariant; } - private: //----------------------------------------------------------------------------------------- bmtMDType * m_pOwningType; @@ -1126,7 +1123,6 @@ class MethodTableBuilder AsyncMethodFlags m_asyncMethodFlags; METHOD_IMPL_TYPE m_implType; // Whether or not the method is a methodImpl body MethodSignature m_methodSig; - bmtMDMethod* m_asyncOtherVariant = NULL; MethodDesc * m_pMD; // MethodDesc created and assigned to this method MethodDesc * m_pUnboxedMD; // Unboxing MethodDesc if this is a virtual method on a valuetype @@ -2027,11 +2023,7 @@ class MethodTableBuilder if ((*this)[i]->GetMethodSignature().GetToken() == tok) { auto result = (*this)[i]; - if (variantLookup == AsyncVariantLookup::AsyncOtherVariant) - { - return result->GetAsyncOtherVariant(); - } - else + if ((variantLookup == AsyncVariantLookup::Async) == result->IsAsyncVariant()) { return result; } diff --git a/src/coreclr/vm/runtimehandles.cpp b/src/coreclr/vm/runtimehandles.cpp index e0fddcae134d7f..da8093d38779e7 100644 --- a/src/coreclr/vm/runtimehandles.cpp +++ b/src/coreclr/vm/runtimehandles.cpp @@ -1947,7 +1947,7 @@ extern "C" MethodDesc* QCALLTYPE RuntimeMethodHandle_GetStubIfNeededSlow(MethodD if (pMethod->IsAsyncVariantMethod()) { // do not report async variants to reflection. - pMethod = pMethod->GetAsyncOtherVariant(/*allowInstParam*/ false); + pMethod = pMethod->GetOrdinaryVariant(/*allowInstParam*/ false); } TypeHandle instType = declaringTypeHandle.AsTypeHandle(); diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index b5161c3f042a66..7e46f7908200a6 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -2281,7 +2281,7 @@ BOOL AsyncThunkStubManager::TraceManager(Thread *thread, MethodDesc* pMD = NonVirtualEntry2MethodDesc(stubIP); if (pMD->IsAsyncThunkMethod()) { - MethodDesc* pOtherMD = pMD->GetAsyncOtherVariant(); + MethodDesc* pOtherMD = pMD->GetOrdinaryVariant(); _ASSERTE_MSG(pOtherMD != NULL, "ATSM::TraceManager: Async thunk has no non-thunk variant to step through to"); LOG((LF_CORDB, LL_INFO1000, "ATSM::TraceManager: Step through async thunk to target - %p\n", pOtherMD)); diff --git a/src/coreclr/vm/zapsig.cpp b/src/coreclr/vm/zapsig.cpp index 09d9800df3e143..d73e609a01936d 100644 --- a/src/coreclr/vm/zapsig.cpp +++ b/src/coreclr/vm/zapsig.cpp @@ -917,7 +917,7 @@ MethodDesc *ZapSig::DecodeMethod(ModuleBase *pInfoModule, // This must be called even if nargs == 0, in order to create an instantiating - // stub for static methods in generic classees if needed, also for BoxedEntryPointStubs + // stub for static methods in generic classes if needed, also for BoxedEntryPointStubs // in non-generic structs. BOOL isInstantiatingStub = (methodFlags & ENCODE_METHOD_SIG_InstantiatingStub); BOOL isUnboxingStub = (methodFlags & ENCODE_METHOD_SIG_UnboxingStub); @@ -927,9 +927,9 @@ MethodDesc *ZapSig::DecodeMethod(ModuleBase *pInfoModule, isUnboxingStub, inst, !(isInstantiatingStub || isUnboxingStub) && !actualOwnerRequired, + isAsyncVariant ? AsyncVariantLookup::Async : AsyncVariantLookup::Ordinary, actualOwnerRequired, - TRUE, - isAsyncVariant == pMethod->IsAsyncVariantMethod() ? AsyncVariantLookup::MatchingAsyncVariant : AsyncVariantLookup::AsyncOtherVariant); + TRUE); if (methodFlags & ENCODE_METHOD_SIG_Constrained) { From 0d1a92224b929e2011fc29d40a4722190d1e2ffe Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 18 Mar 2026 13:17:47 -0700 Subject: [PATCH 03/21] MatchesAsyncVariantLookup --- src/coreclr/vm/genmeth.cpp | 6 +++--- src/coreclr/vm/method.hpp | 14 +++----------- src/coreclr/vm/methodtable.cpp | 4 ++-- src/coreclr/vm/methodtablebuilder.cpp | 4 ++-- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index 88da8fcb0528fb..62f413ada24e67 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -788,7 +788,7 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, methodInst.IsEmpty() && !forceBoxedEntryPoint && !pDefMD->IsUnboxingStub() && - pDefMD->AsyncVariantKind() == asyncVariantLookup) + pDefMD->MatchesAsyncVariantLookup(asyncVariantLookup)) { // Make sure that pDefMD->GetMethodTable() and pExactMT are related types even // if we took the fast path. @@ -819,7 +819,7 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, if (pDefMD->HasClassOrMethodInstantiation() || !methodInst.IsEmpty() || - pDefMD->AsyncVariantKind() != asyncVariantLookup) + !pDefMD->MatchesAsyncVariantLookup(asyncVariantLookup)) { // General checks related to generics: arity (if any) must match and generic method // instantiation (if any) must be well-formed. @@ -848,7 +848,7 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD, && (allowInstParam || !pMDescInCanonMT->RequiresInstArg()) && (forceBoxedEntryPoint == pMDescInCanonMT->IsUnboxingStub()) && (!forceRemotableMethod || !pMDescInCanonMT->IsInterface() || !pMDescInCanonMT->GetMethodTable()->IsSharedByGenericInstantiations()) - && (pMDescInCanonMT->AsyncVariantKind() == asyncVariantLookup)) + && (pMDescInCanonMT->MatchesAsyncVariantLookup(asyncVariantLookup))) { RETURN(pMDescInCanonMT); } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 5aae4c8c5076d0..6c66607447b3f0 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1722,7 +1722,7 @@ class MethodDesc forceBoxedEntryPoint, methodInst, allowInstParam, - pPrimaryMD->AsyncVariantKind(), + pPrimaryMD->IsAsyncVariantMethod() ? AsyncVariantLookup::Async : AsyncVariantLookup::Ordinary, forceRemotableMethod, allowCreate, level); @@ -2034,19 +2034,11 @@ class MethodDesc return hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant); } - inline AsyncVariantLookup AsyncVariantKind() const + inline bool MatchesAsyncVariantLookup(AsyncVariantLookup lookup) const { LIMITED_METHOD_DAC_CONTRACT; - if (HasAsyncMethodData()) - { - AsyncMethodFlags asyncFlags = GetAddrOfAsyncMethodData()->flags; - if (hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant)) - { - return AsyncVariantLookup::Async; - } - } - return AsyncVariantLookup::Ordinary; + return IsAsyncVariantMethod() == (lookup == AsyncVariantLookup::Async); } // Is this an Async variant method for a method that diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 493d90e4a6ac96..79f080cc339280 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -7948,7 +7948,7 @@ MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantL } CONTRACTL_END; - if (pDefMD->AsyncVariantKind() == asyncVariantLookup) + if (pDefMD->MatchesAsyncVariantLookup(asyncVariantLookup)) { return GetParallelMethodDesc(pDefMD); } @@ -7964,7 +7964,7 @@ MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantL MethodDesc* pMD = it.GetMethodDesc(); if (pMD->GetMemberDef() == tkMethod && pMD->GetModule() == mod - && (pMD->AsyncVariantKind() == asyncVariantLookup)) + && (pMD->MatchesAsyncVariantLookup(asyncVariantLookup))) { return pMD; } diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 9ae6f7197ddee0..2c5810e2c25e7f 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -5596,7 +5596,7 @@ MethodTableBuilder::FindDeclMethodOnInterfaceEntry(bmtInterfaceEntry *pItfEntry, if ((slotDeclMethod->GetOwningType() == declRTMethod->GetOwningType()) && (slotDeclMethod->GetMethodDesc()->GetMethodTable() == declRTMethod->GetMethodDesc()->GetMethodTable()) && (slotDeclMethod->GetMethodDesc()->GetMemberDef() == declRTMethod->GetMethodDesc()->GetMemberDef()) && - (slotDeclMethod->GetMethodDesc()->AsyncVariantKind() == variantLookup)) + (slotDeclMethod->GetMethodDesc()->MatchesAsyncVariantLookup(variantLookup))) { declMethod = slotIt->Decl(); break; @@ -6147,7 +6147,7 @@ MethodTableBuilder::bmtMethodHandle MethodTableBuilder::FindDeclMethodOnClassInH iPass == 0 ? &newVisited : NULL)) { // We should find the ordinary variant first. - _ASSERTE(pCurMD->AsyncVariantKind() == AsyncVariantLookup::Ordinary); + _ASSERTE(pCurMD->MatchesAsyncVariantLookup(AsyncVariantLookup::Ordinary)); if (variantLookup != AsyncVariantLookup::Ordinary) { From dc867f67fb809a66048e2a820c845f521a91f3d7 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 18 Mar 2026 18:45:43 -0700 Subject: [PATCH 04/21] moved assert --- src/coreclr/vm/methodtablebuilder.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 2c5810e2c25e7f..f251075ac3de94 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -5581,11 +5581,12 @@ MethodTableBuilder::FindDeclMethodOnInterfaceEntry(bmtInterfaceEntry *pItfEntry, } } + // declSig is for an ordinary method, we should not find an async variant. + _ASSERTE(declMethod.IsNull() || !declMethod.GetMethodDesc()->IsAsyncVariantMethod()); + if (variantLookup != AsyncVariantLookup::Ordinary && !declMethod.IsNull()) { bmtRTMethod* declRTMethod = declMethod.AsRTMethod(); - _ASSERTE(!declRTMethod->GetMethodDesc()->IsAsyncVariantMethod()); - // Other variant may not exist. For example we return Task and the base is generic and returns T. // Then we return Null. declMethod = {}; From 3fa064ba82bfe5260e82ec5132269f41cd46b8d2 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 20 Mar 2026 10:13:58 -0700 Subject: [PATCH 05/21] EmitReturnDroppingThunk --- src/coreclr/vm/asyncthunks.cpp | 222 ++++++++++++++------------ src/coreclr/vm/method.cpp | 7 +- src/coreclr/vm/method.hpp | 32 +++- src/coreclr/vm/methodtablebuilder.cpp | 70 ++++++-- 4 files changed, 212 insertions(+), 119 deletions(-) diff --git a/src/coreclr/vm/asyncthunks.cpp b/src/coreclr/vm/asyncthunks.cpp index 8d67242d22c4bb..30541cc83b94d0 100644 --- a/src/coreclr/vm/asyncthunks.cpp +++ b/src/coreclr/vm/asyncthunks.cpp @@ -25,7 +25,27 @@ bool MethodDesc::TryGenerateAsyncThunk(DynamicResolver** resolver, COR_ILMETHOD_ return false; } - MethodDesc *pAsyncOtherVariant = IsAsyncVariantMethod() ? this->GetOrdinaryVariant() : this->GetAsyncVariant(); + MethodDesc* pAsyncOtherVariant = nullptr; + if (!IsAsyncMethod()) + { + // a non-async thunk is implemented in terms of the async variant which has user code + pAsyncOtherVariant = this->GetAsyncVariant(); + } + else + { + if (!IsReturnDroppingThunk()) + { + // an async thunk is implemented in terms of non-async variant + pAsyncOtherVariant = this->GetOrdinaryVariant(); + } + else + { + // this is a special void-returning async variant that calls + // the normal async variant and drops the result + pAsyncOtherVariant = this->GetAsyncVariant(); + } + } + _ASSERTE(!IsWrapperStub() && !pAsyncOtherVariant->IsWrapperStub()); MetaSig msig(this); @@ -38,13 +58,20 @@ bool MethodDesc::TryGenerateAsyncThunk(DynamicResolver** resolver, COR_ILMETHOD_ pAsyncOtherVariant, (ILStubLinkerFlags)ILSTUB_LINKER_FLAG_NONE); - if (IsAsyncMethod()) + if (!IsAsyncMethod()) { - EmitAsyncMethodThunk(pAsyncOtherVariant, msig, &sl); + EmitTaskReturningThunk(pAsyncOtherVariant, msig, &sl); } else { - EmitTaskReturningThunk(pAsyncOtherVariant, msig, &sl); + if (IsReturnDroppingThunk()) + { + EmitReturnDroppingThunk(pAsyncOtherVariant, msig, &sl); + } + else + { + EmitAsyncMethodThunk(pAsyncOtherVariant, msig, &sl); + } } NewHolder ilResolver = new ILStubResolver(); @@ -132,60 +159,7 @@ void MethodDesc::EmitTaskReturningThunk(MethodDesc* pAsyncCallVariant, MetaSig& pCode->EmitLDARG(localArg++); } - int token; - _ASSERTE(!pAsyncCallVariant->IsWrapperStub()); - if (pAsyncCallVariant->HasClassOrMethodInstantiation()) - { - // For generic code emit generic signatures. - int typeSigToken = mdTokenNil; - if (pAsyncCallVariant->HasClassInstantiation()) - { - SigBuilder typeSigBuilder; - typeSigBuilder.AppendElementType(ELEMENT_TYPE_GENERICINST); - typeSigBuilder.AppendElementType(ELEMENT_TYPE_INTERNAL); - // TODO: (async) Encoding potentially shared method tables in - // signatures of tokens seems odd, but this hits assert - // with the typical method table. - typeSigBuilder.AppendPointer(pAsyncCallVariant->GetMethodTable()); - DWORD numClassTypeArgs = pAsyncCallVariant->GetNumGenericClassArgs(); - typeSigBuilder.AppendData(numClassTypeArgs); - for (DWORD i = 0; i < numClassTypeArgs; ++i) - { - typeSigBuilder.AppendElementType(ELEMENT_TYPE_VAR); - typeSigBuilder.AppendData(i); - } - - DWORD typeSigLen; - PCCOR_SIGNATURE typeSig = (PCCOR_SIGNATURE)typeSigBuilder.GetSignature(&typeSigLen); - typeSigToken = pCode->GetSigToken(typeSig, typeSigLen); - } - - if (pAsyncCallVariant->HasMethodInstantiation()) - { - SigBuilder methodSigBuilder; - DWORD numMethodTypeArgs = pAsyncCallVariant->GetNumGenericMethodArgs(); - methodSigBuilder.AppendByte(IMAGE_CEE_CS_CALLCONV_GENERICINST); - methodSigBuilder.AppendData(numMethodTypeArgs); - for (DWORD i = 0; i < numMethodTypeArgs; ++i) - { - methodSigBuilder.AppendElementType(ELEMENT_TYPE_MVAR); - methodSigBuilder.AppendData(i); - } - - DWORD sigLen; - PCCOR_SIGNATURE sig = (PCCOR_SIGNATURE)methodSigBuilder.GetSignature(&sigLen); - int methodSigToken = pCode->GetSigToken(sig, sigLen); - token = pCode->GetToken(pAsyncCallVariant, typeSigToken, methodSigToken); - } - else - { - token = pCode->GetToken(pAsyncCallVariant, typeSigToken); - } - } - else - { - token = pCode->GetToken(pAsyncCallVariant); - } + int token = GetTokenForThunkTarget(pCode, pAsyncCallVariant); pCode->EmitCALL(token, localArg, logicalResultLocal != UINT_MAX ? 1 : 0); @@ -429,47 +403,15 @@ int MethodDesc::GetTokenForGenericTypeMethodCallWithAsyncReturnType(ILCodeStream return pCode->GetToken(md, typeSigToken); } -// Provided a Task-returning method, emits an async wrapper. -// The emitted code matches method EmitAsyncMethodThunk in the Managed Type System. -void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig& msig, ILStubLinker* pSL) +int MethodDesc::GetTokenForThunkTarget(ILCodeStream* pCode, MethodDesc* md) { - _ASSERTE(!pTaskReturningVariant->IsAsyncThunkMethod()); - _ASSERTE(!pTaskReturningVariant->IsVoid()); - - // Implement IL that is effectively the following: - // { - // Task task = other(arg); - // if (!task.IsCompleted) - // { - // // Magic function which will suspend the current run of async methods - // AsyncHelpers.TransparentAwait(task); - // } - // return AsyncHelpers.CompletedTaskResult(task); - // } - - // For ValueTask: - - // { - // ValueTask vt = other(arg); - // if (!vt.IsCompleted) - // { - // taskOrNotifier = vt.AsTaskOrNotifier() - - // // Magic function which will suspend the current run of async methods - // AsyncHelpers.TransparentAwait(taskOrNotifier); - // } - - // return vt.Result/vt.ThrowIfCompletedUnsuccessfully(); - // } - ILCodeStream* pCode = pSL->NewCodeStream(ILStubLinker::kDispatch); - - int userFuncToken; - _ASSERTE(!pTaskReturningVariant->IsWrapperStub()); - if (pTaskReturningVariant->HasClassOrMethodInstantiation()) + int token; + _ASSERTE(!md->IsWrapperStub()); + if (md->HasClassOrMethodInstantiation()) { // For generic code emit generic signatures. int typeSigToken = mdTokenNil; - if (pTaskReturningVariant->HasClassInstantiation()) + if (md->HasClassInstantiation()) { SigBuilder typeSigBuilder; typeSigBuilder.AppendElementType(ELEMENT_TYPE_GENERICINST); @@ -477,8 +419,8 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig // TODO: (async) Encoding potentially shared method tables in // signatures of tokens seems odd, but this hits assert // with the typical method table. - typeSigBuilder.AppendPointer(pTaskReturningVariant->GetMethodTable()); - DWORD numClassTypeArgs = pTaskReturningVariant->GetNumGenericClassArgs(); + typeSigBuilder.AppendPointer(md->GetMethodTable()); + DWORD numClassTypeArgs = md->GetNumGenericClassArgs(); typeSigBuilder.AppendData(numClassTypeArgs); for (DWORD i = 0; i < numClassTypeArgs; ++i) { @@ -491,10 +433,10 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig typeSigToken = pCode->GetSigToken(typeSig, typeSigLen); } - if (pTaskReturningVariant->HasMethodInstantiation()) + if (md->HasMethodInstantiation()) { SigBuilder methodSigBuilder; - DWORD numMethodTypeArgs = pTaskReturningVariant->GetNumGenericMethodArgs(); + DWORD numMethodTypeArgs = md->GetNumGenericMethodArgs(); methodSigBuilder.AppendByte(IMAGE_CEE_CS_CALLCONV_GENERICINST); methodSigBuilder.AppendData(numMethodTypeArgs); for (DWORD i = 0; i < numMethodTypeArgs; ++i) @@ -506,18 +448,57 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig DWORD sigLen; PCCOR_SIGNATURE sig = (PCCOR_SIGNATURE)methodSigBuilder.GetSignature(&sigLen); int methodSigToken = pCode->GetSigToken(sig, sigLen); - userFuncToken = pCode->GetToken(pTaskReturningVariant, typeSigToken, methodSigToken); + token = pCode->GetToken(md, typeSigToken, methodSigToken); } else { - userFuncToken = pCode->GetToken(pTaskReturningVariant, typeSigToken); + token = pCode->GetToken(md, typeSigToken); } } else { - userFuncToken = pCode->GetToken(pTaskReturningVariant); + token = pCode->GetToken(md); } + return token; +} + +// Provided a Task-returning method, emits an async wrapper. +// The emitted code matches method EmitAsyncMethodThunk in the Managed Type System. +void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig& msig, ILStubLinker* pSL) +{ + _ASSERTE(!pTaskReturningVariant->IsAsyncThunkMethod()); + _ASSERTE(!pTaskReturningVariant->IsVoid()); + + // Implement IL that is effectively the following: + // { + // Task task = other(arg); + // if (!task.IsCompleted) + // { + // // Magic function which will suspend the current run of async methods + // AsyncHelpers.TransparentAwait(task); + // } + // return AsyncHelpers.CompletedTaskResult(task); + // } + + // For ValueTask: + + // { + // ValueTask vt = other(arg); + // if (!vt.IsCompleted) + // { + // taskOrNotifier = vt.AsTaskOrNotifier() + + // // Magic function which will suspend the current run of async methods + // AsyncHelpers.TransparentAwait(taskOrNotifier); + // } + + // return vt.Result/vt.ThrowIfCompletedUnsuccessfully(); + // } + ILCodeStream* pCode = pSL->NewCodeStream(ILStubLinker::kDispatch); + + int token = GetTokenForThunkTarget(pCode, pTaskReturningVariant); + DWORD localArg = 0; if (msig.HasThis()) { @@ -532,11 +513,11 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig if (pTaskReturningVariant->IsAbstract()) { _ASSERTE(pTaskReturningVariant->IsCLRToCOMCall()); - pCode->EmitCALLVIRT(userFuncToken, localArg, 1); + pCode->EmitCALLVIRT(token, localArg, 1); } else { - pCode->EmitCALL(userFuncToken, localArg, 1); + pCode->EmitCALL(token, localArg, 1); } TypeHandle thLogicalRetType = msig.GetRetTypeHandleThrowing(); @@ -637,3 +618,36 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig pCode->EmitRET(); } } + +// Provided an async variant, emits an async wrapper that drops the returned value. +// Used in the covariant return scenario. +void MethodDesc::EmitReturnDroppingThunk(MethodDesc* pAsyncOtherVariant, MetaSig& msig, ILStubLinker* pSL) +{ + _ASSERTE(pAsyncOtherVariant->IsAsyncVariantMethod()); + + _ASSERTE(!pAsyncOtherVariant->IsVoid()); + _ASSERTE(pAsyncOtherVariant->IsVirtual()); + _ASSERTE(this->IsVoid()); + _ASSERTE(this->IsVirtual()); + + // Implement IL that is effectively the following: + // { + // this.other(arg); // CALLVIRT + // return; + // } + ILCodeStream* pCode = pSL->NewCodeStream(ILStubLinker::kDispatch); + int token = GetTokenForThunkTarget(pCode, pAsyncOtherVariant); + + DWORD localArg = 0; + pCode->EmitLDARG(localArg++); + for (UINT iArg = 0; iArg < msig.NumFixedArgs(); iArg++) + { + pCode->EmitLDARG(localArg++); + } + + // other(arg) + pCode->EmitCALLVIRT(token, localArg, 1); + // return; + pCode->EmitPOP(); + pCode->EmitRET(); +} diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 9295fb3d38e534..6e4772c539db5c 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2390,7 +2390,7 @@ bool IsTypeDefOrRefImplementedInSystemModule(Module* pModule, mdToken tk) return false; } -MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG* offsetOfAsyncDetails, bool *isValueTask) +MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG* offsetOfAsyncDetails, ULONG* elementTypeLength, bool *isValueTask) { PCCOR_SIGNATURE initialSig = sig.GetPtr(); uint32_t data; @@ -2433,7 +2433,12 @@ MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG if ((strcmp(name, *isValueTask ? "ValueTask`1" : "Task`1") == 0) && strcmp(_namespace, "System.Threading.Tasks") == 0) { if (IsTypeDefOrRefImplementedInSystemModule(pModule, tk)) + { + PCCOR_SIGNATURE elementStart = sig.GetPtr(); + sig.SkipExactlyOne(); + *elementTypeLength = (ULONG)(sig.GetPtr() - elementStart); return MethodReturnKind::GenericTaskReturningMethod; + } } } } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 6c66607447b3f0..5f9cbc44379e9b 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -76,6 +76,8 @@ enum class AsyncMethodFlags IsAsyncVariantForValueTask = 8, // Method has synthetic body, which forwards to the other variant. Thunk = 16, + // A special thunk to drop return value in covariant return scenario + ReturnDroppingThunk = 32, // The rest of the methods that are not in any of the above groups. // Such methods are not interesting to the Runtime Async feature. // Note: Generic T-returning methods are classified as "None", even if T could be a Task. @@ -277,7 +279,7 @@ enum class MethodReturnKind }; bool IsTypeDefOrRefImplementedInSystemModule(Module* pModule, mdToken tk); -MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG* offsetOfAsyncDetails, bool *isValueTask); +MethodReturnKind ClassifyMethodReturnKind(SigPointer sig, Module* pModule, ULONG* offsetOfAsyncDetails, ULONG* elementTypeLength, bool *isValueTask); inline bool IsTaskReturning(MethodReturnKind input) { @@ -1747,7 +1749,7 @@ class MethodDesc MethodDesc* GetAsyncVariant(BOOL allowInstParam = TRUE) { - _ASSERT(!IsAsyncVariantMethod()); + _ASSERT(!IsAsyncVariantMethod() || IsReturnDroppingThunk()); return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, TRUE); } @@ -2034,11 +2036,33 @@ class MethodDesc return hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant); } + inline bool IsReturnDroppingThunk() const + { + LIMITED_METHOD_DAC_CONTRACT; + if (!HasAsyncMethodData()) + return false; + + AsyncMethodFlags asyncFlags = GetAddrOfAsyncMethodData()->flags; + return hasAsyncFlags(asyncFlags, AsyncMethodFlags::ReturnDroppingThunk); + } + inline bool MatchesAsyncVariantLookup(AsyncVariantLookup lookup) const { LIMITED_METHOD_DAC_CONTRACT; - return IsAsyncVariantMethod() == (lookup == AsyncVariantLookup::Async); + if (lookup == AsyncVariantLookup::Ordinary) + return !IsAsyncVariantMethod(); + + if (lookup == AsyncVariantLookup::Async) + { + if (!HasAsyncMethodData()) + return false; + + AsyncMethodFlags asyncFlags = GetAddrOfAsyncMethodData()->flags; + return hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant) && !hasAsyncFlags(asyncFlags, AsyncMethodFlags::ReturnDroppingThunk); + } + + return false; } // Is this an Async variant method for a method that @@ -2249,7 +2273,9 @@ class MethodDesc bool TryGenerateUnsafeAccessor(DynamicResolver** resolver, COR_ILMETHOD_DECODER** methodILDecoder); void EmitTaskReturningThunk(MethodDesc* pAsyncCallVariant, MetaSig& thunkMsig, ILStubLinker* pSL); void EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig& msig, ILStubLinker* pSL); + void EmitReturnDroppingThunk(MethodDesc* pAsyncOtherVariant, MetaSig& msig, ILStubLinker* pSL); SigPointer GetAsyncThunkResultTypeSig(); + int GetTokenForThunkTarget(ILCodeStream* pCode, MethodDesc* md); int GetTokenForGenericMethodCallWithAsyncReturnType(ILCodeStream* pCode, MethodDesc* md); int GetTokenForGenericTypeMethodCallWithAsyncReturnType(ILCodeStream* pCode, MethodDesc* md); public: diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index f251075ac3de94..ee2406ed1ceb4d 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -2698,7 +2698,8 @@ MethodTableBuilder::EnumerateClassMethods() // as each async method may have two variants. // The method count is typically a modest number though. // We will reserve twice the size for the builder, up to the max, just in case. - DWORD cMethUpperBound = cMethAndGaps * 2; + // TODO: VS "3" only if has covariant returns (or methodimpls?), otherwise 2. + DWORD cMethUpperBound = cMethAndGaps * 3; if ((DWORD)MAX_SLOT_INDEX <= cMethUpperBound) { cMethUpperBound = MAX_SLOT_INDEX - 1; @@ -2782,10 +2783,11 @@ MethodTableBuilder::EnumerateClassMethods() SigParser sig(pMemberSignature, cMemberSignature); ULONG offsetOfAsyncDetails = 0; + ULONG elementTypeLength = 0; bool returnsValueTask = false; MethodReturnKind returnKind = IsDelegate() ? MethodReturnKind::NormalMethod : - ClassifyMethodReturnKind(sig, GetModule(), &offsetOfAsyncDetails, &returnsValueTask); + ClassifyMethodReturnKind(sig, GetModule(), &offsetOfAsyncDetails, &elementTypeLength, &returnsValueTask); bool hasGenericMethodArgsComputed = false; bool hasGenericMethodArgs = this->GetModule()->m_pMethodIsGenericMap->IsGeneric(tok, &hasGenericMethodArgsComputed); @@ -3338,7 +3340,7 @@ MethodTableBuilder::EnumerateClassMethods() // Create a new bmtMDMethod representing this method and add it to the // declared method list. // - for (int insertCount = 0; insertCount < 2; insertCount++) + for (int insertCount = 0; insertCount < 3; insertCount++) { if (bmtMethod->m_cDeclaredMethods >= bmtMethod->m_cMaxDeclaredMethods) { @@ -3410,20 +3412,37 @@ MethodTableBuilder::EnumerateClassMethods() if (!IsMiAsync(dwImplFlags)) asyncFlags |= AsyncMethodFlags::Thunk; + if (insertCount == 2) + asyncFlags |= (AsyncMethodFlags::Thunk | AsyncMethodFlags::ReturnDroppingThunk); + // Here we construct the signature of async call variant given its task-returning counterpart. // 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 tokenLen = 0; - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + ULONG taskTokenLen = 0; + + if (insertCount == 2) + { + // 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; - tokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - taskTypePrefixSize = 1 + tokenLen; // E_T_CLASS/E_T_VALUETYPE + taskTypePrefixSize = 1 + taskTokenLen; // E_T_CLASS/E_T_VALUETYPE taskTypePrefixReplacementSize = 1; // ELEMENT_TYPE_VOID cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; @@ -3434,9 +3453,9 @@ MethodTableBuilder::EnumerateClassMethods() // ". . . tk . . . Method(args);" taskTokenOffsetFromAsyncDetailsOffset = 2; - tokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); + taskTokenLen = CorSigUncompressedDataSize(&pMemberSignature[offsetOfAsyncDetails + taskTokenOffsetFromAsyncDetailsOffset]); - taskTypePrefixSize = 2 + tokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 + taskTypePrefixSize = 2 + taskTokenLen + 1; // E_T_GENERICINST E_T_CLASS/E_T_VALUETYPE 1 taskTypePrefixReplacementSize = 0; cAsyncThunkMemberSignature = cMemberSignature - taskTypePrefixSize + taskTypePrefixReplacementSize; @@ -3458,7 +3477,7 @@ MethodTableBuilder::EnumerateClassMethods() _ASSERTE((cMemberSignature - originalRemainingSigOffset) == (cAsyncThunkMemberSignature - newRemainingSigOffset)); memcpy(pNewMemberSignature + newRemainingSigOffset, pMemberSignature + originalRemainingSigOffset, cMemberSignature - originalRemainingSigOffset); - if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod) + if (returnKind == MethodReturnKind::NonGenericTaskReturningMethod || insertCount == 2) { pNewMemberSignature[newRemainingSigOffset - 1] = ELEMENT_TYPE_VOID; } @@ -3516,6 +3535,34 @@ MethodTableBuilder::EnumerateClassMethods() { break; } + + if (insertCount == 1) + { + // if we return Task + if (!returnsValueTask && + returnKind == MethodReturnKind::GenericTaskReturningMethod && + !this->IsValueClass() && + !IsMdAbstract(dwMemberAttrs) && + IsMdVirtual(dwMemberAttrs)) + { + //// also this is a methodimpl + //if (pNewMethod->GetMethodImplType() == METHOD_IMPL) + //{ + // // check if "need to check for covariance" and add void returning variant + + // // TODO: base may be Object-returning, then we do not care. Probably ok. + + // // for starters add void returning variant always. + + // // void returning should classify as IsAsyncVariant. It is always a thunk. + + // // when matching to base we should either require return match for variants or skip/overwrite when see void returning. + //} + continue; + } + + break; + } } } @@ -6005,7 +6052,8 @@ MethodTableBuilder::ProcessMethodImpls() declMethod = FindDeclMethodOnClassInHierarchy(it, pDeclMT, declSig, asyncVariantOfDeclToFind); } - if (declMethod.IsNull() && asyncVariantOfDeclToFind == AsyncVariantLookup::Async) + if (asyncVariantOfDeclToFind == AsyncVariantLookup::Async && + (declMethod.IsNull() || !MethodSignature::SignaturesEquivalent(declMethod.GetMethodSignature(), it->GetMethodSignature(), FALSE))) { // when implementing/overriding, we may see a Task-returning method // which matches a T-returning method in the interface/base, which would not have variants. From 77affa2ee951946279d3c3717bd66c70ebc30e5d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 20 Mar 2026 18:43:46 -0700 Subject: [PATCH 06/21] reenable covariant compat check for async variants --- src/coreclr/vm/class.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index c532eae900b5b4..dbc9b2514eb188 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -1366,12 +1366,6 @@ void ClassLoader::ValidateMethodsWithCovariantReturnTypes(MethodTable* pMT) continue; } MethodDesc* pMD = pMT->GetMethodDescForSlot(i); - - // Skip validation for async variant methods, as they have different signatures by design - // to support the async calling convention - if (pMD->IsAsyncVariantMethod()) - continue; - MethodDesc* pParentMD = pParentMT->GetMethodDescForSlot(i); if (pMD == pParentMD) From fc6d768bb247351d5d2599d89b497627095a0dda Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 21 Mar 2026 15:20:30 -0700 Subject: [PATCH 07/21] narrow cases when we add an extra variant --- src/coreclr/vm/methodtablebuilder.cpp | 46 ++++++++++++--------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index ee2406ed1ceb4d..855a4a7b8bfd81 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -2697,9 +2697,9 @@ MethodTableBuilder::EnumerateClassMethods() // In a worst case the number of declared methods can double // as each async method may have two variants. // The method count is typically a modest number though. - // We will reserve twice the size for the builder, up to the max, just in case. - // TODO: VS "3" only if has covariant returns (or methodimpls?), otherwise 2. - DWORD cMethUpperBound = cMethAndGaps * 3; + // If we have covariant overrides, then overrides from Task -> Task we will need 3 method descs. + // Reserve the space conservatively, up to the max, for the worst case scenario. + DWORD cMethUpperBound = cMethAndGaps * (bmtMetaData->fHasCovariantOverride ? 3 : 2); if ((DWORD)MAX_SLOT_INDEX <= cMethUpperBound) { cMethUpperBound = MAX_SLOT_INDEX - 1; @@ -3395,7 +3395,7 @@ MethodTableBuilder::EnumerateClassMethods() } else { - // Second pass, add the async variant. + // Extra pass, add an async variant. ULONG cAsyncThunkMemberSignature; ULONG taskTokenOffsetFromAsyncDetailsOffset; @@ -3423,6 +3423,11 @@ MethodTableBuilder::EnumerateClassMethods() if (insertCount == 2) { + // This is a rare case when we need two async variants and this is the second one. + // The need arises when a Task-returning method has a Task returning virtual override. + // We need an extra void-returning thunk that can override the void-returning async variant in the base, + // while the thunk's implementation simply forwards to the T-returning async variant and ignores the return. + // from ". . . Task . . . Method(args);" we construct // ". . . void . . . Method(args);" @@ -3536,32 +3541,21 @@ MethodTableBuilder::EnumerateClassMethods() break; } + // In rare cases we need a void-returning async variant in addition to the T-returning one. + // It is ok to add a void-returning thunk and end up not using it, but we do not want waste. + // Thus we try to filter closer to the cases when the thunk is most certainly will be used. if (insertCount == 1) { - // if we return Task - if (!returnsValueTask && - returnKind == MethodReturnKind::GenericTaskReturningMethod && - !this->IsValueClass() && - !IsMdAbstract(dwMemberAttrs) && - IsMdVirtual(dwMemberAttrs)) + if (implType != METHOD_IMPL || + returnsValueTask || + returnKind != MethodReturnKind::GenericTaskReturningMethod || + this->IsValueClass() || + !IsMdVirtual(dwMemberAttrs) || + !bmtMetaData->fHasCovariantOverride) { - //// also this is a methodimpl - //if (pNewMethod->GetMethodImplType() == METHOD_IMPL) - //{ - // // check if "need to check for covariance" and add void returning variant - - // // TODO: base may be Object-returning, then we do not care. Probably ok. - - // // for starters add void returning variant always. - - // // void returning should classify as IsAsyncVariant. It is always a thunk. - - // // when matching to base we should either require return match for variants or skip/overwrite when see void returning. - //} - continue; + // No need for another variant + break; } - - break; } } } From 93eb2a38fff9deec61afa286956615677b15d783 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 21 Mar 2026 17:16:32 -0700 Subject: [PATCH 08/21] comments and cleanups --- src/coreclr/vm/method.hpp | 3 +++ src/coreclr/vm/methodtable.h | 3 ++- src/coreclr/vm/methodtablebuilder.cpp | 23 ++++++++++++++----- .../covariant-return/covariant-returns.csproj | 4 ---- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 5f9cbc44379e9b..b41c3cf58f204a 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2058,6 +2058,9 @@ class MethodDesc if (!HasAsyncMethodData()) return false; + // Note: AsyncVariantLookup::Async only matches regular async variants. ReturnDroppingThunk intentionally + // does not match any lookups. Noone should call ReturnDroppingThunk directly. The only way it gets + // invoked is when it adds itself as a virtual override to a regular async variant. AsyncMethodFlags asyncFlags = GetAddrOfAsyncMethodData()->flags; return hasAsyncFlags(asyncFlags, AsyncMethodFlags::IsAsyncVariant) && !hasAsyncFlags(asyncFlags, AsyncMethodFlags::ReturnDroppingThunk); } diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index e5bb8869133220..b4bd831b87f604 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -1805,8 +1805,9 @@ class MethodTable // Returns MethodTable that GetRestoredSlot get its values from MethodTable * GetRestoredSlotMT(DWORD slot); - // Used to map methods between instantiations. + // Used to map to "the same" method between instantiations. MethodDesc* GetParallelMethodDesc(MethodDesc* pDefMD); + // Maps methods between instantiations + filters/adjusts the result according to the lookup. MethodDesc* GetParallelMethodDesc(MethodDesc* pDefMD, AsyncVariantLookup asyncVariantLookup); //------------------------------------------------------------------- diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 855a4a7b8bfd81..b96f4712dbc524 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3425,8 +3425,8 @@ MethodTableBuilder::EnumerateClassMethods() { // This is a rare case when we need two async variants and this is the second one. // The need arises when a Task-returning method has a Task returning virtual override. - // We need an extra void-returning thunk that can override the void-returning async variant in the base, - // while the thunk's implementation simply forwards to the T-returning async variant and ignores the return. + // We need an extra void-returning thunk that can override the void-returning async variant in the base. + // The thunk's implementation simply calls the T-returning async variant and ignores the return. // from ". . . Task . . . Method(args);" we construct // ". . . void . . . Method(args);" @@ -6047,11 +6047,22 @@ MethodTableBuilder::ProcessMethodImpls() } if (asyncVariantOfDeclToFind == AsyncVariantLookup::Async && - (declMethod.IsNull() || !MethodSignature::SignaturesEquivalent(declMethod.GetMethodSignature(), it->GetMethodSignature(), FALSE))) + (declMethod.IsNull() || + !MethodSignature::SignaturesEquivalent(declMethod.GetMethodSignature(), it->GetMethodSignature(), FALSE))) { - // when implementing/overriding, we may see a Task-returning method - // which matches a T-returning method in the interface/base, which would not have variants. - // in such case the async variant of the Task-returning method does not implement/override anything. + // There are two scenarios when an async variant may not find a base to override: + // + // 1. We have a Task-returning method than is a Task-returning due to generic substitution of the return type. + // The base method is T-returning and thus does not have an async variant that we can override. + // + // 2. We may have added a void-returning async thunk in anticipation of covariant Task -> Task override. + // The thunk is added very early based on limited type system information and it is not 100% guaranteed that + // we actually have Task -> Task situation. (i.e. we may have Object -> Task override or some other case...) + // When this happens the thunk does not override anything. + // + // It is ok in the above cases to not have a base. It means that the "impl" method should not be called + // polymorphically. + // continue; } diff --git a/src/tests/async/covariant-return/covariant-returns.csproj b/src/tests/async/covariant-return/covariant-returns.csproj index b0866d6e569808..197767e2c4e249 100644 --- a/src/tests/async/covariant-return/covariant-returns.csproj +++ b/src/tests/async/covariant-return/covariant-returns.csproj @@ -1,8 +1,4 @@ - - - True - From a017838b2ebc1b020cfe7ceb0614686d4e1e2715 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:26:03 -0700 Subject: [PATCH 09/21] fix Unix build. --- src/coreclr/debug/daccess/dacdbiimpl.cpp | 2 +- src/coreclr/vm/method.hpp | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index a7e1b1cd50cea2..1891248bb85c10 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -1352,7 +1352,7 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetNativeCodeInfo(VMPTR_DomainAss MethodDesc* pMethodDesc = FindLoadedMethodRefOrDef(pModule, functionToken); if (pMethodDesc != NULL && pMethodDesc->IsAsyncThunkMethod()) { - MethodDesc* pAsyncVariant = pMethodDesc->GetAsyncOtherVariantNoCreate(); + MethodDesc* pAsyncVariant = pMethodDesc->GetOrdinaryVariantNoCreate(); if (pAsyncVariant != NULL) { pMethodDesc = pAsyncVariant; diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index b41c3cf58f204a..3cd796cbbb595c 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1737,19 +1737,18 @@ class MethodDesc MethodDesc* GetOrdinaryVariant(BOOL allowInstParam = TRUE) { - _ASSERT(IsAsyncVariantMethod()); return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, TRUE); } - MethodDesc* GetAsyncOtherVariantNoCreate(BOOL allowInstParam = TRUE) + // 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. + MethodDesc* GetOrdinaryVariantNoCreate(BOOL allowInstParam = TRUE) { - _ASSERTE(HasAsyncOtherVariant()); - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, FALSE, FALSE, AsyncVariantLookup::AsyncOtherVariant); + return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, FALSE); } MethodDesc* GetAsyncVariant(BOOL allowInstParam = TRUE) { - _ASSERT(!IsAsyncVariantMethod() || IsReturnDroppingThunk()); return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, TRUE); } @@ -1757,7 +1756,6 @@ class MethodDesc // for rare cases where we cannot allow GC, but we know that the other variant is already created. MethodDesc* GetAsyncVariantNoCreate(BOOL allowInstParam = TRUE) { - _ASSERT(!IsAsyncVariantMethod()); return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, FALSE); } From d72760b2b1294d1485d93193fca2e8379f67a747 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:26:12 -0700 Subject: [PATCH 10/21] assert --- src/coreclr/vm/method.hpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 3cd796cbbb595c..dfca8516c0a390 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1718,13 +1718,19 @@ class MethodDesc BOOL allowCreate = TRUE, ClassLoadLevel level = CLASS_LOADED) { + // If this assert fires, we may just need to add a lookup that matches AsyncMethodFlags::ReturnDroppingThunk + // It does not look like there is a scenario for directly calling ReturnDroppingThunk right now. + _ASSERTE(!pPrimaryMD->IsReturnDroppingThunk()); + // by default async lookup matches the primaryMD + AsyncVariantLookup variantLookup = pPrimaryMD->IsAsyncVariantMethod() ? AsyncVariantLookup::Async : AsyncVariantLookup::Ordinary; + return FindOrCreateAssociatedMethodDesc( pPrimaryMD, pExactMT, forceBoxedEntryPoint, methodInst, allowInstParam, - pPrimaryMD->IsAsyncVariantMethod() ? AsyncVariantLookup::Async : AsyncVariantLookup::Ordinary, + variantLookup, forceRemotableMethod, allowCreate, level); From ca312af3b1412d50e34f21cad34ad4d25178c21e Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:28:45 -0700 Subject: [PATCH 11/21] more test scenarios --- .../covariant-return/covariant-returns.cs | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/src/tests/async/covariant-return/covariant-returns.cs b/src/tests/async/covariant-return/covariant-returns.cs index 4507f5cd5b7787..999e5ae7d1e42e 100644 --- a/src/tests/async/covariant-return/covariant-returns.cs +++ b/src/tests/async/covariant-return/covariant-returns.cs @@ -8,12 +8,30 @@ public class CovariantReturns { + [Fact] + public static void Test0EntryPoint() + { + Test0().Wait(); + } + [Fact] public static void Test1EntryPoint() { Test1().Wait(); } + [Fact] + public static void Test2EntryPoint() + { + Test2().Wait(); + } + + [Fact] + public static void Test2AEntryPoint() + { + Test2A().Wait(); + } + [MethodImpl(MethodImplOptions.NoInlining)] private static async Task Test0() { @@ -25,9 +43,10 @@ private static async Task Test0() [MethodImpl(MethodImplOptions.NoInlining)] private static async Task Test1() { + // check year to not be concerned with devirtualization. Base b = DateTime.Now.Year > 0 ? new Derived() : new Base(); await b.M1(); - Assert.Equal("Derived.M1;", b.Trace); + Assert.Equal("Derived.M1;Base.M1;", b.Trace); } [MethodImpl(MethodImplOptions.NoInlining)] @@ -35,7 +54,15 @@ private static async Task Test2() { Base b = DateTime.Now.Year > 0 ? new Derived2() : new Base(); await b.M1(); - Assert.Equal("Derived2.M1;Derived.M1;", b.Trace); + Assert.Equal("Derived2.M1;Derived.M1;Base.M1;", b.Trace); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task Test2A() + { + Base b = DateTime.Now.Year > 0 ? new Derived2A() : new Base(); + await b.M1(); + Assert.Equal("Derived2A.M1;DerivedA.M1;Base.M1;", b.Trace); } struct S1 @@ -65,6 +92,7 @@ class Derived : Base public override Task M1() { Trace += "Derived.M1;"; + base.M1().GetAwaiter().GetResult(); return Task.FromResult(new S1(42)); } } @@ -79,4 +107,25 @@ public override async Task M1() return new S1(4242); } } + + class DerivedA : Base + { + public async override Task M1() + { + Trace += "DerivedA.M1;"; + await base.M1(); + return new S1(42); + } + } + + class Derived2A : DerivedA + { + public override async Task M1() + { + Trace += "Derived2A.M1;"; + await Task.Delay(1); + await base.M1(); + return new S1(4242); + } + } } From 3c596e11d0290d18c03cf800e00c651596c8297c Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 24 Mar 2026 09:57:35 -0700 Subject: [PATCH 12/21] Disable the new test on NativeAOT --- src/tests/async/covariant-return/covariant-returns.csproj | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tests/async/covariant-return/covariant-returns.csproj b/src/tests/async/covariant-return/covariant-returns.csproj index 197767e2c4e249..4f69c773ab13b1 100644 --- a/src/tests/async/covariant-return/covariant-returns.csproj +++ b/src/tests/async/covariant-return/covariant-returns.csproj @@ -1,4 +1,8 @@ + + + true + From cd1597a815a0f5bdfb4461a825d8f737e0157d22 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:08:09 -0700 Subject: [PATCH 13/21] tweak TraceManager to find user code in ReturnDroppingThunk scenario --- src/coreclr/vm/stubmgr.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 7e46f7908200a6..33ef239838e22a 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -2284,6 +2284,15 @@ BOOL AsyncThunkStubManager::TraceManager(Thread *thread, MethodDesc* pOtherMD = pMD->GetOrdinaryVariant(); _ASSERTE_MSG(pOtherMD != NULL, "ATSM::TraceManager: Async thunk has no non-thunk variant to step through to"); + // An orinary variant may be a thunk in a rare case when we start from ReturnDroppingThunk. + // In such case the regular async variant must be not be a thunk. + if (pOtherMD->IsAsyncThunkMethod) + { + _ASSERTE(pMD->IsReturnDroppingThunk()); + MethodDesc* pOtherMD = pMD->GetAsyncVariant(); + _ASSERTE(!pOtherMD->IsAsyncThunkMethod()); + } + LOG((LF_CORDB, LL_INFO1000, "ATSM::TraceManager: Step through async thunk to target - %p\n", pOtherMD)); PCODE target = GetStubTarget(pOtherMD); if (target == (PCODE)NULL) From 27d5b1ec2aeffaede4d34c9818b2908f71280ff7 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Tue, 24 Mar 2026 16:10:47 -0700 Subject: [PATCH 14/21] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/methodtablebuilder.cpp | 2 +- src/coreclr/vm/stubmgr.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index b96f4712dbc524..de327502f8851c 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -6052,7 +6052,7 @@ MethodTableBuilder::ProcessMethodImpls() { // There are two scenarios when an async variant may not find a base to override: // - // 1. We have a Task-returning method than is a Task-returning due to generic substitution of the return type. + // 1. We have a Task-returning method that is Task-returning due to generic substitution of the return type. // The base method is T-returning and thus does not have an async variant that we can override. // // 2. We may have added a void-returning async thunk in anticipation of covariant Task -> Task override. diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 33ef239838e22a..514e5eca322aeb 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -2284,12 +2284,12 @@ BOOL AsyncThunkStubManager::TraceManager(Thread *thread, MethodDesc* pOtherMD = pMD->GetOrdinaryVariant(); _ASSERTE_MSG(pOtherMD != NULL, "ATSM::TraceManager: Async thunk has no non-thunk variant to step through to"); - // An orinary variant may be a thunk in a rare case when we start from ReturnDroppingThunk. - // In such case the regular async variant must be not be a thunk. - if (pOtherMD->IsAsyncThunkMethod) + // An ordinary variant may be a thunk in a rare case when we start from ReturnDroppingThunk. + // In such case the regular async variant must not be a thunk. + if (pOtherMD->IsAsyncThunkMethod()) { _ASSERTE(pMD->IsReturnDroppingThunk()); - MethodDesc* pOtherMD = pMD->GetAsyncVariant(); + pOtherMD = pMD->GetAsyncVariant(); _ASSERTE(!pOtherMD->IsAsyncThunkMethod()); } From 875c8956174de875c5ea569fe7befc289f84070d Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:55:06 -0700 Subject: [PATCH 15/21] incorrect assert --- src/coreclr/vm/stubmgr.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/stubmgr.cpp b/src/coreclr/vm/stubmgr.cpp index 514e5eca322aeb..4be4e51389c50d 100644 --- a/src/coreclr/vm/stubmgr.cpp +++ b/src/coreclr/vm/stubmgr.cpp @@ -2282,14 +2282,14 @@ BOOL AsyncThunkStubManager::TraceManager(Thread *thread, if (pMD->IsAsyncThunkMethod()) { MethodDesc* pOtherMD = pMD->GetOrdinaryVariant(); - _ASSERTE_MSG(pOtherMD != NULL, "ATSM::TraceManager: Async thunk has no non-thunk variant to step through to"); + _ASSERTE_MSG(pOtherMD != NULL, "ATSM::TraceManager: Async thunk does not have non-async variant"); // An ordinary variant may be a thunk in a rare case when we start from ReturnDroppingThunk. // In such case the regular async variant must not be a thunk. if (pOtherMD->IsAsyncThunkMethod()) { - _ASSERTE(pMD->IsReturnDroppingThunk()); pOtherMD = pMD->GetAsyncVariant(); + _ASSERTE_MSG(pOtherMD != NULL, "ATSM::TraceManager: Async thunk has no non-thunk variant to step through to"); _ASSERTE(!pOtherMD->IsAsyncThunkMethod()); } From 6fc40b9eb3c540739e13ff7939015f1566eaa1fd Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 26 Mar 2026 18:56:01 -0700 Subject: [PATCH 16/21] Added a test scenario --- .../covariant-return/covariant-returns.cs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/tests/async/covariant-return/covariant-returns.cs b/src/tests/async/covariant-return/covariant-returns.cs index 999e5ae7d1e42e..5cb869e2705a73 100644 --- a/src/tests/async/covariant-return/covariant-returns.cs +++ b/src/tests/async/covariant-return/covariant-returns.cs @@ -129,3 +129,47 @@ public override async Task M1() } } } + +namespace AsyncMicro +{ + public class Program + { + [Fact] + static void TestPrRepro() + { + Derived2 test = new(); + Test(test).GetAwaiter().GetResult(); + } + + private static async Task Test(Base b) + { + await b.Foo(); + } + + public class Base + { + public virtual async Task Foo() + { + Console.WriteLine("Task< Base.Foo"); + } + } + + public class Derived : Base + { + public override async Task Foo() + { + Console.WriteLine("Task Derived.Foo"); + return 123; + } + } + + public class Derived2 : Derived + { + public override async Task Foo() + { + Console.WriteLine("Task Derived2.Foo"); + return await base.Foo(); + } + } + } +} From 48034b3a1f190d6ac46d719bc086952babc01c65 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Sat, 28 Mar 2026 17:07:53 -0700 Subject: [PATCH 17/21] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/methodtablebuilder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index de327502f8851c..c5a1372d9c3427 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -2697,7 +2697,7 @@ MethodTableBuilder::EnumerateClassMethods() // In a worst case the number of declared methods can double // as each async method may have two variants. // The method count is typically a modest number though. - // If we have covariant overrides, then overrides from Task -> Task we will need 3 method descs. + // If we have covariant overrides, such as a base Task method overridden by Task, we will need 3 method descs. // Reserve the space conservatively, up to the max, for the worst case scenario. DWORD cMethUpperBound = cMethAndGaps * (bmtMetaData->fHasCovariantOverride ? 3 : 2); if ((DWORD)MAX_SLOT_INDEX <= cMethUpperBound) @@ -3542,8 +3542,8 @@ MethodTableBuilder::EnumerateClassMethods() } // In rare cases we need a void-returning async variant in addition to the T-returning one. - // It is ok to add a void-returning thunk and end up not using it, but we do not want waste. - // Thus we try to filter closer to the cases when the thunk is most certainly will be used. + // It is ok to add a void-returning thunk and end up not using it, but we want to avoid waste. + // Thus we try to filter closer to the cases when the thunk most certainly will be used. if (insertCount == 1) { if (implType != METHOD_IMPL || From 1c9ff747458d572d86923c85efc900f6ccf56d83 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Sat, 28 Mar 2026 17:31:57 -0700 Subject: [PATCH 18/21] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/tests/async/covariant-return/covariant-returns.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/async/covariant-return/covariant-returns.cs b/src/tests/async/covariant-return/covariant-returns.cs index 5cb869e2705a73..694caf971f15f9 100644 --- a/src/tests/async/covariant-return/covariant-returns.cs +++ b/src/tests/async/covariant-return/covariant-returns.cs @@ -135,7 +135,7 @@ namespace AsyncMicro public class Program { [Fact] - static void TestPrRepro() + public static void TestPrRepro() { Derived2 test = new(); Test(test).GetAwaiter().GetResult(); @@ -150,7 +150,7 @@ public class Base { public virtual async Task Foo() { - Console.WriteLine("Task< Base.Foo"); + Console.WriteLine("Task Base.Foo"); } } From e76dbc8efbbe96afad78b778820b238d52e6f370 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 31 Mar 2026 18:15:39 -0700 Subject: [PATCH 19/21] ask for the same load level as in the prototype variant --- src/coreclr/vm/method.hpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index dfca8516c0a390..9188d081cd5ee4 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1743,26 +1743,30 @@ class MethodDesc MethodDesc* GetOrdinaryVariant(BOOL allowInstParam = TRUE) { - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, TRUE); + MethodTable* mt = GetMethodTable(); + return FindOrCreateAssociatedMethodDesc(this, mt, FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, TRUE, mt->GetLoadLevel()); } // 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. MethodDesc* GetOrdinaryVariantNoCreate(BOOL allowInstParam = TRUE) { - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, FALSE); + MethodTable* mt = GetMethodTable(); + return FindOrCreateAssociatedMethodDesc(this, mt, FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Ordinary, FALSE, FALSE, mt->GetLoadLevel()); } MethodDesc* GetAsyncVariant(BOOL allowInstParam = TRUE) { - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, TRUE); + MethodTable* mt = GetMethodTable(); + return FindOrCreateAssociatedMethodDesc(this, mt, FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, TRUE, mt->GetLoadLevel()); } // 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. MethodDesc* GetAsyncVariantNoCreate(BOOL allowInstParam = TRUE) { - return FindOrCreateAssociatedMethodDesc(this, GetMethodTable(), FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, FALSE); + MethodTable* mt = GetMethodTable(); + return FindOrCreateAssociatedMethodDesc(this, mt, FALSE, GetMethodInstantiation(), allowInstParam, AsyncVariantLookup::Async, FALSE, FALSE, mt->GetLoadLevel()); } // True if a MD is an funny BoxedEntryPointStub (not from the method table) or From f1bf0a9e3ec25c3363a7a4f04adda056d2bc2cb2 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 1 Apr 2026 15:35:03 -0700 Subject: [PATCH 20/21] updated the repro scenario to tests the results (not just see if nothing crashed) --- src/tests/async/covariant-return/covariant-returns.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/tests/async/covariant-return/covariant-returns.cs b/src/tests/async/covariant-return/covariant-returns.cs index 694caf971f15f9..9aaef5f91f7122 100644 --- a/src/tests/async/covariant-return/covariant-returns.cs +++ b/src/tests/async/covariant-return/covariant-returns.cs @@ -134,11 +134,14 @@ namespace AsyncMicro { public class Program { + internal static string Trace; + [Fact] public static void TestPrRepro() { Derived2 test = new(); Test(test).GetAwaiter().GetResult(); + Assert.Equal("Task Derived2.Foo;Task Derived.Foo;", Trace); } private static async Task Test(Base b) @@ -150,7 +153,7 @@ public class Base { public virtual async Task Foo() { - Console.WriteLine("Task Base.Foo"); + Trace += "Task Base.Foo;"; } } @@ -158,7 +161,7 @@ public class Derived : Base { public override async Task Foo() { - Console.WriteLine("Task Derived.Foo"); + Trace += "Task Derived.Foo;"; return 123; } } @@ -167,7 +170,7 @@ public class Derived2 : Derived { public override async Task Foo() { - Console.WriteLine("Task Derived2.Foo"); + Trace += "Task Derived2.Foo;"; return await base.Foo(); } } From 1999896f06b940c3c541c33a44950a09fe4a20ae Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 6 Apr 2026 18:54:30 -0700 Subject: [PATCH 21/21] PR feedback --- src/coreclr/vm/methodtablebuilder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index c5a1372d9c3427..038fa7a8d61417 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3546,12 +3546,12 @@ MethodTableBuilder::EnumerateClassMethods() // Thus we try to filter closer to the cases when the thunk most certainly will be used. if (insertCount == 1) { - if (implType != METHOD_IMPL || + if (!bmtMetaData->fHasCovariantOverride || + implType != METHOD_IMPL || returnsValueTask || returnKind != MethodReturnKind::GenericTaskReturningMethod || this->IsValueClass() || - !IsMdVirtual(dwMemberAttrs) || - !bmtMetaData->fHasCovariantOverride) + !IsMdVirtual(dwMemberAttrs)) { // No need for another variant break;