Fix GS cookie check on ARM in functions with stackalloc (dotnet/coreclr#25628)

* Fix GS cookie check on ARM in functions with stackalloc

The GC cookie check was failing during GC stack walking on ARM for frames
of functions using stackalloc and pinvoke. The InlinedCallFrame stores
only the SP after the stackalloc adjustment and unwinder needs R9
that contains SP before the stackalloc to be able to unwind the frame
to get caller SP. The caller SP is used as a base for getting the GS
cookie address. We were incorrectly setting the R9 in the CONTEXT
to the same value as SP and so the unwinding was getting an incorrect
caller SP.

The fix is to store R9 in the InlinedCallFrame for ARM.

Commit migrated from https://github.com/dotnet/coreclr/commit/2290c1f10e58feef8636679788eb060705dae9b3

diff --git a/src/coreclr/src/inc/corinfo.h b/src/coreclr/src/inc/corinfo.h
index 083450b..9cc8490 100644 (file)
--- a/src/coreclr/src/inc/corinfo.h
+++ b/src/coreclr/src/inc/corinfo.h
@@ -1813,6 +1813,8 @@ struct CORINFO_EE_INFO
         unsigned    offsetOfCalleeSavedFP;
         unsigned    offsetOfCallTarget;
         unsigned    offsetOfReturnAddress;
+        // This offset is used only for ARM
+        unsigned    offsetOfSPAfterProlog;
     }
     inlinedCallFrameInfo;
 

diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp
index f8a5bab..809f48f 100644 (file)
--- a/src/coreclr/src/jit/lower.cpp
+++ b/src/coreclr/src/jit/lower.cpp
@@ -3336,9 +3336,11 @@ GenTree* Lowering::CreateFrameLinkUpdate(FrameLinkAction action)
 //                                                                              localloc and PInvoke in same function)
 //  +30h    +14h    m_pCallerReturnAddress            offsetOfReturnAddress   call site
 //  +38h    +18h    m_pCalleeSavedFP                  offsetOfCalleeSavedFP   not set by JIT
-//          +1Ch    JIT retval spill area (int)                               before call_gc    ???
-//          +20h    JIT retval spill area (long)                              before call_gc    ???
-//          +24h    Saved value of EBP                                        method prolog     ???
+//          +1Ch    m_pThread
+//          +20h    m_pSPAfterProlog                  offsetOfSPAfterProlog   arm only
+//          +20/24h JIT retval spill area (int)                               before call_gc    ???
+//          +24/28h JIT retval spill area (long)                              before call_gc    ???
+//          +28/2Ch Saved value of EBP                                        method prolog     ???
 //
 // Note that in the VM, InlinedCallFrame is a C++ class whose objects have a 'this' pointer that points
 // to the InlinedCallFrame vptr (the 2nd field listed above), and the GS cookie is stored *before*

diff --git a/src/coreclr/src/jit/target.h b/src/coreclr/src/jit/target.h
index 99d7d24..a7aa5f1 100644 (file)
--- a/src/coreclr/src/jit/target.h
+++ b/src/coreclr/src/jit/target.h
@@ -1024,6 +1024,7 @@ typedef unsigned char   regNumberSmall;
   #define RBM_OPT_RSVD             RBM_R10
 
   // We reserve R9 to store SP on entry for stack unwinding when localloc is used
+  // This needs to stay in sync with the ARM version of InlinedCallFrame::UpdateRegDisplay code.
   #define REG_SAVED_LOCALLOC_SP    REG_R9
   #define RBM_SAVED_LOCALLOC_SP    RBM_R9
 

