Fix DAC GetNativeCodeInfo API to use async variant for thunk MethodDescs#126728
Fix DAC GetNativeCodeInfo API to use async variant for thunk MethodDescs#126728tommcdon wants to merge 1 commit intodotnet:mainfrom
Conversation
PR 125900 replaced GetAsyncOtherVariantNoCreate with GetOrdinaryVariantNoCreate, but for a thunk the correct target is the async variant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
Adjusts DAC GetNativeCodeInfo to resolve async thunk MethodDescs to the correct associated implementation MethodDesc by selecting the async variant, addressing a regression introduced by changes to async thunking.
Changes:
- Update
DacDbiInterfaceImpl::GetNativeCodeInfoto useGetAsyncVariantNoCreate()when the resolvedMethodDescis an async thunk. - Ensure native code region/EnC info is reported for the async implementation rather than the thunk.
|
Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary? |
For runtime async methods, I believe FindLoadedMethodRefOrDef always resolves to an async thunk method. So if the method is identified as "async", my guess is that we need to resolve it to the async variant. |
| if (pMethodDesc != NULL && pMethodDesc->IsAsyncThunkMethod()) | ||
| { | ||
| MethodDesc* pAsyncVariant = pMethodDesc->GetOrdinaryVariantNoCreate(); | ||
| MethodDesc* pAsyncVariant = pMethodDesc->GetAsyncVariantNoCreate(); |
There was a problem hiding this comment.
| MethodDesc* pAsyncVariant = pMethodDesc->GetAsyncVariantNoCreate(); | |
| MethodDesc* pAsyncVariant = pMethodDesc->GetAsyncOtherVariantNoCreate(); |
I think this is a little more general. The current code probably works too but this one doesn't require assumptions about which variant we will get from FindLoadedMethodRefOrDef in the ordinary case.
There was a problem hiding this comment.
I think this is a little more general. The current code probably works too but this one doesn't require assumptions about which variant we will get from FindLoadedMethodRefOrDef in the ordinary case.
I agree, we want this logic, but that exact method was removed recently (#125900).
There was a problem hiding this comment.
oh hah... I need to catch up it appears ;p
Okay, that seems reasonable. if (!pMD->IsUnboxingStub() && !pMD->IsAsyncVariantMethod())
{
pModule->EnsuredStoreMethodDef(pMD->GetMemberDef(), pMD);
}(line 2807-2809 in clsload.cpp) This should mean that if we get into the |
Regressed by #125900 from #124354