Skip to content

Commit 4b2a7d5

Browse files
authored
Set of interpreter related fixes for coreclr tests (#118617)
* Set of interpreter related fixes for coreclr tests The changes in this PR were made to fix various issues uncovered by the coreclr tests. * Handle case when a try block is eliminated because it can never be executed. * Ensure call stubs are generated for IL stubs. * Fix shadow stack updating during stack walk * Fix handling managed exceptions thrown as SEH from managed code called by JIT / Interpreter compiler in interpreted code. * Fix resuming after catch in interpreted code for cases when interpreted code calls another interpreted code via a prestub, which currently happens for delegates. * Ensure that ip is saved to the pFrame->ip before calling native methods that can throw to make sure that the stack walk reports correct return IP. * Fix build break * One more build break * Fix WASM build break
1 parent 2cb91fd commit 4b2a7d5

File tree

4 files changed

+111
-9
lines changed

4 files changed

+111
-9
lines changed

src/coreclr/interpreter/compiler.cpp

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,20 +1140,40 @@ void InterpCompiler::BuildEHInfo()
11401140

11411141
INTERP_DUMP("EH info:\n");
11421142

1143-
if (m_methodInfo->EHcount == 0)
1143+
unsigned int nativeEHCount = 0;
1144+
for (unsigned int i = 0; i < m_methodInfo->EHcount; i++)
1145+
{
1146+
CORINFO_EH_CLAUSE clause;
1147+
1148+
m_compHnd->getEHinfo(m_methodInfo->ftn, i, &clause);
1149+
if (m_ppOffsetToBB[clause.TryOffset] != NULL)
1150+
{
1151+
nativeEHCount++;
1152+
}
1153+
}
1154+
1155+
if (nativeEHCount == 0)
11441156
{
11451157
INTERP_DUMP(" None\n");
11461158
return;
11471159
}
11481160

1149-
m_compHnd->setEHcount(m_methodInfo->EHcount);
1161+
m_compHnd->setEHcount(nativeEHCount);
1162+
1163+
unsigned int nativeEHIndex = 0;
11501164
for (unsigned int i = 0; i < m_methodInfo->EHcount; i++)
11511165
{
11521166
CORINFO_EH_CLAUSE clause;
11531167
CORINFO_EH_CLAUSE nativeClause;
11541168

11551169
m_compHnd->getEHinfo(m_methodInfo->ftn, i, &clause);
11561170

1171+
if (m_ppOffsetToBB[clause.TryOffset] == NULL)
1172+
{
1173+
INTERP_DUMP(" Unreachable try region at IL offset %x\n", clause.TryOffset);
1174+
continue;
1175+
}
1176+
11571177
int32_t tryStartNativeOffset;
11581178
int32_t tryEndNativeOffset;
11591179
GetNativeRangeForClause(clause.TryOffset, clause.TryOffset + clause.TryLength, &tryStartNativeOffset, &tryEndNativeOffset);
@@ -1188,7 +1208,7 @@ void InterpCompiler::BuildEHInfo()
11881208
nativeClause.Flags = (CORINFO_EH_CLAUSE_FLAGS)((int)nativeClause.Flags | COR_ILEXCEPTION_CLAUSE_SAMETRY);
11891209
}
11901210

1191-
m_compHnd->setEHinfo(i, &nativeClause);
1211+
m_compHnd->setEHinfo(nativeEHIndex++, &nativeClause);
11921212

11931213
INTERP_DUMP(" try [IR_%04x(%x), IR_%04x(%x)) ", tryStartNativeOffset, clause.TryOffset, tryEndNativeOffset, clause.TryOffset + clause.TryLength);
11941214
if (clause.Flags == CORINFO_EH_CLAUSE_FILTER)
@@ -3372,7 +3392,7 @@ void InterpCompiler::EmitStaticFieldAddress(CORINFO_FIELD_INFO *pFieldInfo, CORI
33723392
}
33733393
default:
33743394
// TODO
3375-
assert(0);
3395+
assert(!"Unsupported (yet) static field accessor");
33763396
break;
33773397
}
33783398

