Look through async thunks in RuntimeHelpers.PrepareMethod

[jakobbotsch]

This function should JIT the method that has the user IL. For runtime async functions reflection exposes the Task-returning variant which is normally a thunk, so treat these transparently.

[Copilot]

Pull request overview

Updates CoreCLR’s ReflectionInvocation_PrepareMethod path to avoid JIT-preparing runtime async thunk methods, and instead prepare the associated variant that owns the user IL.

Changes:

• Detect async thunk MethodDescs in PrepareMethodHelper.
• Redirect preparation to the async variant via GetAsyncVariant() prior to prestub/JIT.

[Copilot]

Reassigning pMD to the async variant before the prestub/wrapper-stub logic can skip preparing the original MethodDesc when the incoming method is a wrapper stub (e.g., value-type unboxing stubs can carry async method data/flags). That can change PrepareMethodHelper behavior (notably for ReflectionInvocation_PrepareDelegate) by no longer calling prestub on the wrapper stub the caller will actually invoke.

Consider preserving the original pMD for the existing prestub + IsWrapperStub() handling, and only resolving/jitting the async variant for the wrapped target (after unwrapping), so both the wrapper stub and the user-IL owning variant get prepared.

Also, GetAsyncVariant() defaults allowInstParam to TRUE, while ReflectionInvocation_PrepareMethod resolves the associated MD with allowInstParam = FALSE; it’s likely safer to pass FALSE here as well to avoid ending up preparing an instantiating-stub variant unexpectedly.

[jkotas]

Maybe do it like this to avoid compiling wrapper stub for the async variant? I think the wrapper stub for the async variant is less likely to be ever used.

    if (pMD->IsWrapperStub())
    {
        if (pMD->ShouldCallPrestub())
            pMD->DoPrestub(NULL);
        pMD = pMD->GetWrappedMethodDesc();
    }

    if (pMD->IsAsyncThunk())
    {
        if (pMD->ShouldCallPrestub())
            pMD->DoPrestub(NULL);
        pMD = pMD->GetAsyncVariant();
    }

    if (pMD->ShouldCallPrestub())
        pMD->DoPrestub(NULL);

[jakobbotsch]

That seems better. Used that.

[hez2010]

Maybe do it like this to avoid compiling wrapper stub for the async variant? I think the wrapper stub for the async variant is less likely to be ever used.

I don't have a strong opinion, but godbolt is relying on PrepareMethod to inspect the JIT disasm of assemblies. I'm not sure in such use cases whether users want/should see the JIT assembly code of async wrappers in addition to the real async variant.

[jkotas]

PrepareMethod triggers JITing of the generic wrapper stubs today. Do users see the disassembly for those?

[hez2010]

Yeah. Users are seeing disassembly for async wrapper stubs today, but I think users should see the async variants, or both async wrapper stubs and the async variants.
Example: https://godbolt.org/z/cqdP7GMqT

[jkotas]

Yes, I agree. With this change, they should see both.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

The new behavior that looks through async thunk methods is not covered by an automated regression test. Please add a test that uses a runtime-async method ([RuntimeAsyncMethodGeneration(true)]) and verifies that calling RuntimeHelpers.PrepareMethod on the reflection-exposed MethodHandle causes the underlying user-IL variant (not just the Task-returning thunk) to be jitted/prepared (e.g., via JIT/ETW/EventPipe events or another observable signal).