From 2afaf66e18e1d4abf90cc1bda0dfd2a126304f23 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 13 Jul 2025 22:30:58 -0700 Subject: [PATCH 1/4] 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. --- .../RuntimeBinder/ComInterop/ExcepInfo.cs | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs index b839dbb7d21db8..c74bbf1d8481aa 100644 --- a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs +++ b/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,22 @@ internal Exception GetException() wReserved = -1; // to ensure that the method gets called only once #endif + // If the scode is non-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); + 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. + if (exception is null) + { + // If we don't recognize the error code, create a generic COMException. + exception = 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 +82,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 +91,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; } From d4eb24873986f722cba05178f11f74b5a22839a7 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 13 Jul 2025 22:36:10 -0700 Subject: [PATCH 2/4] Fix comment. --- .../src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs index c74bbf1d8481aa..5653d659ec9dbd 100644 --- a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs +++ b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs @@ -55,7 +55,7 @@ internal Exception GetException() wReserved = -1; // to ensure that the method gets called only once #endif - // If the scode is non-zero, we use the wCode. The wCode is a legacy of + // 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; From 1417b66111c250dd22a830cac6ffcebbb5c1e4e1 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 14 Jul 2025 07:44:55 -0700 Subject: [PATCH 3/4] Style build break --- .../Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs index 5653d659ec9dbd..36579c733ee657 100644 --- a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs +++ b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ExcepInfo.cs @@ -59,15 +59,10 @@ internal Exception GetException() // 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); // If the error code doesn't resolve to an exception, we create a // generic COMException with the error code and no message. - if (exception is null) - { - // If we don't recognize the error code, create a generic COMException. - exception = new COMException(null, errorCode); - } + Exception exception = Marshal.GetExceptionForHR(errorCode) ?? new COMException(null, errorCode); string? message = ConvertAndFreeBstr(ref bstrDescription); if (message is not null) From 2cefd98b6017678b6974a9ca2100c7fc83772212 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 14 Jul 2025 09:58:58 -0700 Subject: [PATCH 4/4] Add test for wCode scenario. --- .../Interop/COM/NETClients/IDispatch/Program.cs | 17 +++++++++++++++-- .../Interop/COM/NETServer/DispatchTesting.cs | 1 + .../Interop/COM/NativeServer/DispatchTesting.h | 16 ++++++++++++++-- .../COM/ServerContracts/Server.Contracts.cs | 3 ++- .../COM/ServerContracts/Server.Contracts.h | 3 ++- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/tests/Interop/COM/NETClients/IDispatch/Program.cs b/src/tests/Interop/COM/NETClients/IDispatch/Program.cs index 90a6ea994ebb63..4324913a5a1fb6 100644 --- a/src/tests/Interop/COM/NETClients/IDispatch/Program.cs +++ b/src/tests/Interop/COM/NETClients/IDispatch/Program.cs @@ -108,7 +108,7 @@ 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; } @@ -116,7 +116,7 @@ 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}..."); diff --git a/src/tests/Interop/COM/NETServer/DispatchTesting.cs b/src/tests/Interop/COM/NETServer/DispatchTesting.cs index 66461b8c7e47f2..bf9b9d740e90ce 100644 --- a/src/tests/Interop/COM/NETServer/DispatchTesting.cs +++ b/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: diff --git a/src/tests/Interop/COM/NativeServer/DispatchTesting.h b/src/tests/Interop/COM/NativeServer/DispatchTesting.h index b4ee769ed8cb46..4c69174eec7e2e 100644 --- a/src/tests/Interop/COM/NativeServer/DispatchTesting.h +++ b/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(*args[0]), *args[1]); + IDispatchTesting_Exception kind = static_cast(*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]); diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs index ae68bcf34a07bf..a697d0359a2b62 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/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, } diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h index 9e8b256a73d3b3..b73c0c54b8b2f9 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h +++ b/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, };