From a657823ef5cac9fb96f7f4138c894e919742389e Mon Sep 17 00:00:00 2001
From: Jan Kotas <jkotas@microsoft.com>
Date: Mon, 17 Jun 2019 17:15:56 -0700
Subject: [PATCH] Stop ignoring crashes around IUnknown::Release calls (#25210)

We do not want to be masking any memory corruption and bugs in the runtime.
---
 src/vm/comutilnative.cpp |  7 ----
 src/vm/interoputil.cpp   | 96 +-----------------------------------------------
 2 files changed, 2 insertions(+), 101 deletions(-)

diff --git a/src/vm/comutilnative.cpp b/src/vm/comutilnative.cpp
index 1a045f6..27d85f6 100644
--- a/src/vm/comutilnative.cpp
+++ b/src/vm/comutilnative.cpp
@@ -826,13 +826,6 @@ void QCALLTYPE Buffer::MemMove(void *dst, void *src, size_t length)
 {
     QCALL_CONTRACT;
 
-#if !defined(FEATURE_CORESYSTEM)
-    // Callers of memcpy do expect and handle access violations in some scenarios.
-    // Access violations in the runtime dll are turned into fail fast by the vector exception handler by default.
-    // We need to supress this behavior for CoreCLR using AVInRuntimeImplOkayHolder because of memcpy is statically linked in.
-    AVInRuntimeImplOkayHolder avOk;
-#endif
-
     memmove(dst, src, length);
 }
 
diff --git a/src/vm/interoputil.cpp b/src/vm/interoputil.cpp
index a6eab22..cd354cc 100644
--- a/src/vm/interoputil.cpp
+++ b/src/vm/interoputil.cpp
@@ -1404,23 +1404,6 @@ ErrExit:
     return hr;
 }
 
-void SafeRelease_OnException(IUnknown* pUnk, RCW* pRCW)
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-
-#ifndef CROSSGEN_COMPILE
-#ifdef FEATURE_COMINTEROP
-    LogInterop(W("An exception occurred during release"));
-    LogInteropLeak(pUnk);
-#endif // FEATURE_COMINTEROP
-#endif // CROSSGEN_COMPILE
-}
-
 #include <optsmallperfcritical.h>
 //--------------------------------------------------------------------------------
 // Release helper, must be called in preemptive mode.  Only use this variant if
@@ -1437,50 +1420,10 @@ ULONG SafeReleasePreemp(IUnknown * pUnk, RCW * pRCW)
     if (pUnk == NULL)
         return 0;
 
-    ULONG res = 0;
-    Thread * const pThread = GetThreadNULLOk();
-
     // Message pump could happen, so arbitrary managed code could run.
     CONTRACT_VIOLATION(ThrowsViolation | FaultViolation);
 
-    bool fException = false;
-    
-    SCAN_EHMARKER();
-    PAL_CPP_TRY
-    {
-        SCAN_EHMARKER_TRY();
-        // This is a holder to tell the contract system that we're catching all exceptions.
-        CLR_TRY_MARKER();
-
-        // Its very possible that the punk has gone bad before we could release it. This is a common application
-        // error. We may AV trying to call Release, and that AV will show up as an AV in mscorwks, so we'll take
-        // down the Runtime. Mark that an AV is alright, and handled, in this scope using this holder.
-        AVInRuntimeImplOkayHolder AVOkay(pThread);
-
-        res = pUnk->Release();
-
-        SCAN_EHMARKER_END_TRY();
-    }
-    PAL_CPP_CATCH_ALL
-    {
-        SCAN_EHMARKER_CATCH();
-#if defined(STACK_GUARDS_DEBUG)
-        // Catching and just swallowing an exception means we need to tell
-        // the SO code that it should go back to normal operation, as it
-        // currently thinks that the exception is still on the fly.
-        pThread->GetCurrentStackGuard()->RestoreCurrentGuard();
-#endif
-        fException = true;
-        SCAN_EHMARKER_END_CATCH();
-    }
-    PAL_CPP_ENDTRY;
-
-    if (fException)
-    {
-        SafeRelease_OnException(pUnk, pRCW);
-    }
-
-    return res;
+    return pUnk->Release();
 }
 
 //--------------------------------------------------------------------------------
@@ -1504,42 +1447,7 @@ ULONG SafeRelease(IUnknown* pUnk, RCW* pRCW)
     // Message pump could happen, so arbitrary managed code could run.
     CONTRACT_VIOLATION(ThrowsViolation | FaultViolation);
 
-    bool fException = false;
-    
-    SCAN_EHMARKER();
-    PAL_CPP_TRY
-    {
-        SCAN_EHMARKER_TRY();
-        // This is a holder to tell the contract system that we're catching all exceptions.
-        CLR_TRY_MARKER();
-
-        // Its very possible that the punk has gone bad before we could release it. This is a common application
-        // error. We may AV trying to call Release, and that AV will show up as an AV in mscorwks, so we'll take
-        // down the Runtime. Mark that an AV is alright, and handled, in this scope using this holder.
-        AVInRuntimeImplOkayHolder AVOkay(pThread);
-
-        res = pUnk->Release();
-
-        SCAN_EHMARKER_END_TRY();
-    }
-    PAL_CPP_CATCH_ALL
-    {
-        SCAN_EHMARKER_CATCH();
-#if defined(STACK_GUARDS_DEBUG)
-        // Catching and just swallowing an exception means we need to tell
-        // the SO code that it should go back to normal operation, as it
-        // currently thinks that the exception is still on the fly.
-        pThread->GetCurrentStackGuard()->RestoreCurrentGuard();
-#endif
-        fException = true;
-        SCAN_EHMARKER_END_CATCH();
-    }
-    PAL_CPP_ENDTRY;
-
-    if (fException)
-    {
-        SafeRelease_OnException(pUnk, pRCW);
-    }
+    res = pUnk->Release();
 
     GCX_PREEMP_NO_DTOR_END();
 
-- 
2.7.4