From af76ec004c3d7c50abe29ddbfd3be37df30e21cb Mon Sep 17 00:00:00 2001
From: Jan Vorlicek <janvorli@microsoft.com>
Date: Fri, 11 Sep 2015 00:56:25 +0200
Subject: [PATCH] Fix null reference exception handling in JIT_WriteBarrier

This change fixes an issue when a null reference exception happens in the first
instruction of the JIT_WriteBarrier. There were two problems.

First problem was that the native unwinder didn't know that it is unwinding a
frame where the PC is an address of a failing instruction instead of the next
address after a call instruction. So it decremented the PC before looking up the
unwind info. Unfortunately, that means that if the hardware exception happens
in the first instruction, the unwind info is not found and the unwinder resorts
to RBP chain unwinding, which effectively skips one managed frame.

The second problem was that the FaultingExceptionFrame we create when handling
the hardware exception had context pointing to the JIT_WriteBarrier. But that
breaks the stack walker. When it arrives at the FaultingExceptionFrame, it
calls its ReturnAddress method and expects to get a managed code address.
However, in this case, it was getting the address of the JIT_WriteBarrier instead
and that made the stack walker to skip to the next explicit frame, effectively
skipping multiple managed frames that it should have reported.
---
 src/pal/src/exception/seh-unwind.cpp | 29 +++++++++++++++++++++++++----
 src/vm/exceptionhandling.cpp         | 11 ++++++++++-
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/pal/src/exception/seh-unwind.cpp b/src/pal/src/exception/seh-unwind.cpp
index 23a05b4..465c459 100644
--- a/src/pal/src/exception/seh-unwind.cpp
+++ b/src/pal/src/exception/seh-unwind.cpp
@@ -199,8 +199,6 @@ static void GetContextPointers(unw_cursor_t *cursor, unw_context_t *unwContext,
 #endif
 }
 
-#if defined(__APPLE__) || defined(__FreeBSD__) || defined(_ARM64_)
-
 static DWORD64 GetPc(CONTEXT *context)
 {
 #if defined(_AMD64_)
@@ -223,8 +221,6 @@ static void SetPc(CONTEXT *context, DWORD64 pc)
 #endif
 }
 
-#endif // defined(__APPLE__) || defined(__FreeBSD__) || defined(_ARM64_)
-
 BOOL PAL_VirtualUnwind(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers)
 {
     int st;
@@ -234,6 +230,18 @@ BOOL PAL_VirtualUnwind(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextP
     DWORD64 curPc;
 #endif
 
+    if ((context->ContextFlags & CONTEXT_EXCEPTION_ACTIVE) != 0)
+    {
+        // The current frame is a source of hardware exception. Due to the fact that
+        // we use the low level unwinder to unwind just one frame a time, the
+        // unwinder doesn't have the signal_frame flag set. So it doesn't
+        // know that it should not decrement the PC before looking up the unwind info.
+        // So we compensate it by incrementing the PC before passing it to the unwinder.
+        // Without it, the unwinder would not find unwind info if the hardware exception
+        // happened in the first instruction of a function.
+        SetPc(context, GetPc(context) + 1);
+    }
+
 #if UNWIND_CONTEXT_IS_UCONTEXT_T
     WinContextToUnwindContext(context, &unwContext);
 #else
@@ -273,6 +281,19 @@ BOOL PAL_VirtualUnwind(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextP
         return FALSE;
     }
 
+    // Check if the frame we have unwound to is a frame that caused
+    // synchronous signal, like a hardware exception and record it
+    // in the context flags.
+    st = unw_is_signal_frame(&cursor); 
+    if (st > 0)
+    {
+        context->ContextFlags |= CONTEXT_EXCEPTION_ACTIVE;
+    }
+    else
+    {
+        context->ContextFlags &= ~CONTEXT_EXCEPTION_ACTIVE;
+    }
+
     // Update the passed in windows context to reflect the unwind
     //
     UnwindContextToWinContext(&cursor, context);
diff --git a/src/vm/exceptionhandling.cpp b/src/vm/exceptionhandling.cpp
index 982c3e0..8a4fa97 100644
--- a/src/vm/exceptionhandling.cpp
+++ b/src/vm/exceptionhandling.cpp
@@ -5066,7 +5066,16 @@ VOID PALAPI HandleHardwareException(PAL_SEHException* ex)
 #endif // WIN64EXCEPTIONS
             {
                 GCX_COOP();     // Must be cooperative to modify frame chain.
-                fef.InitAndLink(&ex->ContextRecord);
+                CONTEXT context = ex->ContextRecord;
+                if (IsIPInMarkedJitHelper(controlPc))
+                {
+                    // For JIT helpers, we need to set the frame to point to the
+                    // managed code that called the helper, otherwise the stack
+                    // walker would skip all the managed frames upto the next
+                    // explicit frame.
+                    Thread::VirtualUnwindLeafCallFrame(&context);
+                }
+                fef.InitAndLink(&context);
             }
 
 #ifdef _AMD64_
-- 
2.7.4