Repository: dotnet/runtime
File: src/coreclr/vm/stubmgr.cpp (C++)

Code before:
#endif // HAS_THISPTR_RETBUF_PRECODE

        default:
            _ASSERTE_IMPL(!"DoTraceStub: Unexpected precode type");
            break;
        }

        // check if the method has been jitted
        if (!pPrecode->IsPointingToPrestub())
        {
            trace->InitForStub(pPrecode->GetTarget());
            LOG_TRACE_DESTINATION(trace, stubStartAddress, "PrecodeStubManager::DoTraceStub - code");
            return TRUE;
        }

        pMD = pPrecode->GetMethodDesc();
    }

    _ASSERTE(pMD != NULL);

    // For async thunk methods, we need to redirect to the actual async method.
    // We can't set a breakpoint in the async thunk so a call to an unjitted async thunk
    // has to be handled here as opposed to the AsyncThunkStubManager below.
    // We use GetParallelMethodDesc directly rather than GetAsyncOtherVariantNoCreate
    // because FindOrCreateAssociatedMethodDesc can fail with null-deref when
    // GetParallelMethodDesc returns NULL.
    if (pMD->IsAsyncThunkMethod())
    {
        MethodDesc* pMDVariant = pMD->GetMethodTable()->GetCanonicalMethodTable()->GetParallelMethodDesc(pMD, AsyncVariantLookup::AsyncOtherVariant);
        if (pMDVariant != NULL)
        {
            LOG((LF_CORDB, LL_INFO10000, "PrecodeStubManager::DoTraceStub - async thunk %p, tracing to underlying method %p\n", pMD, pMDVariant));
            trace->InitForUnjittedMethod(pMDVariant);
            LOG_TRACE_DESTINATION(trace, stubStartAddress, "PrecodeStubManager::DoTraceStub - async thunk redirect");
            return TRUE;
        }
    }

    // If the method is not IL, then we patch the prestub because no one will ever change the call here at the
    // MethodDesc. If, however, this is an IL method, then we are at risk to have another thread backpatch the call
    // here, so we'd miss if we patched the prestub. Therefore, we go right to the IL method and patch IL offset 0
    // by using TRACE_UNJITTED_METHOD.
    if (!pMD->IsIL() && !pMD->IsILStub())
    {
        trace->InitForStub(GetPreStubEntryPoint());
    }
    else
    {
        trace->InitForUnjittedMethod(pMD);
    }


Review comment (bug):
If GetParallelMethodDesc() returns NULL, is the debugger behavior correct? It appears that we would return the MethodDesc for the AsyncThunk method, then fail to set a breakpoint in it. If we reached this point as part of doing a step-in operation without the breakpoint the step-in would never complete and the code would keep running.

We could change the breakpoint binding logic to allow binding breakpoints when the MethodDesc for an AsyncThunk is given explicitly, but don't resolve user breakpoints set via (Module,RID) onto these Thunk MethodDescs. I think most of the logic to do this is already in place if we make a few tweaks...

IL patches currently have a field pMethodDescFilter where NULL means bind to all methoddescs for the (Module,RID) and non-NULL means only bind when pMD == pMethodDescFilter. We could adjust this definition so that NULL means use a default filter, all MethodDescs that aren't IsDebuggerHidden(). In MapAndBindFunctionPatches you could remove the explicit filter you added and change the [current filtering check](https://github.com/dotnet/runtime/pull/123644/files#diff-929895306dfa5d9f87b16fc8db526518977126da720f701299dc2bb96d5f7f4eR4770-R4774) to call a little helper:

```c++
if(!MatchesBreakpointFilter(pMd, patch)) ...

BOOL MatchesBreakpointFilter(pMD, patch)
{
    return pMD == patch->pMethodDescFilter // explicit filter
     || ((patch->pMethodDescFilter == NULL) && !pMD->IsDebuggerHidden()) // default filter
}
```

You could make a similar change to the [filtering in AddILPatch](https://github.com/dotnet/runtime/pull/123644/files#diff-56c927cca69b678fd1fdbeab66e14e38a36fc6f913cb3d30af7ac37859ac2d3fR2152-R2153). I notice AddILPatch always filters on ENC version whereas MapAndBindFunctionPatches only filters on ENC version for Breakpoint and ENCBreakpoint. Maybe that discrepancy didn't matter before but AddILPatch should probably filter the way MapAndBind does to make this work. Its possible you would do a step-in on an AsyncThunk for a method that is ENC version > 1. I'm not sure if the AsyncThunks get recompiled during ENC so their version number might remain at 1. Its probably more robust if the matching for Stepper patches doesn't require those ENC versions to match.

After those edits this stub manager would need to remove the extra checks and return the thunk MethodDesc. The call to AddPatchToStartOfLatestMethod() after this returns would add the patch with explicit MethodDesc filter, we'd hit that patch later, and then at that point it would be recognized by the AsyncThunkStubManager and complete from there.

Code after:
#endif // HAS_THISPTR_RETBUF_PRECODE

        default:
            _ASSERTE_IMPL(!"DoTraceStub: Unexpected precode type");
            break;
        }

        // check if the method has been jitted
        if (!pPrecode->IsPointingToPrestub())
        {
            trace->InitForStub(pPrecode->GetTarget());
            LOG_TRACE_DESTINATION(trace, stubStartAddress, "PrecodeStubManager::DoTraceStub - code");
            return TRUE;
        }

        pMD = pPrecode->GetMethodDesc();
    }

    _ASSERTE(pMD != NULL);

    // If the method is not IL, then we patch the prestub because no one will ever change the call here at the
    // MethodDesc. If, however, this is an IL method, then we are at risk to have another thread backpatch the call
    // here, so we'd miss if we patched the prestub. Therefore, we go right to the IL method and patch IL offset 0
    // by using TRACE_UNJITTED_METHOD.
    if (!pMD->IsIL() && !pMD->IsILStub())
    {
        trace->InitForStub(GetPreStubEntryPoint());
    }
    else
    {
        trace->InitForUnjittedMethod(pMD);
    }

    LOG_TRACE_DESTINATION(trace, stubStartAddress, "PrecodeStubManager::DoTraceStub - prestub");
    return TRUE;
}

#ifndef DACCESS_COMPILE
BOOL PrecodeStubManager::TraceManager(Thread *thread,
                            TraceDestination *trace,
                            T_CONTEXT *pContext,
                            BYTE **pRetAddr)
{
    CONTRACTL
    {
        NOTHROW;
        GC_NOTRIGGER;
        MODE_ANY;
        PRECONDITION(CheckPointer(thread, NULL_OK));
        PRECONDITION(CheckPointer(trace));
        PRECONDITION(CheckPointer(pContext));