diff --git a/src/coreclr/src/vm/arm/PInvokeStubs.asm b/src/coreclr/src/vm/arm/PInvokeStubs.asm
index 2e129a8..8b700de 100644 (file)
--- a/src/coreclr/src/vm/arm/PInvokeStubs.asm
+++ b/src/coreclr/src/vm/arm/PInvokeStubs.asm
@@ -139,6 +139,7 @@ __PInvokeGenStubFuncName SETS "$__PInvokeGenStubFuncName":CC:"_RetBuffArg"
             str     sp, [r0, #InlinedCallFrame__m_pCallSiteSP]
             str     r11, [r0, #InlinedCallFrame__m_pCalleeSavedFP]
             str     lr, [r0, #InlinedCallFrame__m_pCallerReturnAddress]
+            str     r9, [r0, #InlinedCallFrame__m_pSPAfterProlog]
 
             ;; r1 = GetThread(), TRASHES r2
             INLINE_GETTHREAD r1, r2

diff --git a/src/coreclr/src/vm/arm/asmconstants.h b/src/coreclr/src/vm/arm/asmconstants.h
index ac31e84..60199b5 100644 (file)
--- a/src/coreclr/src/vm/arm/asmconstants.h
+++ b/src/coreclr/src/vm/arm/asmconstants.h
@@ -239,5 +239,8 @@ ASMCONSTANTS_C_ASSERT(InlinedCallFrame__m_pCalleeSavedFP == offsetof(InlinedCall
 #define               InlinedCallFrame__m_pThread 0x18
 ASMCONSTANTS_C_ASSERT(InlinedCallFrame__m_pThread == offsetof(InlinedCallFrame, m_pThread))
 
+#define               InlinedCallFrame__m_pSPAfterProlog 0x1C
+ASMCONSTANTS_C_ASSERT(InlinedCallFrame__m_pSPAfterProlog == offsetof(InlinedCallFrame, m_pSPAfterProlog))
+
 #undef ASMCONSTANTS_RUNTIME_ASSERT
 #undef ASMCONSTANTS_C_ASSERT

diff --git a/src/coreclr/src/vm/arm/pinvokestubs.S b/src/coreclr/src/vm/arm/pinvokestubs.S
index 19e2190..cf4b6d4 100644 (file)
--- a/src/coreclr/src/vm/arm/pinvokestubs.S
+++ b/src/coreclr/src/vm/arm/pinvokestubs.S
@@ -103,6 +103,7 @@
         str     r1, [r4, #InlinedCallFrame__m_pCallSiteSP]
         str     r11, [r4, #InlinedCallFrame__m_pCalleeSavedFP]
         str     lr, [r4, #InlinedCallFrame__m_pCallerReturnAddress]
+        str     r9, [r4, #InlinedCallFrame__m_pSPAfterProlog]
 
         ;; r0 = GetThread()
         bl      C_FUNC(GetThread)

diff --git a/src/coreclr/src/vm/arm/stubs.cpp b/src/coreclr/src/vm/arm/stubs.cpp
index b876224..6da58b5 100644 (file)
--- a/src/coreclr/src/vm/arm/stubs.cpp
+++ b/src/coreclr/src/vm/arm/stubs.cpp
@@ -1326,6 +1326,7 @@ Stub *GenerateInitPInvokeFrameHelper()
     ThumbReg regFrame   = ThumbReg(4);
     ThumbReg regThread  = ThumbReg(5);
     ThumbReg regScratch = ThumbReg(6);
+    ThumbReg regR9 = ThumbReg(9);
 
 #ifdef FEATURE_PAL
     // Erect frame to perform call to GetThread
@@ -1356,9 +1357,12 @@ Stub *GenerateInitPInvokeFrameHelper()
     psl->ThumbEmitLoadRegIndirect(regScratch, regThread, offsetof(Thread, m_pFrame));
     psl->ThumbEmitStoreRegIndirect(regScratch, regFrame, FrameInfo.offsetOfFrameLink - negSpace);
 
-    // str FP, [regFrame + FrameInfo.offsetOfCalleeSavedEbp]
+    // str FP, [regFrame + FrameInfo.offsetOfCalleeSavedFP]
     psl->ThumbEmitStoreRegIndirect(thumbRegFp, regFrame, FrameInfo.offsetOfCalleeSavedFP - negSpace);
 
+    // str R9, [regFrame + FrameInfo.offsetOfSPAfterProlog]
+    psl->ThumbEmitStoreRegIndirect(regR9, regFrame, FrameInfo.offsetOfSPAfterProlog - negSpace);
+
     // mov [regFrame + FrameInfo.offsetOfReturnAddress], 0
     psl->ThumbEmitMovConstant(regScratch, 0);
     psl->ThumbEmitStoreRegIndirect(regScratch, regFrame, FrameInfo.offsetOfReturnAddress - negSpace);
@@ -2374,8 +2378,8 @@ void InlinedCallFrame::UpdateRegDisplay(const PREGDISPLAY pRD)
 
     // This is necessary to unwind methods with alloca. This needs to stay 
     // in sync with definition of REG_SAVED_LOCALLOC_SP in the JIT.
-    pRD->pCurrentContext->R9 = (DWORD) dac_cast<TADDR>(m_pCallSiteSP);
-    pRD->pCurrentContextPointers->R9 = (DWORD *)&m_pCallSiteSP;
+    pRD->pCurrentContext->R9 = (DWORD) dac_cast<TADDR>(m_pSPAfterProlog);
+    pRD->pCurrentContextPointers->R9 = (DWORD *)&m_pSPAfterProlog;
 
     RETURN;
 }

diff --git a/src/coreclr/src/vm/frames.h b/src/coreclr/src/vm/frames.h
index 45d7010..7f35dd0 100644 (file)
--- a/src/coreclr/src/vm/frames.h
+++ b/src/coreclr/src/vm/frames.h
@@ -3024,6 +3024,13 @@ public:
     // stubs, since there is no easy way to inline an implementation of GetThread.
     PTR_VOID             m_pThread;
 
+#ifdef _TARGET_ARM_
+    // Store the value of SP after prolog to ensure we can unwind functions that use
+    // stackalloc. In these functions, the m_pCallSiteSP can already be augmented by
+    // the stackalloc size, which is variable.
+    TADDR               m_pSPAfterProlog;
+#endif // _TARGET_ARM_
+
 public:
     //---------------------------------------------------------------
     // Expose key offsets and values for stub generation.

diff --git a/src/coreclr/src/vm/jitinterface.cpp b/src/coreclr/src/vm/jitinterface.cpp
index e667ddd..2bda507 100644 (file)
--- a/src/coreclr/src/vm/jitinterface.cpp
+++ b/src/coreclr/src/vm/jitinterface.cpp
@@ -10263,6 +10263,9 @@ void InlinedCallFrame::GetEEInfo(CORINFO_EE_INFO::InlinedCallFrameInfo *pInfo)
     pInfo->offsetOfCalleeSavedFP         = sizeof(GSCookie) + offsetof(InlinedCallFrame, m_pCalleeSavedFP);
     pInfo->offsetOfCallTarget            = sizeof(GSCookie) + offsetof(InlinedCallFrame, m_Datum);
     pInfo->offsetOfReturnAddress         = sizeof(GSCookie) + offsetof(InlinedCallFrame, m_pCallerReturnAddress);
+#ifdef _TARGET_ARM_
+    pInfo->offsetOfSPAfterProlog         = sizeof(GSCookie) + offsetof(InlinedCallFrame, m_pSPAfterProlog);
+#endif // _TARGET_ARM_
 }
 
 /*********************************************************************/