Respect ISupportErrorInfo for HRESULT to Exception throwing conversion (#117690)

Author: jkoritzinsky

src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs

@@ -1351,26 +1351,20 @@ internal static void DestroyCleanupList(ref CleanupWorkListElement? pCleanupWork
 
         internal static Exception GetHRExceptionObject(int hr)
         {
-            Exception? ex = null;
-            GetHRExceptionObject(hr, ObjectHandleOnStack.Create(ref ex));
-            ex!.InternalPreserveStackTrace();
-            return ex!;
+            Exception ex = Marshal.GetExceptionForHR(hr)!;
+            ex.InternalPreserveStackTrace();
+            return ex;
         }
 
-        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "StubHelpers_GetHRExceptionObject")]
-        private static partial void GetHRExceptionObject(int hr, ObjectHandleOnStack throwable);
-
 #if FEATURE_COMINTEROP
-        internal static Exception GetCOMHRExceptionObject(int hr, IntPtr pCPCMD, object pThis)
+        internal static Exception GetCOMHRExceptionObject(int hr, IntPtr pCPCMD, IntPtr pUnk)
         {
-            Exception? ex = null;
-            GetCOMHRExceptionObject(hr, pCPCMD, ObjectHandleOnStack.Create(ref pThis), ObjectHandleOnStack.Create(ref ex));
-            ex!.InternalPreserveStackTrace();
-            return ex!;
+            RuntimeMethodHandle handle = RuntimeMethodHandle.FromIntPtr(pCPCMD);
+            RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(handle.GetMethodInfo());
+            Exception ex = Marshal.GetExceptionForHR(hr, declaringType.GUID, pUnk)!;
+            ex.InternalPreserveStackTrace();
+            return ex;
         }
-
-        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "StubHelpers_GetCOMHRExceptionObject")]
-        private static partial void GetCOMHRExceptionObject(int hr, IntPtr pCPCMD, ObjectHandleOnStack pThis, ObjectHandleOnStack throwable);
 #endif // FEATURE_COMINTEROP
 
         [ThreadStatic]

src/coreclr/vm/corelib.h