@@ -5590,6 +5610,10 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
55905610
{
55915611
int32_t ilOffset = (int32_t)(m_ip - m_pILCode);
55925612
int32_t target = (opcode == CEE_LEAVE) ? ilOffset + 5 + *(int32_t*)(m_ip + 1) : (ilOffset + 2 + (int8_t)m_ip[1]);
5613+
if (target < 0 || target >= m_ILCodeSize)
5614+
{
5615+
BADCODE("Invalid leave target");
5616+
}
55935617
InterpBasicBlock *pTargetBB = m_ppOffsetToBB[target];
55945618

55955619
m_pStackPointer = m_pStackBase;

src/coreclr/vm/dllimport.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@
4848
#endif // FEATURE_COMINTEROP
4949

5050
#include "eventtrace.h"
51+
#ifdef FEATURE_INTERPRETER
52+
#include "interpexec.h"
53+
#endif // FEATURE_INTERPRETER
5154

5255
namespace
5356
{
@@ -5693,6 +5696,15 @@ PCODE JitILStub(MethodDesc* pStubMD)
56935696
//
56945697

56955698
pCode = pStubMD->PrepareInitialCode();
5699+
#if defined(FEATURE_INTERPRETER) && defined(FEATURE_JIT)
5700+
// Interpreter-TODO: Figure out how to create the call stub for the IL stub only when it is
5701+
// needed, like we do for the regular methods.
5702+
InterpByteCodeStart *pInterpreterCode = pStubMD->GetInterpreterCode();
5703+
if (pInterpreterCode != NULL)
5704+
{
5705+
CreateNativeToInterpreterCallStub(pInterpreterCode->Method);
5706+
}
5707+
#endif // FEATURE_INTERPRETER && FEATURE_JIT
56965708

56975709
_ASSERTE(pCode == pStubMD->GetNativeCode());
56985710
}

src/coreclr/vm/frames.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,18 @@ void InterpreterFrame::UpdateRegDisplay_Impl(const PREGDISPLAY pRD, bool updateF
19161916
{
19171917
SyncRegDisplayToCurrentContext(pRD);
19181918
TransitionFrame::UpdateRegDisplay_Impl(pRD, updateFloats);
1919+
#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
1920+
// Update the SSP to match the updated regdisplay
1921+
size_t *targetSSP = (size_t *)GetInterpExecMethodSSP();
1922+
if (targetSSP != NULL)
1923+
{
1924+
while (*targetSSP++ != pRD->ControlPC)
1925+
{
1926+
}
1927+
_ASSERTE(targetSSP != NULL);
1928+
pRD->SSP = (TADDR)targetSSP;
1929+
}
1930+
#endif // TARGET_AMD64 && TARGET_WINDOWS
19191931
}
19201932

19211933
#ifndef DACCESS_COMPILE

src/coreclr/vm/interpexec.cpp

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,44 @@ void* DoGenericLookup(void* genericVarAsPtr, InterpGenericLookup* pLookup)
458458
return result;
459459
}
460460

461+
// Filter to ignore SEH exceptions representing C++ exceptions.
462+
LONG IgnoreCppExceptionFilter(PEXCEPTION_POINTERS pExceptionInfo, PVOID pv)
463+
{
464+
return (pExceptionInfo->ExceptionRecord->ExceptionCode == EXCEPTION_MSVC)
465+
? EXCEPTION_CONTINUE_SEARCH
466+
: EXCEPTION_EXECUTE_HANDLER;
467+
}
468+
469+
// Wrapper around MethodDesc::PrepareInitialCode to handle possible managed exceptions thrown by it.
470+
void PrepareInitialCode(MethodDesc *pMD)
471+
{
472+
STATIC_STANDARD_VM_CONTRACT;
473+
474+
struct Param
475+
{
476+
MethodDesc *pMethodDesc;
477+
}
478+
param = { pMD };
479+
480+
PAL_TRY(Param *, pParam, &param)
481+
{
482+
pParam->pMethodDesc->PrepareInitialCode(CallerGCMode::Coop);
483+
}
484+
PAL_EXCEPT_FILTER(IgnoreCppExceptionFilter)
485+
{
486+
// There can be both C++ (thrown by the COMPlusThrow) and managed exceptions thrown
487+
// from the PrepareInitialCode call chain.
488+
// We need to process only managed ones here, the C++ ones are handled by the
489+
// INSTALL_/UNINSTALL_UNWIND_AND_CONTINUE_HANDLER in the InterpExecMethod.
490+
// The managed ones are represented by SEH exception, which cannot be handled there
491+
// because it is not possible to handle both SEH and C++ exceptions in the same frame.
492+
GCX_COOP_NO_DTOR();
493+
OBJECTREF ohThrowable = GET_THREAD()->LastThrownObject();
494+
DispatchManagedException(ohThrowable);
495+
}
496+
PAL_ENDTRY
497+
}
498+
461499
void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFrame *pFrame, InterpThreadContext *pThreadContext, ExceptionClauseArgs *pExceptionClauseArgs)
462500
{
463501
CONTRACTL
@@ -1886,6 +1924,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
18861924
CallStubHeader *pCallStub = (CallStubHeader*)pMethod->pDataItems[calliCookie];
18871925
ip += 5;
18881926

1927+
// Save current execution state for when we return from called method
1928+
pFrame->ip = ip;
1929+
18891930
InvokeCalliStub(LOCAL_VAR(calliFunctionPointerVar, PCODE), pCallStub, stack + callArgsOffset, stack + returnOffset);
18901931
break;
18911932
}
@@ -1907,6 +1948,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
19071948
? *(PCODE *)pMethod->pDataItems[targetAddrSlot]
19081949
: (PCODE)pMethod->pDataItems[targetAddrSlot];
19091950

