From 80bca845e0071e59c82f512429ecd8e11bbf81a3 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 26 Jun 2025 16:49:39 +0200 Subject: [PATCH 1/4] Fix IP adjustment for interpreter EH The exception handling stack frame iterator adjusts IP it returns for all cases when the IP represents a return address. This is to make sure that if the last instruction in a try regios is a call, it is still covered by the try region (the return address would be right after it). For jit/aoted code, throwing a software exceptions results in a call. Only hardware exceptions like division by zero etc report the address of the failing instruction. In the interpreter, when an exception is thrown, the IP address is always the address of the instruction that has triggered the exception. So we should not adjust the IP. This change fixes that by marking exception throwing frame as "faulting", which ensures the adjustment doesn't occur. That information is recorded in the InterpreterFrame when an exception is thrown by the interpreter code and cleared when the execution resumes after catch. --- src/coreclr/vm/excep.cpp | 4 ++++ src/coreclr/vm/frames.cpp | 5 +++++ src/coreclr/vm/frames.h | 11 ++++++++++- src/coreclr/vm/interpexec.cpp | 4 ++++ src/coreclr/vm/stackwalk.cpp | 6 +++++- 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index e0863771d4588c..cf81ee468c27bc 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -7360,6 +7360,10 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra } else { + if (pEntryFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) + { + ((InterpreterFrame*)pEntryFrame)->SetIsFaulting(true); + } DispatchManagedException(orThrowable); } } diff --git a/src/coreclr/vm/frames.cpp b/src/coreclr/vm/frames.cpp index ad434fb4464c60..53ff4599c8453c 100644 --- a/src/coreclr/vm/frames.cpp +++ b/src/coreclr/vm/frames.cpp @@ -1894,6 +1894,11 @@ void InterpreterFrame::SetContextToInterpMethodContextFrame(T_CONTEXT * pContext SetSP(pContext, dac_cast(pFrame)); SetFP(pContext, (TADDR)pFrame->pStack); SetFirstArgReg(pContext, dac_cast(this)); + pContext->ContextFlags = CONTEXT_FULL; + if (m_isFaulting) + { + pContext->ContextFlags |= CONTEXT_EXCEPTION_ACTIVE; + } } void InterpreterFrame::UpdateRegDisplay_Impl(const PREGDISPLAY pRD, bool updateFloats) diff --git a/src/coreclr/vm/frames.h b/src/coreclr/vm/frames.h index 892e55586d7abd..5a7d4e70ff29c3 100644 --- a/src/coreclr/vm/frames.h +++ b/src/coreclr/vm/frames.h @@ -2417,7 +2417,8 @@ class InterpreterFrame : public FramedMethodFrame #ifndef DACCESS_COMPILE InterpreterFrame(TransitionBlock* pTransitionBlock, InterpMethodContextFrame* pContextFrame) : FramedMethodFrame(FrameIdentifier::InterpreterFrame, pTransitionBlock, NULL), - m_pTopInterpMethodContextFrame(pContextFrame) + m_pTopInterpMethodContextFrame(pContextFrame), + m_isFaulting(false) #if defined(HOST_AMD64) && defined(HOST_WINDOWS) , m_SSP(0) #endif @@ -2471,10 +2472,18 @@ class InterpreterFrame : public FramedMethodFrame } #endif // HOST_AMD64 && HOST_WINDOWS + void SetIsFaulting(bool isFaulting) + { + LIMITED_METHOD_CONTRACT; + m_isFaulting = isFaulting; + } + private: // The last known topmost interpreter frame in the InterpExecMethod belonging to // this InterpreterFrame. PTR_InterpMethodContextFrame m_pTopInterpMethodContextFrame; + // Set to true to indicate that the topmost interpreted frame has thrown an exception + bool m_isFaulting; #if defined(HOST_AMD64) && defined(HOST_WINDOWS) // Saved SSP of the InterpExecMethod for resuming after catch into interpreter frames. TADDR m_SSP; diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 372e8b2cfbafc6..bddc4b77c16237 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1737,12 +1737,14 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { throwable = LOCAL_VAR(ip[1], OBJECTREF); } + pInterpreterFrame->SetIsFaulting(true); DispatchManagedException(throwable); UNREACHABLE(); break; } case INTOP_RETHROW: { + pInterpreterFrame->SetIsFaulting(true); DispatchRethrownManagedException(); UNREACHABLE(); break; @@ -2220,6 +2222,8 @@ do { \ pMethod = pFrame->startIp->Method; assert(pMethod->CheckIntegrity()); pThreadContext->pStackPointer = pFrame->pStack + pMethod->allocaSize; + + pInterpreterFrame->SetIsFaulting(false); goto MAIN_LOOP; } diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index 103aa7fe0db6db..c7275c7d9cbefa 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -2860,8 +2860,12 @@ void StackFrameIterator::ProcessCurrentFrame(void) m_interpExecMethodFirstArgReg = (TADDR)GetFirstArgReg(pRD->pCurrentContext); ((PTR_InterpreterFrame)m_crawl.pFrame)->SetContextToInterpMethodContextFrame(pRD->pCurrentContext); + if (pRD->pCurrentContext->ContextFlags & CONTEXT_EXCEPTION_ACTIVE) + { + m_crawl.isInterrupted = true; + m_crawl.hasFaulted = true; + } - pRD->pCurrentContext->ContextFlags = CONTEXT_FULL; SyncRegDisplayToCurrentContext(pRD); ProcessIp(GetControlPC(pRD)); m_walkingInterpreterFrames = m_crawl.isFrameless; From 91137ae59ee5f8474b784cadeef06a73c97723ec Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 26 Jun 2025 19:13:34 +0200 Subject: [PATCH 2/4] Fix build break with interpreter disabled --- src/coreclr/vm/excep.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index cf81ee468c27bc..1a534db3dbfb8f 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -7360,10 +7360,12 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra } else { +#ifdef FEATURE_INTERPRETER if (pEntryFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) { ((InterpreterFrame*)pEntryFrame)->SetIsFaulting(true); } +#endif // FEATURE_INTERPRETER DispatchManagedException(orThrowable); } } From 7a5611b7171af6693d158807e87f6624e03118d3 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 26 Jun 2025 22:36:21 +0200 Subject: [PATCH 3/4] Fix the case when the pEntryFrame is NULL --- src/coreclr/vm/excep.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 1a534db3dbfb8f..b607f488a01591 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -7361,7 +7361,7 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra else { #ifdef FEATURE_INTERPRETER - if (pEntryFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) + if ((pEntryFrame != NULL) && (pEntryFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame)) { ((InterpreterFrame*)pEntryFrame)->SetIsFaulting(true); } From e5df47ce3de858ab129c0d9245ec529c5526b468 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 26 Jun 2025 22:47:35 +0200 Subject: [PATCH 4/4] Actually, the check should be for FRAME_TOP, not NULL --- src/coreclr/vm/excep.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index b607f488a01591..2fe3fba668d368 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -7361,7 +7361,7 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra else { #ifdef FEATURE_INTERPRETER - if ((pEntryFrame != NULL) && (pEntryFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame)) + if ((pEntryFrame != FRAME_TOP) && (pEntryFrame->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame)) { ((InterpreterFrame*)pEntryFrame)->SetIsFaulting(true); }