@@ -1039,7 +1039,7 @@ DEFINE_METHOD(BUFFER,               MEMCOPYGC,              BulkMoveWithWriteBar
 DEFINE_CLASS(STUBHELPERS,           StubHelpers,            StubHelpers)
 DEFINE_METHOD(STUBHELPERS,          GET_DELEGATE_TARGET,    GetDelegateTarget,          SM_Delegate_RetIntPtr)
 #ifdef FEATURE_COMINTEROP
-DEFINE_METHOD(STUBHELPERS,          GET_COM_HR_EXCEPTION_OBJECT,              GetCOMHRExceptionObject,            SM_Int_IntPtr_Obj_RetException)
+DEFINE_METHOD(STUBHELPERS,          GET_COM_HR_EXCEPTION_OBJECT,              GetCOMHRExceptionObject,            SM_Int_IntPtr_IntPtr_RetException)
 DEFINE_METHOD(STUBHELPERS,          GET_COM_IP_FROM_RCW,                      GetCOMIPFromRCW,                    SM_Obj_IntPtr_RefIntPtr_RefBool_RetIntPtr)
 #endif // FEATURE_COMINTEROP
 DEFINE_METHOD(STUBHELPERS,          SET_LAST_ERROR,         SetLastError,               SM_RetVoid)

src/coreclr/vm/dllimport.cpp

@@ -736,14 +736,9 @@ class ILStubState : public StubState
 #ifdef FEATURE_COMINTEROP
         if (SF_IsForwardCOMStub(m_dwStubFlags))
         {
-            // Make sure that the RCW stays alive for the duration of the call. Note that if we do HRESULT
-            // swapping, we'll pass 'this' to GetCOMHRExceptionObject after returning from the target so
-            // GC.KeepAlive is not necessary.
-            if (!SF_IsHRESULTSwapping(m_dwStubFlags))
-            {
-                m_slIL.EmitLoadRCWThis(pcsDispatch, m_dwStubFlags);
-                pcsDispatch->EmitCALL(METHOD__GC__KEEP_ALIVE, 1, 0);
-            }
+            // Make sure that the RCW stays alive for the duration of the call.
+            m_slIL.EmitLoadRCWThis(pcsDispatch, m_dwStubFlags);
+            pcsDispatch->EmitCALL(METHOD__GC__KEEP_ALIVE, 1, 0);
         }
 #endif // FEATURE_COMINTEROP
 
@@ -761,7 +756,7 @@ class ILStubState : public StubState
                 if (SF_IsCOMStub(m_dwStubFlags))
                 {
                     m_slIL.EmitLoadStubContext(pcsDispatch, m_dwStubFlags);
-                    m_slIL.EmitLoadRCWThis(pcsDispatch, m_dwStubFlags);
+                    pcsDispatch->EmitLDLOC(m_slIL.GetTargetInterfacePointerLocalNum());
 
                     pcsDispatch->EmitCALL(METHOD__STUBHELPERS__GET_COM_HR_EXCEPTION_OBJECT, 3, 1);
                 }

src/coreclr/vm/metasig.h

@@ -169,7 +169,7 @@
 
 
 // static methods:
-DEFINE_METASIG_T(SM(Int_IntPtr_Obj_RetException, i I j, C(EXCEPTION)))
+DEFINE_METASIG_T(SM(Int_IntPtr_IntPtr_RetException, i I I, C(EXCEPTION)))
 DEFINE_METASIG_T(SM(Type_CharPtr_RuntimeAssembly_Bool_Bool_IntPtr_RetRuntimeType, P(u) C(ASSEMBLY) F F I, C(CLASS)))
 DEFINE_METASIG_T(SM(Type_RetIntPtr, C(TYPE), I))
 DEFINE_METASIG(SM(RefIntPtr_IntPtr_IntPtr_Int_RetObj, r(I) I I i, j))

src/coreclr/vm/qcallentrypoints.cpp

@@ -518,9 +518,7 @@ static const Entry s_QCall[] =
     DllImportEntry(StubHelpers_ProfilerBeginTransitionCallback)
     DllImportEntry(StubHelpers_ProfilerEndTransitionCallback)
 #endif
-    DllImportEntry(StubHelpers_GetHRExceptionObject)
 #if defined(FEATURE_COMINTEROP)
-    DllImportEntry(StubHelpers_GetCOMHRExceptionObject)
     DllImportEntry(StubHelpers_GetCOMIPFromRCWSlow)
     DllImportEntry(ObjectMarshaler_ConvertToNative)
     DllImportEntry(ObjectMarshaler_ConvertToManaged)

src/coreclr/vm/stubhelpers.cpp

@@ -543,81 +543,6 @@ extern "C" void QCALLTYPE StubHelpers_ProfilerEndTransitionCallback(MethodDesc*
 }
 #endif // PROFILING_SUPPORTED
 
-extern "C" void QCALLTYPE StubHelpers_GetHRExceptionObject(HRESULT hr, QCall::ObjectHandleOnStack result)
-{
-    QCALL_CONTRACT;
-
-    BEGIN_QCALL;
-
-    GCX_COOP();
-
-    OBJECTREF oThrowable = NULL;
-    GCPROTECT_BEGIN(oThrowable);
-
-    // GetExceptionForHR uses equivalant logic as COMPlusThrowHR
-    GetExceptionForHR(hr, &oThrowable);
-    result.Set(oThrowable);
-
-    GCPROTECT_END();
-
-    END_QCALL;
-}
-
-#ifdef FEATURE_COMINTEROP
-extern "C" void QCALLTYPE StubHelpers_GetCOMHRExceptionObject(
-    HRESULT hr,
-    MethodDesc* pMD,
-    QCall::ObjectHandleOnStack pThis,
-    QCall::ObjectHandleOnStack result)
-{
-    QCALL_CONTRACT;
-
-    BEGIN_QCALL;
-
-    GCX_COOP();
-
-    struct
-    {
-        OBJECTREF oThrowable;
-        OBJECTREF oref;
-    } gc;
-    gc.oThrowable = NULL;
-    gc.oref = NULL;
-    GCPROTECT_BEGIN(gc);
-
-    IErrorInfo* pErrorInfo = NULL;
-    if (pMD != NULL)
-    {
-        // Retrieve the interface method table.
-        MethodTable* pItfMT = CLRToCOMCallInfo::FromMethodDesc(pMD)->m_pInterfaceMT;
-
-        // get 'this'
-        gc.oref = ObjectToOBJECTREF(pThis.Get());
-
-        // Get IUnknown pointer for this interface on this object
-        IUnknown* pUnk = ComObject::GetComIPFromRCW(&gc.oref, pItfMT);
-        if (pUnk != NULL)
-        {
-            // Check to see if the component supports error information for this interface.
-            IID ItfIID;
-            pItfMT->GetGuid(&ItfIID, TRUE);
-            pErrorInfo = GetSupportedErrorInfo(pUnk, ItfIID);
-
-            DWORD cbRef = SafeRelease(pUnk);
-            LogInteropRelease(pUnk, cbRef, "IUnk to QI for ISupportsErrorInfo");
-        }
-    }
-
-    // GetExceptionForHR will handle lifetime of IErrorInfo.
-    GetExceptionForHR(hr, pErrorInfo, &gc.oThrowable);
-    result.Set(gc.oThrowable);
-
-    GCPROTECT_END();
-
-    END_QCALL;
-}
-#endif // FEATURE_COMINTEROP
-
 extern "C" void QCALLTYPE StubHelpers_MarshalToManagedVaList(va_list va, VARARGS* pArgIterator)
 {
     QCALL_CONTRACT;

src/coreclr/vm/stubhelpers.h

@@ -46,11 +46,7 @@ extern "C" void* QCALLTYPE StubHelpers_ProfilerBeginTransitionCallback(MethodDes
 extern "C" void QCALLTYPE StubHelpers_ProfilerEndTransitionCallback(MethodDesc* pTargetMD);
 #endif
 
-extern "C" void QCALLTYPE StubHelpers_GetHRExceptionObject(HRESULT hr, QCall::ObjectHandleOnStack result);
-
 #ifdef FEATURE_COMINTEROP
-extern "C" void QCALLTYPE StubHelpers_GetCOMHRExceptionObject(HRESULT hr, MethodDesc *pMD, QCall::ObjectHandleOnStack pThis, QCall::ObjectHandleOnStack result);
-
 extern "C" IUnknown* QCALLTYPE StubHelpers_GetCOMIPFromRCWSlow(QCall::ObjectHandleOnStack pSrc, MethodDesc* pMD, void** ppTarget);
 
 extern "C" void QCALLTYPE ObjectMarshaler_ConvertToNative(QCall::ObjectHandleOnStack pSrcUNSAFE, VARIANT* pDest);

src/libraries/Common/src/Interop/Windows/OleAut32/Interop.GetErrorInfo.cs

@@ -0,0 +1,14 @@
+// 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.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static partial class OleAut32
+    {
+        [LibraryImport(Libraries.OleAut32)]
+        internal static partial int GetErrorInfo(uint dwReserved, out IntPtr ppErrorInfo);
+    }
+}

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

@@ -2157,6 +2157,9 @@
     <Compile Include="$(CommonPath)Interop\Windows\Ole32\Interop.PropVariantClear.cs">
       <Link>Common\Interop\Windows\Ole32\Interop.PropVariantClear.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)Interop\Windows\OleAut32\Interop.GetErrorInfo.cs">
+      <Link>Common\Interop\Windows\OleAut32\Interop.GetErrorInfo.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)Interop\Windows\Secur32\Interop.GetUserNameExW.cs">
       <Link>Common\Interop\Windows\Secur32\Interop.GetUserNameExW.cs</Link>
     </Compile>

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs

@@ -653,6 +653,63 @@ public static IntPtr GetHINSTANCE(Module m)
             return GetExceptionForHRInternal(errorCode, errorInfo);
         }
 
+        public static Exception? GetExceptionForHR(int errorCode, in Guid iid, IntPtr pUnk)
+        {
+            if (errorCode >= 0)
+            {
+                return null;
+            }
+
+            return GetExceptionForHRInternal(errorCode, in iid, pUnk);
+        }
+
+        private static unsafe Exception? GetExceptionForHRInternal(int errorCode, in Guid iid, IntPtr pUnk)
+        {
+            const IntPtr NoErrorInfo = -1; // Use -1 to indicate no error info available
+
+            // Normally, we would check if the interface supports IErrorInfo first. However,
+            // built-in COM calls GetErrorInfo first to clear the error info, so we follow
+            // that pattern here.
+            IntPtr errorInfo = NoErrorInfo;
+
+#if TARGET_WINDOWS
+            Interop.OleAut32.GetErrorInfo(0, out errorInfo);
+            if (errorInfo == IntPtr.Zero)
+            {
+                errorInfo = NoErrorInfo;
+            }
+
+            // If there is error info and we have a pointer to the interface,
+            // we check if it supports ISupportErrorInfo.
+            if (errorInfo != NoErrorInfo && pUnk != IntPtr.Zero)
+            {
+                Guid IID_ISupportErrorInfo = new(0xDF0B3D60, 0x548F, 0x101B, 0x8E, 0x65, 0x08, 0x00, 0x2B, 0x2B, 0xD1, 0x19);
+                int hr = QueryInterface(pUnk, in IID_ISupportErrorInfo, out IntPtr supportErrorInfo);
+                if (hr == 0)
+                {
+                    // Check if the target interface is supported.
+                    // ISupportErrorInfo.InterfaceSupportsErrorInfo slot
+                    fixed (Guid* piid = &iid)
+                    {
+                        hr = ((delegate* unmanaged[MemberFunction]<IntPtr, Guid*, int>)(*(*(void***)supportErrorInfo + 3)))(supportErrorInfo, piid);
+                    }
+                    Release(supportErrorInfo);
+                }
+
+                // If ISupportErrorInfo isn't supported or the target interface doesn't support IErrorInfo,
+                // release the error info and mark it as NoErrorInfo to avoid querying for IErrorInfo again.
+                if (hr != 0)
+                {
+                    Release(errorInfo);
+                    errorInfo = NoErrorInfo;
+                }
+            }
+#endif
+
+            // If the error info is valid, its lifetime will be handled by GetExceptionForHRInternal().
+            return GetExceptionForHRInternal(errorCode, errorInfo);
+        }
+
 #if !CORECLR
 #pragma warning disable IDE0060
         private static Exception? GetExceptionForHRInternal(int errorCode, IntPtr errorInfo)
@@ -865,6 +922,14 @@ public static void ThrowExceptionForHR(int errorCode, IntPtr errorInfo)
             }
         }
 
+        public static void ThrowExceptionForHR(int errorCode, in Guid iid, IntPtr pUnk)
+        {
+            if (errorCode < 0)
+            {
+                throw GetExceptionForHR(errorCode, in iid, pUnk)!;
+            }
+        }
+
         public static IntPtr SecureStringToBSTR(SecureString s)
         {
             if (s is null)