From 85fc534fb7551728402f0b8108f0c211e3da3d89 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 8 Aug 2022 16:57:34 -0700 Subject: [PATCH 1/5] Revert "Revert "Enable inlining P/Invokes into try blocks with no catch or filter clauses (#73032)" (#73551)" This reverts commit 2a1428084c0fedc41e44e4e71b89a7d5dce3af2e. --- src/coreclr/inc/corinfo.h | 5 ++ src/coreclr/jit/importer.cpp | 50 ++++++++++--------- src/coreclr/jit/lower.cpp | 21 +++++--- src/coreclr/vm/exceptionhandling.cpp | 41 +++++++++++---- src/coreclr/vm/i386/excepx86.cpp | 21 ++++++-- .../exceptioninterop/ExceptionInterop.cs | 34 +++++++++++++ 6 files changed, 126 insertions(+), 46 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 5e97c53adb2ff4..9cfbb21fffeacb 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -3244,4 +3244,9 @@ class ICorDynamicInfo : public ICorStaticInfo // #define IMAGE_REL_BASED_REL_THUMB_MOV32_PCREL 0x14 +/**********************************************************************************/ +#ifdef TARGET_64BIT +#define USE_PER_FRAME_PINVOKE_INIT +#endif + #endif // _COR_INFO_H_ diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 6fbafbcc993313..bdf66cc22ee459 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -8428,41 +8428,43 @@ bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block) return true; } -#ifdef TARGET_64BIT - // On 64-bit platforms, we disable pinvoke inlining inside of try regions. - // Note that this could be needed on other architectures too, but we - // haven't done enough investigation to know for sure at this point. - // - // Here is the comment from JIT64 explaining why: - // [VSWhidbey: 611015] - because the jitted code links in the - // Frame (instead of the stub) we rely on the Frame not being - // 'active' until inside the stub. This normally happens by the - // stub setting the return address pointer in the Frame object - // inside the stub. On a normal return, the return address - // pointer is zeroed out so the Frame can be safely re-used, but - // if an exception occurs, nobody zeros out the return address - // pointer. Thus if we re-used the Frame object, it would go - // 'active' as soon as we link it into the Frame chain. - // - // Technically we only need to disable PInvoke inlining if we're - // in a handler or if we're in a try body with a catch or - // filter/except where other non-handler code in this method - // might run and try to re-use the dirty Frame object. - // - // A desktop test case where this seems to matter is - // jit\jit64\ebvts\mcpp\sources2\ijw\__clrcall\vector_ctor_dtor.02\deldtor_clr.exe +#ifdef USE_PER_FRAME_PINVOKE_INIT + // For platforms that use per-P/Invoke InlinedCallFrame initialization, + // we can't inline P/Invokes inside of try blocks where we can resume execution in the same function. + // The runtime can correctly unwind out of an InlinedCallFrame and out of managed code. However, + // it cannot correctly unwind out of an InlinedCallFrame and stop at that frame without also unwinding + // at least one managed frame. In particular, the runtime struggles to restore non-volatile registers + // from the top-most unmanaged call before the InlinedCallFrame. As a result, the runtime does not support + // re-entering the same method frame as the InlinedCallFrame after an exception in unmanaged code. if (block->hasTryIndex()) { // This does not apply to the raw pinvoke call that is inside the pinvoke // ILStub. In this case, we have to inline the raw pinvoke call into the stub, // otherwise we would end up with a stub that recursively calls itself, and end // up with a stack overflow. + // This works correctly because the runtime never emits a catch block in a managed-to-native + // IL stub. If the runtime ever emits a catch block into a managed-to-native stub when using + // P/Invoke helpers, this condition will need to be revisited. if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && opts.ShouldUsePInvokeHelpers()) { return true; } - return false; + // Check if this block's try block or any containing try blocks have catch handlers. + // If any of the containing try blocks have catch handlers, + // we cannot inline a P/Invoke for reasons above. If the handler is a fault or finally handler, + // we can inline a P/Invoke into this block in the try since the code will not resume execution + // in the same method after throwing an exception if only fault or finally handlers are executed. + for (unsigned int ehIndex = block->getTryIndex(); ehIndex != EHblkDsc::NO_ENCLOSING_INDEX; + ehIndex = ehGetEnclosingTryIndex(ehIndex)) + { + if (ehGetDsc(ehIndex)->HasCatchHandler()) + { + return false; + } + } + + return true; } #endif // TARGET_64BIT diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 9753615d62033d..d995e6c4ce047d 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4281,6 +4281,7 @@ GenTree* Lowering::CreateFrameLinkUpdate(FrameLinkAction action) // Return Value: // none // +// See the usages for USE_PER_FRAME_PINVOKE_INIT for more information. void Lowering::InsertPInvokeMethodProlog() { noway_assert(comp->info.compUnmanagedCallCountWithGCTransition); @@ -4377,13 +4378,16 @@ void Lowering::InsertPInvokeMethodProlog() // -------------------------------------------------------- // On 32-bit targets, CORINFO_HELP_INIT_PINVOKE_FRAME initializes the PInvoke frame and then pushes it onto // the current thread's Frame stack. On 64-bit targets, it only initializes the PInvoke frame. + // As a result, don't push the frame onto the frame stack here for any 64-bit targets CLANG_FORMAT_COMMENT_ANCHOR; #ifdef TARGET_64BIT +#ifdef USE_PER_FRAME_PINVOKE_INIT + // For IL stubs, we push the frame once even when we're doing per-pinvoke init. if (comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB)) +#endif // USE_PER_FRAME_PINVOKE_INIT { - // Push a frame - if we are NOT in an IL stub, this is done right before the call - // The init routine sets InlinedCallFrame's m_pNext, so we just set the thead's top-of-stack + // Push a frame. The init routine sets InlinedCallFrame's m_pNext, so we just set the thread's top-of-stack GenTree* frameUpd = CreateFrameLinkUpdate(PushFrame); firstBlockRange.InsertBefore(insertionPoint, LIR::SeqTree(comp, frameUpd)); ContainCheckStoreIndir(frameUpd->AsStoreInd()); @@ -4443,9 +4447,10 @@ void Lowering::InsertPInvokeMethodEpilog(BasicBlock* returnBB DEBUGARG(GenTree* // this in the epilog for IL stubs; for non-IL stubs the frame is popped after every PInvoke call. CLANG_FORMAT_COMMENT_ANCHOR; -#ifdef TARGET_64BIT +#ifdef USE_PER_FRAME_PINVOKE_INIT + // For IL stubs, we push the frame once even when we're doing per-pinvoke init if (comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB)) -#endif // TARGET_64BIT +#endif // USE_PER_FRAME_PINVOKE_INIT { GenTree* frameUpd = CreateFrameLinkUpdate(PopFrame); returnBlockRange.InsertBefore(insertionPoint, LIR::SeqTree(comp, frameUpd)); @@ -4601,7 +4606,7 @@ void Lowering::InsertPInvokeCallProlog(GenTreeCall* call) // contains PInvokes; on 64-bit targets this is necessary in non-stubs. CLANG_FORMAT_COMMENT_ANCHOR; -#ifdef TARGET_64BIT +#ifdef USE_PER_FRAME_PINVOKE_INIT if (!comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB)) { // Set the TCB's frame to be the one we just created. @@ -4613,7 +4618,7 @@ void Lowering::InsertPInvokeCallProlog(GenTreeCall* call) BlockRange().InsertBefore(insertBefore, LIR::SeqTree(comp, frameUpd)); ContainCheckStoreIndir(frameUpd->AsStoreInd()); } -#endif // TARGET_64BIT +#endif // USE_PER_FRAME_PINVOKE_INIT // IMPORTANT **** This instruction must be the last real instruction **** // It changes the thread's state to Preemptive mode @@ -4679,7 +4684,7 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) // this happens after every PInvoke call in non-stubs. 32-bit targets instead mark the frame as inactive. CLANG_FORMAT_COMMENT_ANCHOR; -#ifdef TARGET_64BIT +#ifdef USE_PER_FRAME_PINVOKE_INIT if (!comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB)) { tree = CreateFrameLinkUpdate(PopFrame); @@ -4703,7 +4708,7 @@ void Lowering::InsertPInvokeCallEpilog(GenTreeCall* call) BlockRange().InsertBefore(insertionPoint, constantZero, storeCallSiteTracker); ContainCheckStoreLoc(storeCallSiteTracker); -#endif // TARGET_64BIT +#endif // USE_PER_FRAME_PINVOKE_INIT } //------------------------------------------------------------------------ diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 21fe40769384be..5ec98e6f8acaa6 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -16,6 +16,7 @@ #include "virtualcallstub.h" #include "utilcode.h" #include "interoplibinterface.h" +#include "corinfo.h" #if defined(TARGET_X86) #define USE_CURRENT_CONTEXT_IN_FILTER @@ -1776,8 +1777,10 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // InlinedCallFrames (ICF) are allocated, initialized and linked to the Frame chain // by the code generated by the JIT for a method containing a PInvoke. // - // JIT generates code that links in the ICF at the start of the method and unlinks it towards - // the method end. Thus, ICF is present on the Frame chain at any given point so long as the + // On platforms where USE_PER_FRAME_PINVOKE_INIT is not defined, + // the JIT generates code that links in the ICF + // at the start of the method and unlinks it towards the method end. + // Thus, ICF is present on the Frame chain at any given point so long as the // method containing the PInvoke is on the stack. // // Now, if the method containing ICF catches an exception, we will reset the Frame chain @@ -1815,13 +1818,16 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // below the callerSP for which we will invoke ExceptionUnwind. // // Thus, ICF::ExceptionUnwind should not do anything significant. If any of these assumptions - // break, then the next best thing will be to make the JIT link/unlink the frame dynamically. + // break, then the next best thing will be to make the JIT link/unlink the frame dynamically // - // If the current method executing is from precompiled ReadyToRun code, then the above is no longer - // applicable because each PInvoke is wrapped by calls to the JIT_PInvokeBegin and JIT_PInvokeEnd - // helpers, which push and pop the ICF to the current thread. Unlike jitted code, the ICF is not - // linked during the method prolog, and unlinked at the epilog (it looks more like the X64 case). + // If the current method executing is from precompiled ReadyToRun code, each PInvoke is wrapped + // by calls to the JIT_PInvokeBegin and JIT_PInvokeEnd helpers, + // which push and pop the ICF to the current thread. The ICF is not + // linked during the method prolog, and unlinked at the epilog. // In that case, we need to unlink the ICF during unwinding here. + // On platforms where USE_PER_FRAME_PINVOKE_INIT is defined, the JIT generates code that links in + // the ICF immediately before and after a PInvoke in non-IL-stubs, like ReadyToRun. + // See the usages for USE_PER_FRAME_PINVOKE_INIT for more information. if (fTargetUnwind && (pFrame->GetVTablePtr() == InlinedCallFrame::GetMethodFrameVPtr())) { @@ -1830,8 +1836,12 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // // 1) ICF address is higher than the current frame's SP (which we get from DispatcherContext), AND // 2) ICF address is below callerSP. - if ((GetSP(pDispatcherContext->ContextRecord) < (TADDR)pICF) && - ((UINT_PTR)pICF < uCallerSP)) + // 3) ICF is active. + // - IL stubs link the frame in for the whole stub, so if an exception is thrown during marshalling, + // the ICF will be on the frame chain and inactive. + if ((GetSP(pDispatcherContext->ContextRecord) < (TADDR)pICF) + && ((UINT_PTR)pICF < uCallerSP) + && InlinedCallFrame::FrameHasActiveCall(pICF)) { pICFForUnwindTarget = pFrame; @@ -1840,9 +1850,18 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // to the JIT_PInvokeBegin and JIT_PInvokeEnd helpers, which push and pop the ICF on the thread. The // ICF is not linked at the method prolog and unlined at the epilog when running R2R code. Since the // JIT_PInvokeEnd helper will be skipped, we need to unlink the ICF here. If the executing method - // has another pinovoke, it will re-link the ICF again when the JIT_PInvokeBegin helper is called + // has another pinvoke, it will re-link the ICF again when the JIT_PInvokeBegin helper is called. - if (ExecutionManager::IsReadyToRunCode(((InlinedCallFrame*)pFrame)->m_pCallerReturnAddress)) + TADDR returnAddress = ((InlinedCallFrame*)pFrame)->m_pCallerReturnAddress; +#ifdef USE_PER_FRAME_PINVOKE_INIT + // If we're setting up the frame for each P/Invoke for the given platform, + // then we do this for all P/Invokes except ones in IL stubs. + if (!ExecutionManager::GetCodeMethodDesc(returnAddress)->IsILStub()) +#else + // If we aren't setting up the frame for each P/Invoke (instead setting up once per method), + // then ReadyToRun code is the only code using the per-P/Invoke logic. + if (ExecutionManager::IsReadyToRunCode(returnAddress)) +#endif { pICFForUnwindTarget = pICFForUnwindTarget->Next(); } diff --git a/src/coreclr/vm/i386/excepx86.cpp b/src/coreclr/vm/i386/excepx86.cpp index 68c68f7f258a0e..54576261f2e3ef 100644 --- a/src/coreclr/vm/i386/excepx86.cpp +++ b/src/coreclr/vm/i386/excepx86.cpp @@ -28,6 +28,7 @@ #include "eeconfig.h" #include "vars.hpp" #include "generics.h" +#include "corinfo.h" #include "asmconstants.h" #include "virtualcallstub.h" @@ -2970,6 +2971,8 @@ void ResumeAtJitEH(CrawlFrame* pCf, // Check that the InlinedCallFrame is in the method with the exception handler. There can be other // InlinedCallFrame somewhere up the call chain that is not related to the current exception // handling. + + // See the usages for USE_PER_FRAME_PINVOKE_INIT for more information. #ifdef DEBUG TADDR handlerFrameSP = pCf->GetRegisterSet()->SP; @@ -2982,10 +2985,22 @@ void ResumeAtJitEH(CrawlFrame* pCf, NULL /* StackwalkCacheUnwindInfo* */); _ASSERTE(unwindSuccess); - if (((TADDR)pThread->m_pFrame < pCf->GetRegisterSet()->SP) && ExecutionManager::IsReadyToRunCode(((InlinedCallFrame*)pThread->m_pFrame)->m_pCallerReturnAddress)) + if (((TADDR)pThread->m_pFrame < pCf->GetRegisterSet()->SP)) { - _ASSERTE((TADDR)pThread->m_pFrame >= handlerFrameSP); - pThread->m_pFrame->Pop(pThread); + TADDR returnAddress = ((InlinedCallFrame*)pThread->m_pFrame)->m_pCallerReturnAddress; +#ifdef USE_PER_FRAME_PINVOKE_INIT + // If we're setting up the frame for each P/Invoke for the given platform, + // then we do this for all P/Invokes except ones in IL stubs. + if (!ExecutionManager::GetCodeMethodDesc(returnAddress)->IsILStub()) +#else + // If we aren't setting up the frame for each P/Invoke (instead setting up once per method), + // then ReadyToRun code is the only code using the per-P/Invoke logic. + if (ExecutionManager::IsReadyToRunCode(returnAddress)) +#endif + { + _ASSERTE((TADDR)pThread->m_pFrame >= handlerFrameSP); + pThread->m_pFrame->Pop(pThread); + } } } diff --git a/src/tests/baseservices/exceptions/exceptioninterop/ExceptionInterop.cs b/src/tests/baseservices/exceptions/exceptioninterop/ExceptionInterop.cs index 1c6f43e12e4a45..f0f479dfc55b11 100644 --- a/src/tests/baseservices/exceptions/exceptioninterop/ExceptionInterop.cs +++ b/src/tests/baseservices/exceptions/exceptioninterop/ExceptionInterop.cs @@ -122,4 +122,38 @@ public static void ThrowNativeExceptionAndCatchInFrameWithFinally() Assert.True(caughtException); } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + [SkipOnMono("Exception interop not supported on Mono.")] + public static void ThrowNativeExceptionInFrameWithFinallyCatchInOuterFrame() + { + bool caughtException = false; + try + { + ThrowInFrameWithFinally(); + } + catch + { + caughtException = true; + } + + Assert.True(caughtException); + + [MethodImpl(MethodImplOptions.NoInlining)] + static void ThrowInFrameWithFinally() + { + try + { + ThrowException(); + } + finally + { + // Try calling another P/Invoke in the finally block before the catch + // to make sure we have everything set up + // to recover from the exceptional control flow. + NativeFunction(); + } + } + } } From faaca3898d09481ce41d2bc78c409f2acc6a312c Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 9 Aug 2022 13:46:06 -0700 Subject: [PATCH 2/5] Try reorganizing the code slightly to have the exact same code flow as beforehand on ARM32 --- src/coreclr/vm/exceptionhandling.cpp | 10 +++++----- src/coreclr/vm/i386/excepx86.cpp | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 5ec98e6f8acaa6..0f9f4831133cab 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -1837,11 +1837,9 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // 1) ICF address is higher than the current frame's SP (which we get from DispatcherContext), AND // 2) ICF address is below callerSP. // 3) ICF is active. - // - IL stubs link the frame in for the whole stub, so if an exception is thrown during marshalling, - // the ICF will be on the frame chain and inactive. + // - if ((GetSP(pDispatcherContext->ContextRecord) < (TADDR)pICF) - && ((UINT_PTR)pICF < uCallerSP) - && InlinedCallFrame::FrameHasActiveCall(pICF)) + && ((UINT_PTR)pICF < uCallerSP)) { pICFForUnwindTarget = pFrame; @@ -1856,7 +1854,9 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( #ifdef USE_PER_FRAME_PINVOKE_INIT // If we're setting up the frame for each P/Invoke for the given platform, // then we do this for all P/Invokes except ones in IL stubs. - if (!ExecutionManager::GetCodeMethodDesc(returnAddress)->IsILStub()) + // IL stubs link the frame in for the whole stub, so if an exception is thrown during marshalling, + // the ICF will be on the frame chain and inactive. + if (returnAddress != NULL && !ExecutionManager::GetCodeMethodDesc(returnAddress)->IsILStub()) #else // If we aren't setting up the frame for each P/Invoke (instead setting up once per method), // then ReadyToRun code is the only code using the per-P/Invoke logic. diff --git a/src/coreclr/vm/i386/excepx86.cpp b/src/coreclr/vm/i386/excepx86.cpp index 54576261f2e3ef..72456b7777dc6b 100644 --- a/src/coreclr/vm/i386/excepx86.cpp +++ b/src/coreclr/vm/i386/excepx86.cpp @@ -2971,7 +2971,7 @@ void ResumeAtJitEH(CrawlFrame* pCf, // Check that the InlinedCallFrame is in the method with the exception handler. There can be other // InlinedCallFrame somewhere up the call chain that is not related to the current exception // handling. - + // See the usages for USE_PER_FRAME_PINVOKE_INIT for more information. #ifdef DEBUG @@ -2991,7 +2991,7 @@ void ResumeAtJitEH(CrawlFrame* pCf, #ifdef USE_PER_FRAME_PINVOKE_INIT // If we're setting up the frame for each P/Invoke for the given platform, // then we do this for all P/Invokes except ones in IL stubs. - if (!ExecutionManager::GetCodeMethodDesc(returnAddress)->IsILStub()) + if (returnAddress != NULL && !ExecutionManager::GetCodeMethodDesc(returnAddress)->IsILStub()) #else // If we aren't setting up the frame for each P/Invoke (instead setting up once per method), // then ReadyToRun code is the only code using the per-P/Invoke logic. From edb2998cb78cc0b57051278c387240419792e5c2 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 9 Aug 2022 14:38:22 -0700 Subject: [PATCH 3/5] Reenable test --- src/tests/issues.targets | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index c2e6cc66043921..f660de28526f18 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -109,9 +109,6 @@ https://github.com/dotnet/runtime/issues/ - - https://github.com/dotnet/runtime/issues/73247 - times out From 32e3cf8941d51560db268a0cd533ed9bd559f60a Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 15 Aug 2022 16:03:41 -0700 Subject: [PATCH 4/5] Try just changing the lock definition file since I can't repro the failure locally. --- src/coreclr/inc/CrstTypes.def | 2 +- src/coreclr/inc/crsttypes.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index 3d2cdcf8203b18..493718b57a0059 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -202,7 +202,7 @@ Crst Exception End Crst ExecutableAllocatorLock - AcquiredAfter LoaderHeap ArgBasedStubCache UMEntryThunkFreeListLock + AcquiredAfter LoaderHeap ArgBasedStubCache UMEntryThunkFreeListLock COMCallWrapper End Crst ExecuteManRangeLock diff --git a/src/coreclr/inc/crsttypes.h b/src/coreclr/inc/crsttypes.h index b5e52aef0a4afb..4ce24a712e0485 100644 --- a/src/coreclr/inc/crsttypes.h +++ b/src/coreclr/inc/crsttypes.h @@ -158,7 +158,7 @@ int g_rgCrstLevelMap[] = -1, // CrstClrNotification 6, // CrstCodeFragmentHeap 9, // CrstCodeVersioning - 0, // CrstCOMCallWrapper + 3, // CrstCOMCallWrapper 5, // CrstCOMWrapperCache 3, // CrstDataTest1 0, // CrstDataTest2 From 69c7616d4da9c2ca280292041e808355e8e0d345 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 18 Aug 2022 10:12:34 -0700 Subject: [PATCH 5/5] Apply suggestions from code review --- src/coreclr/vm/exceptionhandling.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 0f9f4831133cab..b3f15526c9d861 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -1836,10 +1836,8 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // // 1) ICF address is higher than the current frame's SP (which we get from DispatcherContext), AND // 2) ICF address is below callerSP. - // 3) ICF is active. - // - - if ((GetSP(pDispatcherContext->ContextRecord) < (TADDR)pICF) - && ((UINT_PTR)pICF < uCallerSP)) + if ((GetSP(pDispatcherContext->ContextRecord) < (TADDR)pICF) && + ((UINT_PTR)pICF < uCallerSP)) { pICFForUnwindTarget = pFrame;