From de325a4de386f416230045478fd36af37d6a341f Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 27 Feb 2024 20:20:23 +0100 Subject: [PATCH] Fix the new EH with hot-cold split code The new EH was not taking into account the fact that with hot-cold split methods, the offset used for looking up EH funclets need to be computed as if the hot and cold regions were consecutive in memory. That have caused failures in a number of tests when the tests themselves were compiled with R2R and hot-cold split enabled. Close #98915, #98916, #98917, #98918 --- .../ExceptionServices/InternalCalls.cs | 2 +- .../src/System/Runtime/ExceptionHandling.cs | 35 +++++++++++++++---- .../src/System/Runtime/InternalCalls.cs | 2 +- src/coreclr/nativeaot/Runtime/EHHelpers.cpp | 16 +++++++-- src/coreclr/vm/exceptionhandling.cpp | 4 +-- src/coreclr/vm/exceptionhandlingqcalls.h | 2 +- 6 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/InternalCalls.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/InternalCalls.cs index 4ae608fc17d23d..228f58c0ea4d06 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/InternalCalls.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/InternalCalls.cs @@ -42,7 +42,7 @@ internal static unsafe partial bool RhpCallFilterFunclet( [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "EHEnumInitFromStackFrameIterator")] [return: MarshalAs(UnmanagedType.Bool)] - internal static unsafe partial bool RhpEHEnumInitFromStackFrameIterator(ref StackFrameIterator pFrameIter, byte** pMethodStartAddress, void* pEHEnum); + internal static unsafe partial bool RhpEHEnumInitFromStackFrameIterator(ref StackFrameIterator pFrameIter, out EH.MethodRegionInfo pMethodRegionInfo, void* pEHEnum); [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "EHEnumNext")] [return: MarshalAs(UnmanagedType.Bool)] diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs index c8d9a0a74c2645..89855a54113d2e 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs @@ -68,6 +68,14 @@ private struct EHEnum private IntPtr _dummy; // For alignment } + internal struct MethodRegionInfo + { + internal byte* _hotStartAddress; + internal nuint _hotSize; + internal byte* _coldStartAddress; + internal nuint _coldSize; + } + #pragma warning disable IDE0060 // This is a fail-fast function used by the runtime as a last resort that will terminate the process with // as little effort as possible. No guarantee is made about the semantics of this fail-fast. @@ -932,6 +940,20 @@ private static void DebugVerifyHandlingFrame(UIntPtr handlingFrameSP) "Handling frame must have a valid stack frame pointer"); } + // Caclulate the code offset from the start of the method as if the hot and cold regions were + // stored sequentially in memory. + private static uint CalculateCodeOffset(byte* pbControlPC, in MethodRegionInfo methodRegionInfo) + { + uint codeOffset = (uint)(pbControlPC - methodRegionInfo._hotStartAddress); + // If the PC is in the cold region, adjust the offset to be relative to the start of the method. + if ((methodRegionInfo._coldSize != 0) && (codeOffset >= methodRegionInfo._hotSize)) + { + codeOffset = (uint)(methodRegionInfo._hotSize + (nuint)(pbControlPC - methodRegionInfo._coldStartAddress)); + } + + return codeOffset; + } + private static void UpdateStackTrace(object exceptionObj, UIntPtr curFramePtr, IntPtr ip, UIntPtr sp, ref bool isFirstRethrowFrame, ref UIntPtr prevFramePtr, ref bool isFirstFrame, ref ExInfo exInfo) { @@ -958,13 +980,13 @@ private static bool FindFirstPassHandler(object exception, uint idxStart, tryRegionIdx = MaxTryRegionIdx; EHEnum ehEnum; - byte* pbMethodStartAddress; - if (!InternalCalls.RhpEHEnumInitFromStackFrameIterator(ref frameIter, &pbMethodStartAddress, &ehEnum)) + MethodRegionInfo methodRegionInfo; + if (!InternalCalls.RhpEHEnumInitFromStackFrameIterator(ref frameIter, out methodRegionInfo, &ehEnum)) return false; byte* pbControlPC = frameIter.ControlPC; - uint codeOffset = (uint)(pbControlPC - pbMethodStartAddress); + uint codeOffset = CalculateCodeOffset(pbControlPC, in methodRegionInfo); uint lastTryStart = 0, lastTryEnd = 0; @@ -1111,13 +1133,14 @@ private static void InvokeSecondPass(ref ExInfo exInfo, uint idxStart) private static void InvokeSecondPass(ref ExInfo exInfo, uint idxStart, uint idxLimit) { EHEnum ehEnum; - byte* pbMethodStartAddress; - if (!InternalCalls.RhpEHEnumInitFromStackFrameIterator(ref exInfo._frameIter, &pbMethodStartAddress, &ehEnum)) + MethodRegionInfo methodRegionInfo; + + if (!InternalCalls.RhpEHEnumInitFromStackFrameIterator(ref exInfo._frameIter, out methodRegionInfo, &ehEnum)) return; byte* pbControlPC = exInfo._frameIter.ControlPC; - uint codeOffset = (uint)(pbControlPC - pbMethodStartAddress); + uint codeOffset = CalculateCodeOffset(pbControlPC, in methodRegionInfo); uint lastTryStart = 0, lastTryEnd = 0; diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs index 806208187e92fc..8cf281392abf04 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs @@ -177,7 +177,7 @@ internal static int RhEndNoGCRegion() [RuntimeImport(Redhawk.BaseName, "RhpEHEnumInitFromStackFrameIterator")] [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern unsafe bool RhpEHEnumInitFromStackFrameIterator(ref StackFrameIterator pFrameIter, byte** pMethodStartAddress, void* pEHEnum); + internal static extern unsafe bool RhpEHEnumInitFromStackFrameIterator(ref StackFrameIterator pFrameIter, out EH.MethodRegionInfo pMethodRegionInfo, void* pEHEnum); [RuntimeImport(Redhawk.BaseName, "RhpEHEnumNext")] [MethodImpl(MethodImplOptions.InternalCall)] diff --git a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp index 28cb9e617f09bb..1a54b9bcc9b55b 100644 --- a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp @@ -29,13 +29,25 @@ #include "MethodTable.inl" #include "CommonMacros.inl" +struct MethodRegionInfo +{ + void* hotStartAddress; + size_t hotSize; + void* coldStartAddress; + size_t coldSize; +}; + COOP_PINVOKE_HELPER(FC_BOOL_RET, RhpEHEnumInitFromStackFrameIterator, ( - StackFrameIterator* pFrameIter, void ** pMethodStartAddressOut, EHEnum* pEHEnum)) + StackFrameIterator* pFrameIter, MethodRegionInfo* pMethodRegionInfoOut, EHEnum* pEHEnum)) { ICodeManager * pCodeManager = pFrameIter->GetCodeManager(); pEHEnum->m_pCodeManager = pCodeManager; - FC_RETURN_BOOL(pCodeManager->EHEnumInit(pFrameIter->GetMethodInfo(), pMethodStartAddressOut, &pEHEnum->m_state)); + pMethodRegionInfoOut->hotSize = 0; // unknown + pMethodRegionInfoOut->coldStartAddress = nullptr; + pMethodRegionInfoOut->coldSize = 0; + + FC_RETURN_BOOL(pCodeManager->EHEnumInit(pFrameIter->GetMethodInfo(), &pMethodRegionInfoOut->hotStartAddress, &pEHEnum->m_state)); } COOP_PINVOKE_HELPER(FC_BOOL_RET, RhpEHEnumNext, (EHEnum* pEHEnum, EHClause* pEHClause)) diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index beb919c5b226b3..cb50ee8fe66efc 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -7970,7 +7970,7 @@ struct ExtendedEHClauseEnumerator : EH_CLAUSE_ENUMERATOR unsigned EHCount; }; -extern "C" BOOL QCALLTYPE EHEnumInitFromStackFrameIterator(StackFrameIterator *pFrameIter, BYTE** pMethodStartAddress, EH_CLAUSE_ENUMERATOR * pEHEnum) +extern "C" BOOL QCALLTYPE EHEnumInitFromStackFrameIterator(StackFrameIterator *pFrameIter, IJitManager::MethodRegionInfo* pMethodRegionInfo, EH_CLAUSE_ENUMERATOR * pEHEnum) { QCALL_CONTRACT; @@ -7984,7 +7984,7 @@ extern "C" BOOL QCALLTYPE EHEnumInitFromStackFrameIterator(StackFrameIterator *p IJitManager* pJitMan = pFrameIter->m_crawl.GetJitManager(); const METHODTOKEN& MethToken = pFrameIter->m_crawl.GetMethodToken(); - *pMethodStartAddress = (BYTE*)pJitMan->JitTokenToStartAddress(MethToken); + pJitMan->JitTokenToMethodRegionInfo(MethToken, pMethodRegionInfo); pExtendedEHEnum->EHCount = pJitMan->InitializeEHEnumeration(MethToken, pEHEnum); END_QCALL; diff --git a/src/coreclr/vm/exceptionhandlingqcalls.h b/src/coreclr/vm/exceptionhandlingqcalls.h index 7747c14f531d2f..7054080cef3c00 100644 --- a/src/coreclr/vm/exceptionhandlingqcalls.h +++ b/src/coreclr/vm/exceptionhandlingqcalls.h @@ -17,7 +17,7 @@ extern "C" void QCALLTYPE CallFinallyFunclet(BYTE* pHandlerIP, REGDISPLAY* pvReg extern "C" BOOL QCALLTYPE CallFilterFunclet(QCall::ObjectHandleOnStack exceptionObj, BYTE* pFilterP, REGDISPLAY* pvRegDisplay); extern "C" void QCALLTYPE ResumeAtInterceptionLocation(REGDISPLAY* pvRegDisplay); extern "C" void QCALLTYPE AppendExceptionStackFrame(QCall::ObjectHandleOnStack exceptionObj, SIZE_T ip, SIZE_T sp, int flags, ExInfo *pExInfo); -extern "C" BOOL QCALLTYPE EHEnumInitFromStackFrameIterator(StackFrameIterator *pFrameIter, BYTE** pMethodStartAddress, EH_CLAUSE_ENUMERATOR * pEHEnum); +extern "C" BOOL QCALLTYPE EHEnumInitFromStackFrameIterator(StackFrameIterator *pFrameIter, IJitManager::MethodRegionInfo *pMethodRegionInfo, EH_CLAUSE_ENUMERATOR * pEHEnum); extern "C" BOOL QCALLTYPE EHEnumNext(EH_CLAUSE_ENUMERATOR* pEHEnum, RhEHClause* pEHClause); extern "C" bool QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStackwalkCtx, bool instructionFault, bool* pIsExceptionIntercepted); extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, unsigned int* uExCollideClauseIdx, bool* fUnwoundReversePInvoke, bool* pIsExceptionIntercepted);