Skip to content

Fix GVM dispatch for covariant-return async methods (closes #127197)#129481

Draft
Copilot wants to merge 2 commits into
copilot/enable-async-method-thunk-testfrom
copilot/fix-covariant-return-stack-overflow
Draft

Fix GVM dispatch for covariant-return async methods (closes #127197)#129481
Copilot wants to merge 2 commits into
copilot/enable-async-method-thunk-testfrom
copilot/fix-covariant-return-stack-overflow

Conversation

Copilot AI commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

ResolveGenericVirtualMethod was passing asyncVariantLookup = Ordinary (the default) to FindOrCreateAssociatedMethodDesc. When the vtable slot resolved to an async-variant or ReturnDroppingThunk (RDT) MethodDesc, GetParallelMethodDesc(pDefMD, Ordinary) walked back to the primary thunk instead. The primary thunk then emitted CALLVIRT back to the async-variant slot, which GVM dispatch again resolved to the primary thunk — infinite recursion → stack overflow.

The fix is a one-line change in ResolveGenericVirtualMethod:

// Before — asyncVariantLookup defaults to Ordinary, misidentifying async-variant/RDT targets
RETURN(MethodDesc::FindOrCreateAssociatedMethodDesc(
    pTargetMDBeforeGenericMethodArgs, pTargetMT,
    (pTargetMT->IsValueType()), pStaticMD->GetMethodInstantiation(),
    FALSE));

// After — propagate the async variant kind of the resolved vtable entry
RETURN(MethodDesc::FindOrCreateAssociatedMethodDesc(
    pTargetMDBeforeGenericMethodArgs, pTargetMT,
    (pTargetMT->IsValueType()), pStaticMD->GetMethodInstantiation(),
    FALSE,
    pTargetMDBeforeGenericMethodArgs->GetAsyncVariantLookup()));

GetAsyncVariantLookup() returns Async, ReturnDroppingThunk, or Ordinary depending on what the resolved slot holds, ensuring FindOrCreateAssociatedMethodDesc creates the correctly-typed instantiated MD in all three cases.

This fix depends on the prerequisite AsyncVariantLookup::ReturnDroppingThunk enum value, GetAsyncVariantLookup(), the 3-way variant selector in FindOrCreateAssociatedMethodDesc, and the bool → AsyncVariantLookup widening of InstMethodHashTable::FindMethodDesc / InstantiatedMethodDesc::FindLoadedInstantiatedMethodDesc already in this PR. TestGenericVirtualMethod is re-enabled.

Copilot generated this PR description. A human author reviewed and is responsible for these changes.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

ResolveGenericVirtualMethod was passing asyncVariantLookup=Ordinary
(default) to FindOrCreateAssociatedMethodDesc. When the resolved vtable
slot contained an async-variant or ReturnDroppingThunk MethodDesc,
GetParallelMethodDesc(pDefMD, Ordinary) would walk back to the primary
thunk instead of the async variant, causing the primary thunk to emit
CALLVIRT back to the async-variant slot, which GVM dispatch again
resolved to the primary thunk — infinite recursion / stack overflow.

Fix: pass pTargetMDBeforeGenericMethodArgs->GetAsyncVariantLookup() so
FindOrCreateAssociatedMethodDesc finds the correctly-typed instantiated
MD (Async or ReturnDroppingThunk) matching the resolved vtable entry.

Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix stack overflow in covariant returns test Fix GVM dispatch for covariant-return async methods (closes #127197) Jun 16, 2026
Copilot AI requested a review from VSadov June 16, 2026 22:23
@VSadov

VSadov commented Jun 17, 2026

Copy link
Copy Markdown
Member

@copilot test failures need to be addressed

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot test failures need to be addressed

The committed fix in ResolveGenericVirtualMethod (passing pTargetMDBeforeGenericMethodArgs->GetAsyncVariantLookup() to FindOrCreateAssociatedMethodDesc) is in the build that CI ran, but the test is still stack-overflowing at VirtualFunctionPointer → CallInstance.

After deep analysis of the dispatch chain:

  1. CallInstance(new Derived())VirtualFunctionPointer(b, Base.$async_void<object>) → slot S → Mid.$rdt (inherited by Derived, since Derived doesn't introduce its own $rdt when fHasCovariantOverride is false for Derived)
  2. The fix correctly returns Mid.InstanceMethod<object>$rdt_stub (instantiating stub wrapping Mid.InstanceMethod<__Canon>$rdt)
  3. The canonical Mid.InstanceMethod<__Canon>$rdt calls VirtualFunctionPointer(this, Mid.$async_int<MVAR[0]>) via CALLVIRT in the thunk IL

The logic says this should resolve to Derived.InstanceMethod<object>$async_int without recursion. However, the test still overflows. I was not able to complete a local build to observe the actual behavior and determine the deeper root cause before running out of time. The fix needs further investigation by a human reviewer familiar with the runtime-async dispatch mechanics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants