Fix one more exception handling issue

This change fixes exception handling issue that resulted in a finally block being invoked multiple
times in case of a rethrown exception in some specific cases.
The problem was caused by the ExceptionTracker::StackRange::CombineWith that was not handling
correctly the case when the current stack range was empty. It was copying only the upper limit
from the previous range and leaving the lower limit at the default (max) value.
This case happens in the interleaved exception handling when processing the first managed
frame after a block of native frames was unwound.
There is one additional fix in the UnwindManagedExceptionPass1 where we were popping one
frame from the thread's frame list before performing the native frames unwind. This was
not correct since in some cases, there can be more frames that need to be removed.

diff --git a/src/vm/exceptionhandling.cpp b/src/vm/exceptionhandling.cpp
index 3996318..6886f4c 100644 (file)
--- a/src/vm/exceptionhandling.cpp
+++ b/src/vm/exceptionhandling.cpp
@@ -4587,10 +4587,10 @@ VOID DECLSPEC_NORETURN UnwindManagedExceptionPass1(PAL_SEHException& ex)
 
             *currentFlags = firstPassFlags;
 
-            // Pop the last managed frame so that when the native frames are unwound and
-            // the UnwindManagedExceptionPass1 is resumed at the next managed frame, that
-            // managed frame is the current one set in the thread object.
-            GetThread()->GetFrame()->Pop();
+            // Pop all frames that are below the block of native frames and that would be
+            // in the unwound part of the stack when UnwindManagedExceptionPass1 is resumed 
+            // at the next managed frame.
+            UnwindFrameChain(GetThread(), (VOID*)frameContext.Rsp);
 
             // Now we need to unwind the native frames until we reach managed frames again or the exception is
             // handled in the native code.
@@ -5527,6 +5527,15 @@ void ExceptionTracker::StackRange::CombineWith(StackFrame sfCurrent, StackRange*
     }
     else
     {
+#ifdef FEATURE_PAL
+        // When the current range is empty, copy the low bound too. Otherwise a degenerate range would get
+        // created and tests for stack frame in the stack range would always fail.
+        // TODO: Check if we could enable it for non-PAL as well.
+        if (IsEmpty())
+        {
+            m_sfLowBound = pPreviousRange->m_sfLowBound;
+        }
+#endif // FEATURE_PAL
         m_sfHighBound = pPreviousRange->m_sfHighBound;
     }
 }