Skip to content

Commit cbe05f9

Browse files
authored
JIT: don't optimize calls to CORINFO_HELP_VIRTUAL_FUNC_PTR for interface class lookups (#82209)
The jit currently replaces calls to `CORINFO_HELP_VIRTUAL_FUNC_PTR` whose results are unused with null pointer checks. But for interface classes this helper can throw a variety of exceptions, so this optimization is incorrect. Fixes #82127.
1 parent b79684a commit cbe05f9

File tree

3 files changed

+41
-23
lines changed

3 files changed

+41
-23
lines changed

src/coreclr/jit/gentree.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4197,6 +4197,7 @@ enum GenTreeCallFlags : unsigned int
41974197
GTF_CALL_M_STRESS_TAILCALL = 0x04000000, // the call is NOT "tail" prefixed but GTF_CALL_M_EXPLICIT_TAILCALL was added because of tail call stress mode
41984198
GTF_CALL_M_EXPANDED_EARLY = 0x08000000, // the Virtual Call target address is expanded and placed in gtControlExpr in Morph rather than in Lower
41994199
GTF_CALL_M_HAS_LATE_DEVIRT_INFO = 0x10000000, // this call has late devirtualzation info
4200+
GTF_CALL_M_LDVIRTFTN_INTERFACE = 0x20000000, // ldvirtftn on an interface type
42004201
};
42014202

42024203
inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a)

src/coreclr/jit/importer.cpp

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3116,58 +3116,75 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr,
31163116
CORINFO_RESOLVED_TOKEN* pResolvedToken,
31173117
CORINFO_CALL_INFO* pCallInfo)
31183118
{
3119-
if ((pCallInfo->methodFlags & CORINFO_FLG_EnC) && !(pCallInfo->classFlags & CORINFO_FLG_INTERFACE))
3119+
const bool isInterface = (pCallInfo->classFlags & CORINFO_FLG_INTERFACE) == CORINFO_FLG_INTERFACE;
3120+
3121+
if ((pCallInfo->methodFlags & CORINFO_FLG_EnC) && !isInterface)
31203122
{
31213123
NO_WAY("Virtual call to a function added via EnC is not supported");
31223124
}
31233125

3126+
GenTreeCall* call = nullptr;
3127+
31243128
// NativeAOT generic virtual method
31253129
if ((pCallInfo->sig.sigInst.methInstCount != 0) && IsTargetAbi(CORINFO_NATIVEAOT_ABI))
31263130
{
31273131
GenTree* runtimeMethodHandle =
31283132
impLookupToTree(pResolvedToken, &pCallInfo->codePointerLookup, GTF_ICON_METHOD_HDL, pCallInfo->hMethod);
3129-
return gtNewHelperCallNode(CORINFO_HELP_GVMLOOKUP_FOR_SLOT, TYP_I_IMPL, thisPtr, runtimeMethodHandle);
3133+
call = gtNewHelperCallNode(CORINFO_HELP_GVMLOOKUP_FOR_SLOT, TYP_I_IMPL, thisPtr, runtimeMethodHandle);
31303134
}
31313135

31323136
#ifdef FEATURE_READYTORUN
3133-
if (opts.IsReadyToRun())
3137+
else if (opts.IsReadyToRun())
31343138
{
31353139
if (!pCallInfo->exactContextNeedsRuntimeLookup)
31363140
{
3137-
GenTreeCall* call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr);
3138-
3141+
call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr);
31393142
call->setEntryPoint(pCallInfo->codePointerLookup.constLookup);
3140-
3141-
return call;
31423143
}
3143-
31443144
// We need a runtime lookup. NativeAOT has a ReadyToRun helper for that too.
3145-
if (IsTargetAbi(CORINFO_NATIVEAOT_ABI))
3145+
else if (IsTargetAbi(CORINFO_NATIVEAOT_ABI))
31463146
{
31473147
GenTree* ctxTree = getRuntimeContextTree(pCallInfo->codePointerLookup.lookupKind.runtimeLookupKind);
31483148

3149-
return impReadyToRunHelperToTree(pResolvedToken, CORINFO_HELP_READYTORUN_GENERIC_HANDLE, TYP_I_IMPL,
3149+
call = impReadyToRunHelperToTree(pResolvedToken, CORINFO_HELP_READYTORUN_GENERIC_HANDLE, TYP_I_IMPL,
31503150
&pCallInfo->codePointerLookup.lookupKind, ctxTree);
31513151
}
31523152
}
31533153
#endif
31543154

3155-
// Get the exact descriptor for the static callsite
3156-
GenTree* exactTypeDesc = impParentClassTokenToHandle(pResolvedToken);
3157-
if (exactTypeDesc == nullptr)
3158-
{ // compDonotInline()
3159-
return nullptr;
3160-
}
3155+
if (call == nullptr)
3156+
{
3157+
// Get the exact descriptor for the static callsite
3158+
GenTree* exactTypeDesc = impParentClassTokenToHandle(pResolvedToken);
3159+
if (exactTypeDesc == nullptr)
3160+
{
3161+
assert(compIsForInlining());
3162+
return nullptr;
3163+
}
31613164

3162-
GenTree* exactMethodDesc = impTokenToHandle(pResolvedToken);
3163-
if (exactMethodDesc == nullptr)
3164-
{ // compDonotInline()
3165-
return nullptr;
3165+
GenTree* exactMethodDesc = impTokenToHandle(pResolvedToken);
3166+
if (exactMethodDesc == nullptr)
3167+
{
3168+
assert(compIsForInlining());
3169+
return nullptr;
3170+
}
3171+
3172+
// Call helper function. This gets the target address of the final destination callsite.
3173+
//
3174+
call = gtNewHelperCallNode(CORINFO_HELP_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr, exactTypeDesc, exactMethodDesc);
31663175
}
31673176

3168-
// Call helper function. This gets the target address of the final destination callsite.
3177+
assert(call != nullptr);
3178+
3179+
if (isInterface)
3180+
{
3181+
// Annotate helper so later on if helper result is unconsumed we know it is not sound
3182+
// to optimize the call into a null check.
3183+
//
3184+
call->gtCallMoreFlags |= GTF_CALL_M_LDVIRTFTN_INTERFACE;
3185+
}
31693186

3170-
return gtNewHelperCallNode(CORINFO_HELP_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr, exactTypeDesc, exactMethodDesc);
3187+
return call;
31713188
}
31723189

31733190
//------------------------------------------------------------------------

src/coreclr/jit/morph.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7802,7 +7802,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
78027802
#endif
78037803
}
78047804

7805-
if ((call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) == 0 &&
7805+
if (((call->gtCallMoreFlags & (GTF_CALL_M_SPECIAL_INTRINSIC | GTF_CALL_M_LDVIRTFTN_INTERFACE)) == 0) &&
78067806
(call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_VIRTUAL_FUNC_PTR)
78077807
#ifdef FEATURE_READYTORUN
78087808
|| call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR)

0 commit comments

Comments
 (0)