From 32741b9e97f901242394dfc682c228a5d810f454 Mon Sep 17 00:00:00 2001
From: Juan Sebastian Hoyos Ayala <juan.hoyos@microsoft.com>
Date: Mon, 9 Jul 2018 14:28:57 -0700
Subject: [PATCH] GS cookie check fix for debugger stackwalks port

This bug fix is a port from the equivalent fix in framework. The
debugger tried performing a stackwalk in the epilog due to the JIT
incorrectly reporting epilogue information. This caused an invalid
GS cookie to be checked and caused the debugger to crash. A flag was
added to allow debug stackwalks to skip the cookie check.
---
 src/debug/daccess/dacimpl.h | 2 +-
 src/debug/ee/debugger.cpp   | 2 +-
 src/debug/ee/frameinfo.cpp  | 4 +++-
 src/vm/stackwalk.cpp        | 6 +++---
 src/vm/threads.h            | 8 ++++++++
 5 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/debug/daccess/dacimpl.h b/src/debug/daccess/dacimpl.h
index 2647c8d300..6147de3dde 100644
--- a/src/debug/daccess/dacimpl.h
+++ b/src/debug/daccess/dacimpl.h
@@ -2005,7 +2005,7 @@ private:
         mCurr = &mHead;
         
         // Walk the stack, set mEnumerated to true to ensure we don't do it again.
-        unsigned int flagsStackWalk = ALLOW_INVALID_OBJECTS|ALLOW_ASYNC_STACK_WALK;
+        unsigned int flagsStackWalk = ALLOW_INVALID_OBJECTS|ALLOW_ASYNC_STACK_WALK|SKIP_GSCOOKIE_CHECK;
 #if defined(WIN64EXCEPTIONS)
         flagsStackWalk |= GC_FUNCLET_REFERENCE_REPORTING;
 #endif // defined(WIN64EXCEPTIONS)
diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp
index 2ca8762f98..a8d590e250 100644
--- a/src/debug/ee/debugger.cpp
+++ b/src/debug/ee/debugger.cpp
@@ -12588,7 +12588,7 @@ bool Debugger::IsThreadAtSafePlaceWorker(Thread *thread)
                                  Debugger::AtSafePlaceStackWalkCallback,
                                  (VOID*)(&atSafePlace),
                                  QUICKUNWIND | HANDLESKIPPEDFRAMES |
-                                 DISABLE_MISSING_FRAME_DETECTION);
+                                 DISABLE_MISSING_FRAME_DETECTION | SKIP_GSCOOKIE_CHECK);
 
 #ifdef LOGGING
     if (!atSafePlace)
diff --git a/src/debug/ee/frameinfo.cpp b/src/debug/ee/frameinfo.cpp
index 0387ca9217..f8f8932cd8 100644
--- a/src/debug/ee/frameinfo.cpp
+++ b/src/debug/ee/frameinfo.cpp
@@ -2141,7 +2141,9 @@ StackWalkAction DebuggerWalkStack(Thread *thread,
 
         result = g_pEEInterface->StackWalkFramesEx(thread, &data.regDisplay,
                                                    DebuggerWalkStackProc,
-                                                   &data, flags | HANDLESKIPPEDFRAMES | NOTIFY_ON_U2M_TRANSITIONS | ALLOW_ASYNC_STACK_WALK);
+                                                   &data,
+                                                   flags | HANDLESKIPPEDFRAMES | NOTIFY_ON_U2M_TRANSITIONS |
+                                                   ALLOW_ASYNC_STACK_WALK | SKIP_GSCOOKIE_CHECK);
     }
     else
     {
diff --git a/src/vm/stackwalk.cpp b/src/vm/stackwalk.cpp
index d324e411d7..345a67074d 100644
--- a/src/vm/stackwalk.cpp
+++ b/src/vm/stackwalk.cpp
@@ -1231,7 +1231,7 @@ BOOL StackFrameIterator::Init(Thread *    pThread,
     }
     INDEBUG(m_pRealStartFrame = m_crawl.pFrame);
 
-    if (m_crawl.pFrame != FRAME_TOP)
+    if (m_crawl.pFrame != FRAME_TOP && !(m_flags & SKIP_GSCOOKIE_CHECK))
     {
         m_crawl.SetCurGSCookie(Frame::SafeGetGSCookiePtr(m_crawl.pFrame));
     }
@@ -1324,7 +1324,7 @@ BOOL StackFrameIterator::ResetRegDisp(PREGDISPLAY pRegDisp,
         _ASSERTE(m_crawl.pFrame != NULL);
     }
 
-    if (m_crawl.pFrame != FRAME_TOP)
+    if (m_crawl.pFrame != FRAME_TOP && !(m_flags & SKIP_GSCOOKIE_CHECK))
     {
         m_crawl.SetCurGSCookie(Frame::SafeGetGSCookiePtr(m_crawl.pFrame));
     }
@@ -3151,7 +3151,7 @@ void StackFrameIterator::PreProcessingForManagedFrames(void)
                                                         &m_crawl.codeManState);
 #endif // !DACCESS_COMPILE
 
-    if (m_pCachedGSCookie)
+    if (!(m_flags & SKIP_GSCOOKIE_CHECK) && m_pCachedGSCookie)
     {
         m_crawl.SetCurGSCookie(m_pCachedGSCookie);
     }
diff --git a/src/vm/threads.h b/src/vm/threads.h
index ed16ac7587..e4b6487f55 100644
--- a/src/vm/threads.h
+++ b/src/vm/threads.h
@@ -3360,6 +3360,14 @@ public:
     // Refer to StackFrameIterator::Filter for detailed comments on this flag.
     #define GC_FUNCLET_REFERENCE_REPORTING 0x8000
 
+    // Stackwalking normally checks GS cookies on the fly, but there are cases in which the JIT reports
+    // incorrect epilog information. This causes the debugger to request stack walks in the epilog, checking
+    // an now invalid cookie. This flag allows the debugger stack walks to disable GS cookie checking.
+    
+    // This is a workaround for the debugger stackwalking. In general, the stackwalker and CrawlFrame
+    // may still execute GS cookie tracking/checking code paths.
+    #define SKIP_GSCOOKIE_CHECK 0x10000
+
     StackWalkAction StackWalkFramesEx(
                         PREGDISPLAY pRD,        // virtual register set at crawl start
                         PSTACKWALKFRAMESCALLBACK pCallback,
-- 
2.34.1