Address COM interop case involving legacy wCode usage. (#117596)

* Address COM interop case involving legacy wCode usage.

The wCode field in the EXCEPINFO is a 16-bit signed integer. This
is a legacy field for 16-bit Windows, but is still used in many COM
servers. The current implementation assumed it would result in an
Exception using Marshal.GetExceptionForHR(), but converting a
short to an int, will rarely result in a negative value which means
null will almost always be returned. This change checks if the
exception is null and if so, creates a simple COMException with the
error code.

Author: AaronRobinsonMSFT

src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs

@@ -33,7 +33,7 @@ static ExcepInfo()
         }
 #endif
 
-        private static string ConvertAndFreeBstr(ref IntPtr bstr)
+        private static string? ConvertAndFreeBstr(ref IntPtr bstr)
         {
             if (bstr == IntPtr.Zero)
             {
@@ -55,11 +55,17 @@ internal Exception GetException()
             wReserved = -1; // to ensure that the method gets called only once
 #endif
 
+            // If the scode is zero, we use the wCode. The wCode is a legacy of
+            // 16-bit Windows error codes. This means it will never be an error
+            // scode (HRESULT, < 0) and we will get a null exception.
             int errorCode = (scode != 0) ? scode : wCode;
-            Exception exception = Marshal.GetExceptionForHR(errorCode);
 
-            string message = ConvertAndFreeBstr(ref bstrDescription);
-            if (message != null)
+            // If the error code doesn't resolve to an exception, we create a
+            // generic COMException with the error code and no message.
+            Exception exception = Marshal.GetExceptionForHR(errorCode) ?? new COMException(null, errorCode);
+
+            string? message = ConvertAndFreeBstr(ref bstrDescription);
+            if (message is not null)
             {
                 // If we have a custom message, create a new Exception object with the message set correctly.
                 // We need to create a new object because "exception.Message" is a read-only property.
@@ -71,7 +77,7 @@ internal Exception GetException()
                 {
                     Type exceptionType = exception.GetType();
                     ConstructorInfo ctor = exceptionType.GetConstructor(new Type[] { typeof(string) });
-                    if (ctor != null)
+                    if (ctor is not null)
                     {
                         exception = (Exception)ctor.Invoke(new object[] { message });
                     }
@@ -80,8 +86,8 @@ internal Exception GetException()
 
             exception.Source = ConvertAndFreeBstr(ref bstrSource);
 
-            string helpLink = ConvertAndFreeBstr(ref bstrHelpFile);
-            if (helpLink != null && dwHelpContext != 0)
+            string? helpLink = ConvertAndFreeBstr(ref bstrHelpFile);
+            if (helpLink is not null && dwHelpContext != 0)
             {
                 helpLink += "#" + dwHelpContext;
             }

src/tests/Interop/COM/NETClients/IDispatch/Program.cs

@@ -108,15 +108,15 @@ static void Validate_Double_In_ReturnAndUpdateByRef()
 
         static int GetErrorCodeFromHResult(int hresult)
         {
-            // https://msdn.microsoft.com/en-us/library/cc231198.aspx
+            // https://msdn.microsoft.com/library/cc231198.aspx
             return hresult & 0xffff;
         }
 
         static void Validate_Exception()
         {
             var dispatchTesting = (DispatchTesting)new DispatchTestingClass();
 
-            int errorCode = 127;
+            int errorCode = 1127;
             string resultString = errorCode.ToString("x");
             try
             {
@@ -130,6 +130,19 @@ static void Validate_Exception()
                 Assert.Equal(e.Message, resultString);
             }
 
+            try
+            {
+                Console.WriteLine($"Calling {nameof(DispatchTesting.TriggerException)} with {nameof(IDispatchTesting_Exception.DispLegacy)} {errorCode}...");
+                dispatchTesting.TriggerException(IDispatchTesting_Exception.DispLegacy, errorCode);
+                Assert.Fail("DISP exception not thrown properly");
+            }
+            catch (COMException e)
+            {
+                Assert.Equal(e.ErrorCode, errorCode); // The legacy DISP exception returns the error code unmodified.
+                Assert.Equal(e.HResult, errorCode);
+                Assert.Equal(e.Message, resultString);
+            }
+
             try
             {
                 Console.WriteLine($"Calling {nameof(DispatchTesting.TriggerException)} with {nameof(IDispatchTesting_Exception.HResult)} {errorCode}...");

src/tests/Interop/COM/NETServer/DispatchTesting.cs

@@ -55,6 +55,7 @@ public void TriggerException(IDispatchTesting_Exception excep, int errorCode)
         switch (excep)
         {
         case IDispatchTesting_Exception.Disp:
+        case IDispatchTesting_Exception.DispLegacy:
             throw new Exception();
         case IDispatchTesting_Exception.HResult:
         case IDispatchTesting_Exception.Int:

src/tests/Interop/COM/NativeServer/DispatchTesting.h

@@ -240,6 +240,7 @@ class DispatchTesting : public UnknownImpl, public IDispatchTesting
         switch (excep)
         {
         case IDispatchTesting_Exception_Disp:
+        case IDispatchTesting_Exception_Disp_Legacy:
             return DISP_E_EXCEPTION;
         case IDispatchTesting_Exception_HResult:
             return HRESULT_FROM_WIN32(errorCode);
@@ -457,11 +458,22 @@ class DispatchTesting : public UnknownImpl, public IDispatchTesting
             args[1] = &currArg->intVal;
         }
 
-        hr = TriggerException(static_cast<IDispatchTesting_Exception>(*args[0]), *args[1]);
+        IDispatchTesting_Exception kind = static_cast<IDispatchTesting_Exception>(*args[0]);
+        hr = TriggerException(kind, *args[1]);
         if (hr == DISP_E_EXCEPTION)
         {
             *puArgErr = 1;
-            pExcepInfo->scode = HRESULT_FROM_WIN32(*args[1]);
+            if (kind == IDispatchTesting_Exception_Disp_Legacy)
+            {
+                pExcepInfo->wCode = *args[1]; // Legacy exception code
+                pExcepInfo->scode = 0;
+            }
+            else
+            {
+                assert(kind == IDispatchTesting_Exception_Disp);
+                pExcepInfo->wCode = 0;
+                pExcepInfo->scode = HRESULT_FROM_WIN32(*args[1]);
+            }
 
             WCHAR buffer[ARRAY_SIZE(W("4294967295"))];
             _snwprintf_s(buffer, ARRAY_SIZE(buffer), _TRUNCATE, W("%x"), *args[1]);

src/tests/Interop/COM/ServerContracts/Server.Contracts.cs

@@ -237,7 +237,8 @@ public interface IErrorMarshalTesting
 
     public enum IDispatchTesting_Exception
     {
-        Disp,
+        Disp,       // scode
+        DispLegacy, // wCode
         HResult,
         Int,
     }

src/tests/Interop/COM/ServerContracts/Server.Contracts.h

@@ -413,7 +413,8 @@ IErrorMarshalTesting : IUnknown
 
 enum IDispatchTesting_Exception
 {
-    IDispatchTesting_Exception_Disp,
+    IDispatchTesting_Exception_Disp,        // scode
+    IDispatchTesting_Exception_Disp_Legacy, // wCode
     IDispatchTesting_Exception_HResult,
     IDispatchTesting_Exception_Int,
 };