From: Jan Vorlicek <janvorli@microsoft.com>
Date: Thu, 2 Apr 2020 20:45:07 +0000 (+0200)
Subject: Move the reverse pinvoke frame check to EECodeManager
X-Git-Tag: submit/tizen/20210909.063632~8718^2~3
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a2f2bb92948de32eefed2a1f689cbc646c00405c;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

Move the reverse pinvoke frame check to EECodeManager

Reflects PR feedback.
---

diff --git a/src/coreclr/src/inc/eetwain.h b/src/coreclr/src/inc/eetwain.h
index 6f61b51..e53f4a6 100644
--- a/src/coreclr/src/inc/eetwain.h
+++ b/src/coreclr/src/inc/eetwain.h
@@ -308,10 +308,12 @@ virtual bool IsInSynchronizedRegion(
 virtual size_t GetFunctionSize(GCInfoToken gcInfoToken) = 0;
 
 /*
-Returns the ReturnKind of a given function as reported in the GC info.
+*  Get information necessary for return address hijacking of the method represented by the gcInfoToken.
+*  If it can be hijacked, it sets the returnKind output parameter to the kind of the return value and
+*  returns true.
+*  If hijacking is not possible for some reason, it return false.
 */
-
-virtual ReturnKind GetReturnKind(GCInfoToken gcInfotoken) = 0;
+virtual bool GetReturnAddressHijackInfo(GCInfoToken gcInfoToken, ReturnKind * returnKind) = 0;
 
 #ifndef USE_GC_INFO_DECODER
 /*
@@ -585,9 +587,12 @@ virtual
 size_t GetFunctionSize(GCInfoToken gcInfoToken);
 
 /*
-Returns the ReturnKind of a given function.
+*  Get information necessary for return address hijacking of the method represented by the gcInfoToken.
+*  If it can be hijacked, it sets the returnKind output parameter to the kind of the return value and
+*  returns true.
+*  If hijacking is not possible for some reason, it return false.
 */
-virtual ReturnKind GetReturnKind(GCInfoToken gcInfotoken);
+virtual bool GetReturnAddressHijackInfo(GCInfoToken gcInfoToken, ReturnKind * returnKind);
 
 #ifndef USE_GC_INFO_DECODER
 /*
diff --git a/src/coreclr/src/vm/eetwain.cpp b/src/coreclr/src/vm/eetwain.cpp
index 42b701d..972b7ae 100644
--- a/src/coreclr/src/vm/eetwain.cpp
+++ b/src/coreclr/src/vm/eetwain.cpp
@@ -5731,9 +5731,12 @@ size_t EECodeManager::GetFunctionSize(GCInfoToken gcInfoToken)
 
 /*****************************************************************************
 *
-*  Returns the size of a given function.
+*  Get information necessary for return address hijacking of the method represented by the gcInfoToken.
+*  If it can be hijacked, it sets the returnKind output parameter to the kind of the return value and
+*  returns true.
+*  If hijacking is not possible for some reason, it return false.
 */
-ReturnKind EECodeManager::GetReturnKind(GCInfoToken gcInfoToken)
+bool EECodeManager::GetReturnAddressHijackInfo(GCInfoToken gcInfoToken, ReturnKind * returnKind)
 {
     CONTRACTL{
         NOTHROW;
@@ -5746,12 +5749,20 @@ ReturnKind EECodeManager::GetReturnKind(GCInfoToken gcInfoToken)
 
     DecodeGCHdrInfo(gcInfoToken, 0, &info);
 
-    return info.returnKind;
+    *returnKind = info.returnKind;
+    return true;
 #else // !USE_GC_INFO_DECODER
 
-    GcInfoDecoder gcInfoDecoder(gcInfoToken, DECODE_RETURN_KIND);
-    return gcInfoDecoder.GetReturnKind();
+    GcInfoDecoder gcInfoDecoder(gcInfoToken, GcInfoDecoderFlags(DECODE_RETURN_KIND | DECODE_REVERSE_PINVOKE_VAR));
+
+    if (gcInfoDecoder.GetReversePInvokeFrameStackSlot() != NO_REVERSE_PINVOKE_FRAME)
+    {
+        // Hijacking of NativeCallable method is not allowed
+        return false;
+    }
 
+    *returnKind = gcInfoDecoder.GetReturnKind();
+    return true;
 #endif // USE_GC_INFO_DECODER
 }
 
diff --git a/src/coreclr/src/vm/threadsuspend.cpp b/src/coreclr/src/vm/threadsuspend.cpp
index 2722f36..6c91b9a 100644
--- a/src/coreclr/src/vm/threadsuspend.cpp
+++ b/src/coreclr/src/vm/threadsuspend.cpp
@@ -5616,27 +5616,28 @@ void STDCALL OnHijackWorker(HijackArgs * pArgs)
 #endif // HIJACK_NONINTERRUPTIBLE_THREADS
 }
 
-ReturnKind GetReturnKind(Thread *pThread, EECodeInfo *codeInfo)
+bool GetReturnAddressHijackInfo(Thread *pThread, EECodeInfo *codeInfo, void** hijackAddress)
 {
+    ReturnKind returnKind;
     GCInfoToken gcInfoToken = codeInfo->GetGCInfoToken();
-    ReturnKind returnKind = codeInfo->GetCodeManager()->GetReturnKind(gcInfoToken);
-    _ASSERTE(IsValidReturnKind(returnKind));
-    return returnKind;
-}
+    if (!codeInfo->GetCodeManager()->GetReturnAddressHijackInfo(gcInfoToken, &returnKind))
+    {
+        return false;
+    }
 
-VOID * GetHijackAddr(Thread *pThread, EECodeInfo *codeInfo)
-{
-    ReturnKind returnKind = GetReturnKind(pThread, codeInfo);
+    _ASSERTE(IsValidReturnKind(returnKind));
     pThread->SetHijackReturnKind(returnKind);
 
 #ifdef TARGET_X86
     if (returnKind == RT_Float)
     {
-        return reinterpret_cast<VOID *>(OnHijackFPTripThread);
+        *hijackAddress = reinterpret_cast<VOID *>(OnHijackFPTripThread);
     }
 #endif // TARGET_X86
 
-    return reinterpret_cast<VOID *>(OnHijackTripThread);
+    *hijackAddress = reinterpret_cast<VOID *>(OnHijackTripThread);
+
+    return true;
 }
 
 #ifndef TARGET_UNIX
@@ -6092,11 +6093,10 @@ BOOL Thread::HandledJITCase(BOOL ForTaskSwitchIn)
             // it or not.
             EECodeInfo codeInfo(ip);
 
-            GcInfoDecoder gcInfoDecoder(codeInfo.GetGCInfoToken(), DECODE_REVERSE_PINVOKE_VAR);
+            VOID *pvHijackAddr;
 
-            if (gcInfoDecoder.GetReversePInvokeFrameStackSlot() == NO_REVERSE_PINVOKE_FRAME)
+            if (GetReturnAddressHijackInfo(this, &codeInfo, &pvHijackAddr))
             {
-                VOID *pvHijackAddr = GetHijackAddr(this, &codeInfo);
 
 #ifdef FEATURE_ENABLE_GCPOLL
                 // On platforms that support both hijacking and GC polling