Fix DAC GetNativeCodeInfo API to use async variant for thunk MethodDescs

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated

[tommcdon]

Regressed by #125900 from #124354

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[Copilot]

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::GetNativeCodeInfo to use GetAsyncVariantNoCreate() when the resolved MethodDesc is an async thunk.
• Ensure native code region/EnC info is reported for the async implementation rather than the thunk.

[max-charlamb]

Should we always try to get the non-thunk variant? Regardless of if it is async or ordinary?

[tommcdon]

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.

[max-charlamb]

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.

Okay, that seems reasonable. FindLoadedMethodRefOrDef should always return the ordinary variant given this check when adding method defs to the m_MethodDefToDescMap

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 pMethodDesc->IsAsyncThunkMethod() check, it implies that we have an ordinary thunk and getting the async method will be the true body. I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

[tommcdon]

I am a little concerned this falls apart with EnC given that registers the methods to the map without the above check.

We have a separate code change for supporting EnC here - #125397.

[tommcdon]

/ba-g failures are unrelated