diff --git a/src/coreclr/vm/callsiteinspect.cpp b/src/coreclr/vm/callsiteinspect.cpp index dabbe89a49772..8209e41e6a7d4 100644 --- a/src/coreclr/vm/callsiteinspect.cpp +++ b/src/coreclr/vm/callsiteinspect.cpp @@ -433,7 +433,8 @@ void CallsiteInspect::PropagateOutParametersBackToCallsite( *(ARG_SLOT *)(frame->GetReturnValuePtr()) = retVal; } #ifdef ENREGISTERED_RETURNTYPE_MAXSIZE - else if (argit.HasNonStandardByvalReturn()) + else if (argit.HasNonStandardByvalReturn() + && !(flags & CallsiteDetails::HResultReturn)) { // In these cases, put the pointer to the return buffer into // the frame's return value slot. diff --git a/src/coreclr/vm/callsiteinspect.h b/src/coreclr/vm/callsiteinspect.h index 373b9347dfd9c..4ca66eca9feba 100644 --- a/src/coreclr/vm/callsiteinspect.h +++ b/src/coreclr/vm/callsiteinspect.h @@ -25,6 +25,7 @@ struct CallsiteDetails BeginInvoke = 0x01, EndInvoke = 0x02, Ctor = 0x04, + HResultReturn = 0x08, }; INT32 Flags; }; diff --git a/src/coreclr/vm/clrtocomcall.cpp b/src/coreclr/vm/clrtocomcall.cpp index 06d28f507249b..c604a6c8a9011 100644 --- a/src/coreclr/vm/clrtocomcall.cpp +++ b/src/coreclr/vm/clrtocomcall.cpp @@ -364,7 +364,7 @@ UINT32 CLRToCOMEventCallWorker(ComPlusMethodFrame* pFrame, ComPlusCallMethodDesc return 0; } -CallsiteDetails CreateCallsiteDetails(_In_ FramedMethodFrame *pFrame) +static CallsiteDetails CreateCallsiteDetails(_In_ FramedMethodFrame *pFrame) { CONTRACTL { @@ -442,10 +442,20 @@ CallsiteDetails CreateCallsiteDetails(_In_ FramedMethodFrame *pFrame) SigTypeContext::InitTypeContext(pMD, actualType, &typeContext); } + // If the signature is marked preserve sig, then the return + // is required to be an HRESULT, per COM rules. We set a flag to + // indicate this state to avoid issues when a C# developer defines + // an HRESULT in C# as a ValueClass with a single int field. This + // is convenient but does violate the COM ABI. Setting the flag + // lets us permit this convention and allow either a 4 byte primitive + // or the commonly used C# type "struct HResult { int Value; }". + if (IsMiPreserveSig(pMD->GetImplAttrs())) + callsiteFlags |= CallsiteDetails::HResultReturn; + _ASSERTE(!signature.IsEmpty() && pModule != nullptr); // Create details - return CallsiteDetails{ { signature, pModule, &typeContext }, pFrame, pMD, fIsDelegate }; + return CallsiteDetails{ { signature, pModule, &typeContext }, pFrame, pMD, fIsDelegate, callsiteFlags }; } UINT32 CLRToCOMLateBoundWorker( diff --git a/src/tests/Interop/COM/NETClients/IDispatch/Program.cs b/src/tests/Interop/COM/NETClients/IDispatch/Program.cs index 71839d9afffb8..fc9144575f0d9 100644 --- a/src/tests/Interop/COM/NETClients/IDispatch/Program.cs +++ b/src/tests/Interop/COM/NETClients/IDispatch/Program.cs @@ -139,6 +139,21 @@ static void Validate_Exception() Assert.Equal(GetErrorCodeFromHResult(e.HResult), errorCode); // Failing HRESULT exceptions contain CLR generated messages } + + // Calling methods through IDispatch::Invoke() (i.e., late-bound) doesn't + // propagate the HRESULT when marked with PreserveSig. It is always 0. + { + Console.WriteLine($"Calling {nameof(DispatchTesting.TriggerException)} (PreserveSig) with {nameof(IDispatchTesting_Exception.Int)} {errorCode}..."); + var dispatchTesting2 = (IDispatchTestingPreserveSig1)dispatchTesting; + Assert.Equal(0, dispatchTesting2.TriggerException(IDispatchTesting_Exception.Int, errorCode)); + } + + { + // Validate the HRESULT as a value type construct works for IDispatch. + Console.WriteLine($"Calling {nameof(DispatchTesting.TriggerException)} (PreserveSig, ValueType) with {nameof(IDispatchTesting_Exception.Int)} {errorCode}..."); + var dispatchTesting3 = (IDispatchTestingPreserveSig2)dispatchTesting; + Assert.Equal(0, dispatchTesting3.TriggerException(IDispatchTesting_Exception.Int, errorCode).Value); + } } static void Validate_StructNotSupported() diff --git a/src/tests/Interop/COM/NETServer/DispatchTesting.cs b/src/tests/Interop/COM/NETServer/DispatchTesting.cs index 477e5751f69e7..66461b8c7e47f 100644 --- a/src/tests/Interop/COM/NETServer/DispatchTesting.cs +++ b/src/tests/Interop/COM/NETServer/DispatchTesting.cs @@ -57,6 +57,7 @@ public void TriggerException(IDispatchTesting_Exception excep, int errorCode) case IDispatchTesting_Exception.Disp: throw new Exception(); case IDispatchTesting_Exception.HResult: + case IDispatchTesting_Exception.Int: throw new System.ComponentModel.Win32Exception(errorCode); } } diff --git a/src/tests/Interop/COM/NativeServer/DispatchTesting.h b/src/tests/Interop/COM/NativeServer/DispatchTesting.h index 927439fe03dc4..fbe7db6c1bad7 100644 --- a/src/tests/Interop/COM/NativeServer/DispatchTesting.h +++ b/src/tests/Interop/COM/NativeServer/DispatchTesting.h @@ -243,6 +243,8 @@ class DispatchTesting : public UnknownImpl, public IDispatchTesting return DISP_E_EXCEPTION; case IDispatchTesting_Exception_HResult: return HRESULT_FROM_WIN32(errorCode); + case IDispatchTesting_Exception_Int: + return errorCode; default: return S_FALSE; // Return a success case to indicate failure to trigger a failure. } diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs index 758c200acaaba..0bac21e66ee17 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs @@ -209,6 +209,7 @@ public enum IDispatchTesting_Exception { Disp, HResult, + Int, } [StructLayout(LayoutKind.Sequential)] @@ -220,6 +221,12 @@ public struct HFA_4 public float w; } + [StructLayout(LayoutKind.Sequential)] + public struct HRESULT + { + public int Value; + } + [ComVisible(true)] [Guid("a5e04c1c-474e-46d2-bbc0-769d04e12b54")] [InterfaceType(ComInterfaceType.InterfaceIsIDispatch)] @@ -257,6 +264,32 @@ void DoubleNumeric_ReturnByRef ( System.Collections.IEnumerator GetEnumerator(); } + [ComVisible(true)] + [Guid("a5e04c1c-474e-46d2-bbc0-769d04e12b54")] + [InterfaceType(ComInterfaceType.InterfaceIsIDispatch)] + public interface IDispatchTestingPreserveSig1 + { + void Reserved1(); + void Reserved2(); + void Reserved3(); + + [PreserveSig] + int TriggerException(IDispatchTesting_Exception excep, int errorCode); + } + + [ComVisible(true)] + [Guid("a5e04c1c-474e-46d2-bbc0-769d04e12b54")] + [InterfaceType(ComInterfaceType.InterfaceIsIDispatch)] + public interface IDispatchTestingPreserveSig2 + { + void Reserved1(); + void Reserved2(); + void Reserved3(); + + [PreserveSig] + HRESULT TriggerException(IDispatchTesting_Exception excep, int errorCode); + } + [ComVisible(true)] [Guid("83AFF8E4-C46A-45DB-9D91-2ADB5164545E")] [InterfaceType(ComInterfaceType.InterfaceIsIDispatch)] diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h index 3c9a1fcb06cbe..1eb0528aae4b7 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h @@ -385,6 +385,7 @@ enum IDispatchTesting_Exception { IDispatchTesting_Exception_Disp, IDispatchTesting_Exception_HResult, + IDispatchTesting_Exception_Int, }; struct __declspec(uuid("a5e04c1c-474e-46d2-bbc0-769d04e12b54"))