1951+
// Save current execution state for when we return from called method
1952+
pFrame->ip = ip;
1953+
19101954
InlinedCallFrame inlinedCallFrame;
19111955
inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip;
19121956
inlinedCallFrame.m_pCallSiteSP = pFrame;
@@ -1942,6 +1986,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
19421986
OBJECTREF targetMethodObj = delegateObj->GetTarget();
19431987
LOCAL_VAR(callArgsOffset, OBJECTREF) = targetMethodObj;
19441988

1989+
// Save current execution state for when we return from called method
1990+
pFrame->ip = ip;
1991+
19451992
// TODO! Once we are investigating performance here, we may want to optimize this so that
19461993
// delegate calls to interpeted methods don't have to go through the native invoke here, but for
19471994
// now this should work well.
@@ -1959,6 +2006,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
19592006
CALL_INTERP_SLOT:
19602007
targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot];
19612008
CALL_INTERP_METHOD:
2009+
// Save current execution state for when we return from called method
2010+
pFrame->ip = ip;
2011+
19622012
InterpByteCodeStart* targetIp = targetMethod->GetInterpreterCode();
19632013
if (targetIp == NULL)
19642014
{
@@ -1975,7 +2025,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
19752025
// Attempt to setup the interpreter code for the target method.
19762026
if ((targetMethod->IsIL() || targetMethod->IsNoMetadata()) && !targetMethod->IsUnboxingStub())
19772027
{
1978-
targetMethod->PrepareInitialCode(CallerGCMode::Coop);
2028+
PrepareInitialCode(targetMethod);
19792029
}
19802030
targetIp = targetMethod->GetInterpreterCode();
19812031
}
@@ -1987,9 +2037,6 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
19872037
}
19882038
}
19892039

1990-
// Save current execution state for when we return from called method
1991-
pFrame->ip = ip;
1992-
19932040
// Allocate child frame.
19942041
{
19952042
InterpMethodContextFrame *pChildFrame = pFrame->pNext;
@@ -2587,7 +2634,14 @@ do \
25872634
// Unwind the interpreter stack upto the resume frame
25882635
while (pFrame != pResumeFrame)
25892636
{
2590-
assert(pFrame != NULL);
2637+
if (pFrame == NULL)
2638+
{
2639+
// In scenarios when the interpreted code ends up calling other interpreted code through the precode, there are two separate
2640+
// sequences of interpreted frames without any AOTed/JITted frames in between. In such case, the topmost native frame
2641+
// the ResumeAfterCatchException is thrown from may not be the one that corresponds to the target interpreted frame.
2642+
// Thus, we need to rethrow it to let it propagate further.
2643+
throw;
2644+
}
25912645
pThreadContext->frameDataAllocator.PopInfo(pFrame);
25922646
pFrame->ip = 0;
25932647
pFrame = pFrame->pParent;

0 commit comments

Comments
 (0)