Fix TryInvokeICustomQueryInterface exception handling (#117518)

* Fix TryInvokeICustomQueryInterface exception handling

When the InteropLibImports::TryInvokeICustomQueryInterface is called
from a native thread and a managed exception occurs in the managed
ICustomQueryInterface interface implementation, the exception handling
reports that exception as unhandled because it doesn't know tha the
InteropLibImports::TryInvokeICustomQueryInterface catches managed
exceptions and converts them to HRESULTs.
This change fixes it by adding DebuggerU2MCatchHandlerFrame to the
InteropLibImports::TryInvokeICustomQueryInterface. That allows EH to
understand that the exception that flows to native code will be caught
and so it should not report it as unhandled.

Close #117393

* Add regression test

* Fix unix test build

* Fix unix test build again

Author: janvorli

src/coreclr/vm/interoplibinterface_comwrappers.cpp

@@ -355,14 +355,18 @@ namespace InteropLibImports
                 return TryInvokeICustomQueryInterfaceResult::FailedToInvoke;
         }
 
+        // Switch to Cooperative mode since object references
+        // are being manipulated and the catchFrame needs that so that it can push
+        // itself to the explicit frame stack.
+        GCX_COOP();
+        // Indicate to the debugger and exception handling that managed exceptions are being caught
+        // here.
+        DebuggerU2MCatchHandlerFrame catchFrame(true /* catchesAllExceptions */);
+
         HRESULT hr;
         auto result = TryInvokeICustomQueryInterfaceResult::FailedToInvoke;
         EX_TRY_THREAD(CURRENT_THREAD)
         {
-            // Switch to Cooperative mode since object references
-            // are being manipulated.
-            GCX_COOP();
-
             struct
             {
                 OBJECTREF objRef;
@@ -380,6 +384,8 @@ namespace InteropLibImports
         }
         EX_CATCH_HRESULT(hr);
 
+        catchFrame.Pop();
+
         // Assert valid value.
         _ASSERTE(TryInvokeICustomQueryInterfaceResult::Min <= result
             && result <= TryInvokeICustomQueryInterfaceResult::Max);

src/tests/Regressions/coreclr/GitHub_117393/CMakeLists.txt

@@ -0,0 +1,10 @@
+project (nativetest117393)
+include_directories( ${INC_PLATFORM_DIR} )
+set(SOURCES nativetest117393.cpp)
+
+# add the shared library
+add_library (nativetest117393 SHARED ${SOURCES})
+target_link_libraries(nativetest117393 PRIVATE ${LINK_LIBRARIES_ADDITIONAL})
+
+# add the install targets
+install (TARGETS nativetest117393 DESTINATION bin)

src/tests/Regressions/coreclr/GitHub_117393/nativetest117393.cpp

@@ -0,0 +1,73 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#include <../../Interop/common/xplatform.h>
+#include <../../Interop/common/ComHelpers.h>
+#include "platformdefines.h"
+
+#ifdef _WIN32
+#pragma warning(push)
+#pragma warning(disable:4265 4577)
+#include <thread>
+#pragma warning(pop)
+#else // _WIN32
+#include <pthread.h>
+#endif // _WIN32
+
+// #include <platformdefines.h>
+
+// Interface ID for ITest
+// {92BAA992-DB5A-4ADD-977B-B22838EE91FD}
+static const GUID IID_ITest =
+{ 0x92baa992, 0xdb5a, 0x4add, { 0x97, 0x7b, 0xb2, 0x28, 0x38, 0xee, 0x91, 0xfd } };
+
+// Interface definition for ITest
+MIDL_INTERFACE("92BAA992-DB5A-4ADD-977B-B22838EE91FD")
+ITest : public IUnknown
+{
+    virtual HRESULT STDMETHODCALLTYPE Test() = 0;
+};
+
+void NativeTestThread(IUnknown* pUnknown)
+{
+    ITest* pITest = nullptr;
+    pUnknown->QueryInterface(IID_ITest, reinterpret_cast<void**>(&pITest));
+    // This tests causes the QueryInterface to fail, so we don't release the pITest
+}
+
+#ifndef _WIN32
+void* NativeTestThreadUnix(void* pUnknown)
+{
+    NativeTestThread((IUnknown*)pUnknown);
+    return NULL;
+}
+
+#define AbortIfFail(st) if (st != 0) abort()
+
+#endif // !_WIN32
+
+extern "C" DLL_EXPORT void TestFromNativeThread(IUnknown* pUnknown)
+{
+#ifdef _WIN32
+    std::thread t1(NativeTestThread, pUnknown);
+    t1.join();
+#else // _WIN32
+    // For Unix, we need to use pthreads to create the thread so that we can set its stack size.
+    // We need to set the stack size due to the very small (80kB) default stack size on MUSL
+    // based Linux distros.
+    pthread_attr_t attr;
+    int st = pthread_attr_init(&attr);
+    AbortIfFail(st);
+
+    // set stack size to 1.5MB
+    st = pthread_attr_setstacksize(&attr, 0x180000);
+    AbortIfFail(st);
+
+    pthread_t t;
+    st = pthread_create(&t, &attr, NativeTestThreadUnix, (void*)pUnknown);
+    AbortIfFail(st);
+
+    st = pthread_join(t, NULL);
+    AbortIfFail(st);
+#endif // _WIN32
+}

src/tests/Regressions/coreclr/GitHub_117393/test117393.cs

@@ -0,0 +1,70 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+using Xunit;
+
+namespace Test117393
+{
+    class TestObject : ICustomQueryInterface
+    {
+        public CustomQueryInterfaceResult GetInterface(ref Guid iid, out IntPtr ppv)
+        {
+            // Induce NullReferenceException
+            string s = null;
+            Console.WriteLine(s.Length);
+            ppv = IntPtr.Zero;
+	        return CustomQueryInterfaceResult.Failed;
+        }
+    }
+
+    class TestWrappers : ComWrappers
+    {
+        protected override unsafe ComInterfaceEntry* ComputeVtables(object obj, CreateComInterfaceFlags flags, out int count)
+        {
+            // Unknown type
+            count = 0;
+            return null;
+        }
+
+        protected override object? CreateObject(IntPtr externalComObject, CreateObjectFlags flags)
+        {
+            return null;
+        }
+
+        protected override void ReleaseObjects(IEnumerable objects)
+        {
+        }
+    }
+
+    public class Program
+    {
+        [DllImport("nativetest117393")]
+        private static extern void TestFromNativeThread(IntPtr pUnknown);
+
+        [Fact]
+        public static void TestEntryPoint()
+        {
+            bool reportedUnhandledException = false;
+            UnhandledExceptionEventHandler handler = (sender, e) =>
+            {
+                reportedUnhandledException = true;
+            };
+
+            var cw = new TestWrappers();
+            TestObject obj = new TestObject();
+            // Create a managed object wrapper for the Demo object.
+            // Note the returned COM interface will always be for IUnknown.
+            IntPtr ccwUnknown = cw.GetOrCreateComInterfaceForObject(obj, CreateComInterfaceFlags.None);
+            AppDomain.CurrentDomain.UnhandledException += handler;
+            TestFromNativeThread(ccwUnknown);
+            AppDomain.CurrentDomain.UnhandledException -= handler;
+            Marshal.Release(ccwUnknown);
+
+            Assert.False(reportedUnhandledException, "There should be no unhandled exception on the secondary thread");
+        }
+    }
+}

src/tests/Regressions/coreclr/GitHub_117393/test117393.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <!-- Needed for CMakeProjectReference -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="test117393.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <CMakeProjectReference Include="CMakeLists.txt" />
+  </ItemGroup>
+</Project>