Skip to content

[clr-interp] Fix jmp instruction to tolerate the call.tail opcode failing to actually tail call #123513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
davidwrighton merged 1 commit into from
Jan 23, 2026
Merged

[clr-interp] Fix jmp instruction to tolerate the call.tail opcode failing to actually tail call #123513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 111 additions & 102 deletions src/coreclr/interpreter/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5283,6 +5283,110 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re
}
}

void InterpCompiler::EmitRet(CORINFO_METHOD_INFO* methodInfo)
{
CORINFO_SIG_INFO sig = methodInfo->args;
InterpType retType = GetInterpType(sig.retType);

if ((retType != InterpTypeVoid) && (retType != InterpTypeByRef))
{
CheckStackExact(1);
m_pStackPointer[-1].BashStackTypeToI_ForLocalVariableAddress();
}

if ((m_isSynchronized || m_isAsyncMethodWithContextSaveRestore) && m_currentILOffset < m_ILCodeSizeFromILHeader)
{
// We are in a synchronized/async method, but we need to go through the finally/fault first
int32_t ilOffset = (int32_t)(m_ip - m_pILCode);
int32_t target = m_synchronizedOrAsyncPostFinallyOffset;

if (retType != InterpTypeVoid)
{
CheckStackExact(1);
if (m_synchronizedOrAsyncRetValVarIndex == -1)
{
PushInterpType(retType, m_pVars[m_pStackPointer[-1].var].clsHnd);
m_synchronizedOrAsyncRetValVarIndex = m_pStackPointer[-1].var;
m_pStackPointer--;
INTERP_DUMP("Created ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex);
}
INTERP_DUMP("Store to ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex);
EmitStoreVar(m_synchronizedOrAsyncRetValVarIndex);
}
else
{
CheckStackExact(0);
}
EmitLeave(ilOffset, target);
return;
}

if (m_methodInfo->args.isAsyncCall())
{
// We're doing a standard return. Set the continuation return to NULL.
AddIns(INTOP_SET_CONTINUATION_NULL);
}

if (retType == InterpTypeVoid)
{
CheckStackExact(0);
AddIns(INTOP_RET_VOID);
}
else if (retType == InterpTypeVT)
{
CheckStackExact(1);
AddIns(INTOP_RET_VT);
m_pStackPointer--;
int32_t retVar = m_pStackPointer[0].var;
m_pLastNewIns->SetSVar(retVar);
m_pLastNewIns->data[0] = m_pVars[retVar].size;
}
else
{
CheckStackExact(1);

#ifdef TARGET_64BIT
// nint and int32 can be used interchangeably. Add implicit conversions.
if (m_pStackPointer[-1].GetStackType() == StackTypeI4 && g_stackTypeFromInterpType[retType] == StackTypeI8)
EmitConv(m_pStackPointer - 1, StackTypeI8, INTOP_CONV_I8_I4);
#endif
if (m_pStackPointer[-1].GetStackType() == StackTypeR4 && g_stackTypeFromInterpType[retType] == StackTypeR8)
EmitConv(m_pStackPointer - 1, StackTypeR8, INTOP_CONV_R8_R4);
else if (m_pStackPointer[-1].GetStackType() == StackTypeR8 && g_stackTypeFromInterpType[retType] == StackTypeR4)
EmitConv(m_pStackPointer - 1, StackTypeR4, INTOP_CONV_R4_R8);

if (m_pStackPointer[-1].GetStackType() != g_stackTypeFromInterpType[retType])
{
StackType retStackType = g_stackTypeFromInterpType[retType];
StackType stackType = m_pStackPointer[-1].GetStackType();

if (stackType == StackTypeI && (retStackType == StackTypeO || retStackType == StackTypeByRef))
{
// Allow implicit conversion from nint to ref or byref
}
else if (retStackType == StackTypeI && (stackType == StackTypeO || stackType == StackTypeByRef))
{
// Allow implicit conversion from ref or byref to nint
}
#ifdef TARGET_64BIT
else if (retStackType == StackTypeI4 && stackType == StackTypeI8)
{
// nint and int32 can be used interchangeably. Add implicit conversions.
// Allow implicit conversion from int64 to int32 which is just a truncation
}
#endif
else
{
BADCODE("return type mismatch");
}
}

AddIns(GetRetForType(retType));
m_pStackPointer--;
m_pLastNewIns->SetSVar(m_pStackPointer[0].var);
}
}

void SetSlotToTrue(TArray<bool, MemPoolAllocator> &gcRefMap, int32_t slotOffset)
{
assert(slotOffset % sizeof(void*) == 0);
Expand Down Expand Up @@ -8149,108 +8253,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)

case CEE_RET:
{
CORINFO_SIG_INFO sig = methodInfo->args;
InterpType retType = GetInterpType(sig.retType);

if ((retType != InterpTypeVoid) && (retType != InterpTypeByRef))
{
CheckStackExact(1);
m_pStackPointer[-1].BashStackTypeToI_ForLocalVariableAddress();
}

if ((m_isSynchronized || m_isAsyncMethodWithContextSaveRestore) && m_currentILOffset < m_ILCodeSizeFromILHeader)
{
// We are in a synchronized/async method, but we need to go through the finally/fault first
int32_t ilOffset = (int32_t)(m_ip - m_pILCode);
int32_t target = m_synchronizedOrAsyncPostFinallyOffset;

if (retType != InterpTypeVoid)
{
CheckStackExact(1);
if (m_synchronizedOrAsyncRetValVarIndex == -1)
{
PushInterpType(retType, m_pVars[m_pStackPointer[-1].var].clsHnd);
m_synchronizedOrAsyncRetValVarIndex = m_pStackPointer[-1].var;
m_pStackPointer--;
INTERP_DUMP("Created ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex);
}
INTERP_DUMP("Store to ret val var V%d\n", m_synchronizedOrAsyncRetValVarIndex);
EmitStoreVar(m_synchronizedOrAsyncRetValVarIndex);
}
else
{
CheckStackExact(0);
}
EmitLeave(ilOffset, target);
linkBBlocks = false;
m_ip++;
break;
}

if (m_methodInfo->args.isAsyncCall())
{
// We're doing a standard return. Set the continuation return to NULL.
AddIns(INTOP_SET_CONTINUATION_NULL);
}

if (retType == InterpTypeVoid)
{
CheckStackExact(0);
AddIns(INTOP_RET_VOID);
}
else if (retType == InterpTypeVT)
{
CheckStackExact(1);
AddIns(INTOP_RET_VT);
m_pStackPointer--;
int32_t retVar = m_pStackPointer[0].var;
m_pLastNewIns->SetSVar(retVar);
m_pLastNewIns->data[0] = m_pVars[retVar].size;
}
else
{
CheckStackExact(1);

#ifdef TARGET_64BIT
// nint and int32 can be used interchangeably. Add implicit conversions.
if (m_pStackPointer[-1].GetStackType() == StackTypeI4 && g_stackTypeFromInterpType[retType] == StackTypeI8)
EmitConv(m_pStackPointer - 1, StackTypeI8, INTOP_CONV_I8_I4);
#endif
if (m_pStackPointer[-1].GetStackType() == StackTypeR4 && g_stackTypeFromInterpType[retType] == StackTypeR8)
EmitConv(m_pStackPointer - 1, StackTypeR8, INTOP_CONV_R8_R4);
else if (m_pStackPointer[-1].GetStackType() == StackTypeR8 && g_stackTypeFromInterpType[retType] == StackTypeR4)
EmitConv(m_pStackPointer - 1, StackTypeR4, INTOP_CONV_R4_R8);

if (m_pStackPointer[-1].GetStackType() != g_stackTypeFromInterpType[retType])
{
StackType retStackType = g_stackTypeFromInterpType[retType];
StackType stackType = m_pStackPointer[-1].GetStackType();

if (stackType == StackTypeI && (retStackType == StackTypeO || retStackType == StackTypeByRef))
{
// Allow implicit conversion from nint to ref or byref
}
else if (retStackType == StackTypeI && (stackType == StackTypeO || stackType == StackTypeByRef))
{
// Allow implicit conversion from ref or byref to nint
}
#ifdef TARGET_64BIT
else if (retStackType == StackTypeI4 && stackType == StackTypeI8)
{
// nint and int32 can be used interchangeably. Add implicit conversions.
// Allow implicit conversion from int64 to int32 which is just a truncation
}
#endif
else
{
BADCODE("return type mismatch");
}
}

AddIns(GetRetForType(retType));
m_pStackPointer--;
m_pLastNewIns->SetSVar(m_pStackPointer[0].var);
}
EmitRet(methodInfo);
m_ip++;
linkBBlocks = false;
break;
Expand Down Expand Up @@ -9385,7 +9388,13 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
// CEE_JMP inside a funclet is not allowed
BADCODE("CEE_JMP inside funclet");
}
if (m_isSynchronized || m_isAsyncMethodWithContextSaveRestore)
{
BADCODE("CEE_JMP in synchronized or async method");
}
EmitCall(m_pConstrainedToken, readonly, true /* tailcall */, false /*newObj*/, false /*isCalli*/);
EmitRet(methodInfo); // The tail-call infrastructure in the interpreter is not 100% guaranteed to do a
// tail-call, so inject the ret logic here to cover that case.
linkBBlocks = false;
break;
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/interpreter/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ class InterpCompiler
void EmitShiftOp(int32_t opBase);
void EmitCompareOp(int32_t opBase);
void EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool readonly, bool tailcall, bool newObj, bool isCalli);
void EmitRet(CORINFO_METHOD_INFO* methodInfo);
void EmitSuspend(const CORINFO_CALL_INFO &callInfo, ContinuationContextHandling ContinuationContextHandling);
void EmitCalli(bool isTailCall, void* calliCookie, int callIFunctionPointerVar, CORINFO_SIG_INFO* callSiteSig);
bool EmitNamedIntrinsicCall(NamedIntrinsic ni, bool nonVirtualCall, CORINFO_CLASS_HANDLE clsHnd, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO sig);
Expand Down
Loading