From b7c564db36b5d23b741c49f752752c7bbc98ebd4 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 4 Jun 2022 15:42:44 +0800 Subject: [PATCH 01/14] Move help link parsing to managed --- .../src/System/Exception.CoreCLR.cs | 27 +++++++ src/coreclr/vm/comutilnative.cpp | 78 ++----------------- src/coreclr/vm/corelib.h | 1 + src/coreclr/vm/metasig.h | 3 + 4 files changed, 39 insertions(+), 70 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs index 2189ef31c7f89..0ea420f103d6f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs @@ -274,5 +274,32 @@ private bool CanSetRemoteStackTrace() return true; } + + // used by vm + internal string? GetHelpContext(out int helpContext) + { + helpContext = 0; + string? helpFile = HelpLink; + + int poundPos, digitEnd; + + if (helpFile is null || (poundPos = helpFile.LastIndexOf('#')) == -1) + { + return helpFile; + } + + for (digitEnd = poundPos + 1; digitEnd < helpFile.Length; digitEnd++) + { + if (char.IsWhiteSpace(helpFile[digitEnd])) + break; + } + + if (int.TryParse(helpFile.AsSpan(poundPos + 1, digitEnd - poundPos - 1), out helpContext)) + { + helpFile = helpFile.Substring(0, poundPos); + } + + return helpFile; + } } } diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index fc486eb17268d..1380898d72d11 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -369,79 +369,17 @@ static void GetExceptionHelp(OBJECTREF objException, BSTR *pbstrHelpFile, DWORD GCPROTECT_BEGIN(objException); - // read Exception.HelpLink property - MethodDescCallSite getHelpLink(METHOD__EXCEPTION__GET_HELP_LINK, &objException); + // call managed code to parse help context + MethodDescCallSite getHelpContext(METHOD__EXCEPTION__GET_HELP_CONTEXT, &objException); - ARG_SLOT GetHelpLinkArgs[] = { ObjToArgSlot(objException)}; - *pbstrHelpFile = BStrFromString(getHelpLink.Call_RetSTRINGREF(GetHelpLinkArgs)); + ARG_SLOT GetHelpContextArgs[] = + { + ObjToArgSlot(objException), + PtrToArgSlot(pdwHelpContext) + }; + *pbstrHelpFile = BStrFromString(getHelpContext.Call_RetSTRINGREF(GetHelpContextArgs)); GCPROTECT_END(); - - // parse the help file to check for the presence of helpcontext - int len = SysStringLen(*pbstrHelpFile); - int pos = len; - WCHAR *pwstr = *pbstrHelpFile; - if (pwstr) { - BOOL fFoundPound = FALSE; - - for (pos = len - 1; pos >= 0; pos--) { - if (pwstr[pos] == W('#')) { - fFoundPound = TRUE; - break; - } - } - - if (fFoundPound) { - int PoundPos = pos; - int NumberStartPos = -1; - BOOL bNumberStarted = FALSE; - BOOL bNumberFinished = FALSE; - BOOL bInvalidDigitsFound = FALSE; - - _ASSERTE(pwstr[pos] == W('#')); - - // Check to see if the string to the right of the pound a valid number. - for (pos++; pos < len; pos++) { - if (bNumberFinished) { - if (!COMCharacter::nativeIsWhiteSpace(pwstr[pos])) { - bInvalidDigitsFound = TRUE; - break; - } - } - else if (bNumberStarted) { - if (COMCharacter::nativeIsWhiteSpace(pwstr[pos])) { - bNumberFinished = TRUE; - } - else if (!COMCharacter::nativeIsDigit(pwstr[pos])) { - bInvalidDigitsFound = TRUE; - break; - } - } - else { - if (COMCharacter::nativeIsDigit(pwstr[pos])) { - NumberStartPos = pos; - bNumberStarted = TRUE; - } - else if (!COMCharacter::nativeIsWhiteSpace(pwstr[pos])) { - bInvalidDigitsFound = TRUE; - break; - } - } - } - - if (bNumberStarted && !bInvalidDigitsFound) { - // Grab the help context and remove it from the help file. - *pdwHelpContext = (DWORD)wtoi(&pwstr[NumberStartPos], len - NumberStartPos); - - // Allocate a new help file string of the right length. - BSTR strOld = *pbstrHelpFile; - *pbstrHelpFile = SysAllocStringLen(strOld, PoundPos); - SysFreeString(strOld); - if (!*pbstrHelpFile) - COMPlusThrowOM(); - } - } - } } // NOTE: caller cleans up any partially initialized BSTRs in pED diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index eeeb76769f930..cd4da4f84bf1c 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -335,6 +335,7 @@ DEFINE_PROPERTY(EXCEPTION, MESSAGE, Message, DEFINE_PROPERTY(EXCEPTION, SOURCE, Source, Str) DEFINE_PROPERTY(EXCEPTION, HELP_LINK, HelpLink, Str) DEFINE_METHOD(EXCEPTION, INTERNAL_PRESERVE_STACK_TRACE, InternalPreserveStackTrace, IM_RetVoid) +DEFINE_METHOD(EXCEPTION, GET_HELP_CONTEXT, GetHelpContext, IM_RefInt_RetStr) DEFINE_CLASS(SYSTEM_EXCEPTION, System, SystemException) diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 56570b28ee9d7..78032dbbaa336 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -543,6 +543,9 @@ DEFINE_METASIG(SM(Str_RetArrStr, s, a(s))) // Execution Context DEFINE_METASIG_T(SM(SyncCtx_ArrIntPtr_Bool_Int_RetInt, C(SYNCHRONIZATION_CONTEXT) a(I) F i, i)) +// Exception +DEFINE_METASIG(IM(RefInt_RetStr), r(i), s) + #ifdef FEATURE_COMINTEROP // The signature of the method System.Runtime.InteropServices.ICustomQueryInterface.GetInterface DEFINE_METASIG_T(IM(RefGuid_OutIntPtr_RetCustomQueryInterfaceResult, r(g(GUID)) r(I), g(CUSTOMQUERYINTERFACERESULT))) From 359faf39587ec468414a5b63a86876255c546c80 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 4 Jun 2022 15:45:15 +0800 Subject: [PATCH 02/14] Cleanup wtoi and nativeIsDigit --- src/coreclr/vm/comutilnative.cpp | 63 -------------------------------- src/coreclr/vm/util.cpp | 17 --------- src/coreclr/vm/util.hpp | 1 - 3 files changed, 81 deletions(-) diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 1380898d72d11..c8f4e9c77dc78 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -41,69 +41,6 @@ #include "arraynative.inl" -/*===================================IsDigit==================================== -**Returns a bool indicating whether the character passed in represents a ** -**digit. -==============================================================================*/ -bool IsDigit(WCHAR c, int radix, int *result) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - PRECONDITION(CheckPointer(result)); - } - CONTRACTL_END; - - if (IS_DIGIT(c)) { - *result = DIGIT_TO_INT(c); - } - else if (c>='A' && c<='Z') { - //+10 is necessary because A is actually 10, etc. - *result = c-'A'+10; - } - else if (c>='a' && c<='z') { - //+10 is necessary because a is actually 10, etc. - *result = c-'a'+10; - } - else { - *result = -1; - } - - if ((*result >=0) && (*result < radix)) - return true; - - return false; -} - -INT32 wtoi(_In_reads_(length) WCHAR* wstr, DWORD length) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - PRECONDITION(CheckPointer(wstr)); - PRECONDITION(length >= 0); - } - CONTRACTL_END; - - DWORD i = 0; - int value; - INT32 result = 0; - - while ( (i < length) && (IsDigit(wstr[i], 10 ,&value)) ) { - //Read all of the digits and convert to a number - result = result*10 + value; - i++; - } - - return result; -} - - - // // // EXCEPTION NATIVE diff --git a/src/coreclr/vm/util.cpp b/src/coreclr/vm/util.cpp index 5f3a5bf922c44..9713f2fd56438 100644 --- a/src/coreclr/vm/util.cpp +++ b/src/coreclr/vm/util.cpp @@ -2160,23 +2160,6 @@ BOOL COMCharacter::nativeIsWhiteSpace(WCHAR c) #endif // !TARGET_UNIX } -/*================================nativeIsDigit================================= -**The locally available version of IsDigit. Designed to be called by other -**native methods. The work is mostly done by GetCharacterInfoHelper -**Args: c -- the character to check. -**Returns: true if c is whitespace, false otherwise. -**Exceptions: Only those thrown by GetCharacterInfoHelper. -==============================================================================*/ -BOOL COMCharacter::nativeIsDigit(WCHAR c) -{ - WRAPPER_NO_CONTRACT; -#ifndef TARGET_UNIX - return((GetCharacterInfoHelper(c, CT_CTYPE1) & C1_DIGIT)!=0); -#else // !TARGET_UNIX - return iswdigit(c); -#endif // !TARGET_UNIX -} - BOOL RuntimeFileNotFound(HRESULT hr) { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/util.hpp b/src/coreclr/vm/util.hpp index aaae98e569f0e..2292a7775cc4b 100644 --- a/src/coreclr/vm/util.hpp +++ b/src/coreclr/vm/util.hpp @@ -894,7 +894,6 @@ class COMCharacter { public: //These are here for support from native code. They are never called from our managed classes. static BOOL nativeIsWhiteSpace(WCHAR c); - static BOOL nativeIsDigit(WCHAR c); }; // ====================================================================================== From 0fa60f58d592470352fe3fbc13797d03a75a4201 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 5 Jun 2022 16:17:50 +0800 Subject: [PATCH 03/14] Fix metasig declaration --- src/coreclr/vm/metasig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 78032dbbaa336..f581aaea0e5fe 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -544,7 +544,7 @@ DEFINE_METASIG(SM(Str_RetArrStr, s, a(s))) DEFINE_METASIG_T(SM(SyncCtx_ArrIntPtr_Bool_Int_RetInt, C(SYNCHRONIZATION_CONTEXT) a(I) F i, i)) // Exception -DEFINE_METASIG(IM(RefInt_RetStr), r(i), s) +DEFINE_METASIG(IM(RefInt_RetStr, r(i), s)) #ifdef FEATURE_COMINTEROP // The signature of the method System.Runtime.InteropServices.ICustomQueryInterface.GetInterface From 1be1d903b3a2b6e2f9bce8cab8ca2ad4ed401d25 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 9 Jun 2022 15:08:32 +0800 Subject: [PATCH 04/14] Guard GetHelpContext under FEATURE_COMINTEROP --- src/coreclr/vm/corelib.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index cd4da4f84bf1c..ff721a8613ccd 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -335,7 +335,9 @@ DEFINE_PROPERTY(EXCEPTION, MESSAGE, Message, DEFINE_PROPERTY(EXCEPTION, SOURCE, Source, Str) DEFINE_PROPERTY(EXCEPTION, HELP_LINK, HelpLink, Str) DEFINE_METHOD(EXCEPTION, INTERNAL_PRESERVE_STACK_TRACE, InternalPreserveStackTrace, IM_RetVoid) +#ifdef FEATURE_COMINTEROP DEFINE_METHOD(EXCEPTION, GET_HELP_CONTEXT, GetHelpContext, IM_RefInt_RetStr) +#endif DEFINE_CLASS(SYSTEM_EXCEPTION, System, SystemException) From 1afacf7f10ea74069086e92c47cfe9941748666f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 9 Jun 2022 15:11:40 +0800 Subject: [PATCH 05/14] Remove definition of GetHelpLink and guard more methods under cominterop --- src/coreclr/vm/corelib.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index ff721a8613ccd..8871e6795a882 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -330,12 +330,11 @@ DEFINE_FIELD_U(_xptrs, ExceptionObject, _xptrs) DEFINE_FIELD_U(_xcode, ExceptionObject, _xcode) DEFINE_FIELD_U(_HResult, ExceptionObject, _HResult) DEFINE_CLASS(EXCEPTION, System, Exception) +DEFINE_METHOD(EXCEPTION, INTERNAL_PRESERVE_STACK_TRACE, InternalPreserveStackTrace, IM_RetVoid) +#ifdef FEATURE_COMINTEROP DEFINE_METHOD(EXCEPTION, GET_CLASS_NAME, GetClassName, IM_RetStr) DEFINE_PROPERTY(EXCEPTION, MESSAGE, Message, Str) DEFINE_PROPERTY(EXCEPTION, SOURCE, Source, Str) -DEFINE_PROPERTY(EXCEPTION, HELP_LINK, HelpLink, Str) -DEFINE_METHOD(EXCEPTION, INTERNAL_PRESERVE_STACK_TRACE, InternalPreserveStackTrace, IM_RetVoid) -#ifdef FEATURE_COMINTEROP DEFINE_METHOD(EXCEPTION, GET_HELP_CONTEXT, GetHelpContext, IM_RefInt_RetStr) #endif From ac5339394c182dfa34bfca307d6d0082250cc7cf Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 9 Jun 2022 16:35:45 +0800 Subject: [PATCH 06/14] DWORD should be uint --- .../System.Private.CoreLib/src/System/Exception.CoreCLR.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs index 0ea420f103d6f..ac33b4fe8e9a2 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs @@ -276,7 +276,7 @@ private bool CanSetRemoteStackTrace() } // used by vm - internal string? GetHelpContext(out int helpContext) + internal string? GetHelpContext(out uint helpContext) { helpContext = 0; string? helpFile = HelpLink; @@ -294,7 +294,7 @@ private bool CanSetRemoteStackTrace() break; } - if (int.TryParse(helpFile.AsSpan(poundPos + 1, digitEnd - poundPos - 1), out helpContext)) + if (uint.TryParse(helpFile.AsSpan(poundPos + 1, digitEnd - poundPos - 1), out helpContext)) { helpFile = helpFile.Substring(0, poundPos); } From 1c357efb2016dd8f03dbb7b18016bacf17cc957d Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 9 Jun 2022 16:37:32 +0800 Subject: [PATCH 07/14] Add help context marshaling test --- .../COM/NETClients/Primitives/ErrorTests.cs | 27 ++++++++---- .../COM/NETServer/ErrorMarshalTesting.cs | 9 ++++ .../NativeClients/Primitives/ErrorTests.cpp | 43 +++++++++++++++++++ .../COM/NativeServer/ErrorMarshalTesting.h | 19 ++++++++ .../COM/ServerContracts/Server.Contracts.cs | 2 + .../COM/ServerContracts/Server.Contracts.h | 4 ++ 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs b/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs index b741416ba56b1..5f610c24c9368 100644 --- a/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs +++ b/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs @@ -19,6 +19,7 @@ public void Run() { this.VerifyExpectedException(); this.VerifyReturnHResult(); + this.VerifyHelpLink(); } private void VerifyExpectedException() @@ -40,15 +41,15 @@ private void VerifyReturnHResult() var hrs = new[] { - unchecked((int)0x80004001), - unchecked((int)0x80004003), - unchecked((int)0x80070005), - unchecked((int)0x80070057), - unchecked((int)0x8000ffff), - -1, - 1, - 2 - }; + unchecked((int)0x80004001), + unchecked((int)0x80004003), + unchecked((int)0x80070005), + unchecked((int)0x80070057), + unchecked((int)0x8000ffff), + -1, + 1, + 2 + }; foreach (var hr in hrs) { @@ -56,5 +57,13 @@ private void VerifyReturnHResult() Assert.Equal(hr, this.server.Return_As_HResult_Struct(hr).hr); } } + + private void VerifyHelpLink() + { + string helpLink = "C:\\Windows\\system32\\dummy.hlp"; + uint helpContext = 5678; + var ex = Assert.Throws(() => { this.server.Throw_HResult_HelpLink(unchecked((int)-1), helpLink, helpContext); }); + Assert.Equal($"{helpLink}#{helpContext}", ex.HelpLink); + } } } diff --git a/src/tests/Interop/COM/NETServer/ErrorMarshalTesting.cs b/src/tests/Interop/COM/NETServer/ErrorMarshalTesting.cs index 37a3fbdbfaf4e..3d0a43a5813e5 100644 --- a/src/tests/Interop/COM/NETServer/ErrorMarshalTesting.cs +++ b/src/tests/Interop/COM/NETServer/ErrorMarshalTesting.cs @@ -30,4 +30,13 @@ public Server.Contract.HResult Return_As_HResult_Struct(int hresultToReturn) { return new Server.Contract.HResult { hr = hresultToReturn }; } + + public void Throw_HResult_HelpLink(int hresultToReturn, string helpLink, uint helpContext) + { + Marshal.GetExceptionForHR(hresultToReturn); + + Exception e = Marshal.GetExceptionForHR(hresultToReturn); + e.HelpLink = $"{helpLink}#{helpContext}"; + throw e; + } } diff --git a/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp b/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp index 99639a40e052a..20034e01c2991 100644 --- a/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp +++ b/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp @@ -75,6 +75,48 @@ namespace THROW_FAIL_IF_FALSE(hr == hrMaybe); } } + + void VerifyHelpContext(_In_ IErrorMarshalTesting *et) + { + ::printf("Verify expected helplink and context\n"); + + HRESULT hrs[] = + { + E_NOTIMPL, + E_POINTER, + E_ACCESSDENIED, + E_INVALIDARG, + E_UNEXPECTED, + HRESULT{-1}, + S_FALSE, + HRESULT{2} + }; + + BSTR helpLink = SysAllocString(OLESTR("C:\\Windows\\system32\\dummy.hlp")); + + for (int i = 0; i < ARRAY_SIZE(hrs); ++i) + { + HRESULT hr = hrs[i]; + DWORD helpContext = (DWORD)(i + 0x1234); + HRESULT hrMaybe = et->Throw_HResult_HelpLink(hr, helpLink, helpContext); + THROW_FAIL_IF_FALSE(hr == hrMaybe); + + IErrorInfo* pErrInfo; + THROW_IF_FAILED(GetErrorInfo(0, &pErrInfo)); + + BSTR helpLinkMaybe; + THROW_IF_FAILED(pErrInfo->GetHelpFile(&helpLinkMaybe)); + THROW_FAIL_IF_FALSE(VarBstrCmp(helpLink, helpLinkMaybe, LANG_ENGLISH, 0) == VARCMP_EQ); + SysFreeString(helpLinkMaybe); + + DWORD helpContextMaybe; + THROW_IF_FAILED(pErrInfo->GetHelpContext(&helpContextMaybe)); + THROW_FAIL_IF_FALSE(helpContext == helpContextMaybe); + pErrInfo->Release(); + } + + SysFreeString(helpLink); + } } void Run_ErrorTests() @@ -89,4 +131,5 @@ void Run_ErrorTests() VerifyExpectedException(errorMarshal); VerifyReturnHResult(errorMarshal); VerifyReturnHResultStruct(errorMarshal); + VerifyHelpContext(errorMarshal); } diff --git a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h index 46884b594f1dd..0d281f8ee4f9e 100644 --- a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h +++ b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h @@ -26,6 +26,25 @@ class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting return hresultToReturn; } + DEF_FUNC(Throw_HResult_HelpLink)( + /*[in]*/ int hresultToReturn, + /*[in]*/ BSTR helpLink, + /*[in]*/ DWORD helpContext) + { + ICreateErrorInfo* pCreateErrInfo; + CreateErrorInfo(&pCreateErrInfo); + pCreateErrInfo->SetHelpFile(helpLink); + pCreateErrInfo->SetHelpContext(helpContext); + + IErrorInfo* pErrInfo; + pCreateErrInfo->QueryInterface(IID_IErrorInfo, (void**)&pErrInfo); + SetErrorInfo(0, pErrInfo); + pErrInfo->Release(); + pCreateErrInfo->Release(); + + return HRESULT{ hresultToReturn }; + } + public: // IUnknown STDMETHOD(QueryInterface)( /* [in] */ REFIID riid, diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs index 30c428ca5b5d1..cd382bababb9e 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs @@ -201,6 +201,8 @@ public interface IErrorMarshalTesting [PreserveSig] HResult Return_As_HResult_Struct(int hresultToReturn); + + void Throw_HResult_HelpLink(int hresultToReturn, string helpLink, uint helpContext); } public enum IDispatchTesting_Exception diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h index 8b705a94d28ca..6e74d8b7d71bd 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h @@ -375,6 +375,10 @@ IErrorMarshalTesting : IUnknown /*[in]*/ int hresultToReturn ) = 0; virtual int STDMETHODCALLTYPE Return_As_HResult_Struct ( /*[in]*/ int hresultToReturn ) = 0; + virtual HRESULT STDMETHODCALLTYPE Throw_HResult_HelpLink ( + /*[in]*/ int hresultToReturn, + /*[in]*/ BSTR helpLink, + /*[in]*/ DWORD helpContext ) = 0; }; enum IDispatchTesting_Exception From ab2814f22851095dd4c0d213336388106633e19a Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Fri, 10 Jun 2022 13:44:28 +0800 Subject: [PATCH 08/14] Adjust marshaling test --- .../COM/NETClients/Primitives/ErrorTests.cs | 2 +- .../NativeClients/Primitives/ErrorTests.cpp | 11 ++++----- .../COM/NativeServer/ErrorMarshalTesting.h | 23 ++++++++++--------- .../COM/ServerContracts/Server.Contracts.h | 2 +- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs b/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs index 5f610c24c9368..17c5a41674f8a 100644 --- a/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs +++ b/src/tests/Interop/COM/NETClients/Primitives/ErrorTests.cs @@ -60,7 +60,7 @@ private void VerifyReturnHResult() private void VerifyHelpLink() { - string helpLink = "C:\\Windows\\system32\\dummy.hlp"; + string helpLink = "X:\\NotA\\RealPath\\dummy.hlp"; uint helpContext = 5678; var ex = Assert.Throws(() => { this.server.Throw_HResult_HelpLink(unchecked((int)-1), helpLink, helpContext); }); Assert.Equal($"{helpLink}#{helpContext}", ex.HelpLink); diff --git a/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp b/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp index 20034e01c2991..93e35a78ce499 100644 --- a/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp +++ b/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp @@ -92,7 +92,7 @@ namespace HRESULT{2} }; - BSTR helpLink = SysAllocString(OLESTR("C:\\Windows\\system32\\dummy.hlp")); + LPCWSTR helpLink = L"X:\\NotA\\RealPath\\dummy.hlp"; for (int i = 0; i < ARRAY_SIZE(hrs); ++i) { @@ -100,22 +100,19 @@ namespace DWORD helpContext = (DWORD)(i + 0x1234); HRESULT hrMaybe = et->Throw_HResult_HelpLink(hr, helpLink, helpContext); THROW_FAIL_IF_FALSE(hr == hrMaybe); - - IErrorInfo* pErrInfo; + + ComSmartPtr pErrInfo; THROW_IF_FAILED(GetErrorInfo(0, &pErrInfo)); BSTR helpLinkMaybe; THROW_IF_FAILED(pErrInfo->GetHelpFile(&helpLinkMaybe)); - THROW_FAIL_IF_FALSE(VarBstrCmp(helpLink, helpLinkMaybe, LANG_ENGLISH, 0) == VARCMP_EQ); + THROW_FAIL_IF_FALSE(TP_wcmp_s(helpLink, helpLinkMaybe) == 0); SysFreeString(helpLinkMaybe); DWORD helpContextMaybe; THROW_IF_FAILED(pErrInfo->GetHelpContext(&helpContextMaybe)); THROW_FAIL_IF_FALSE(helpContext == helpContextMaybe); - pErrInfo->Release(); } - - SysFreeString(helpLink); } } diff --git a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h index 0d281f8ee4f9e..6e0a1c7393c4e 100644 --- a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h +++ b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h @@ -28,19 +28,20 @@ class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting DEF_FUNC(Throw_HResult_HelpLink)( /*[in]*/ int hresultToReturn, - /*[in]*/ BSTR helpLink, + /*[in]*/ LPCWSTR helpLink, /*[in]*/ DWORD helpContext) { - ICreateErrorInfo* pCreateErrInfo; - CreateErrorInfo(&pCreateErrInfo); - pCreateErrInfo->SetHelpFile(helpLink); - pCreateErrInfo->SetHelpContext(helpContext); - - IErrorInfo* pErrInfo; - pCreateErrInfo->QueryInterface(IID_IErrorInfo, (void**)&pErrInfo); - SetErrorInfo(0, pErrInfo); - pErrInfo->Release(); - pCreateErrInfo->Release(); + HRESULT hr; + + ComSmartPtr pCreateErrInfo; + BSTR bstrHelpLink = SysAllocString(helpLink); + RETURN_IF_FAILED(::CreateErrorInfo(&pCreateErrInfo)); + RETURN_IF_FAILED(pCreateErrInfo->SetHelpFile(bstrHelpLink)); + RETURN_IF_FAILED(pCreateErrInfo->SetHelpContext(helpContext)); + + ComSmartPtr pErrInfo; + RETURN_IF_FAILED(pCreateErrInfo->QueryInterface(IID_IErrorInfo, (void**)&pErrInfo)); + RETURN_IF_FAILED(SetErrorInfo(0, pErrInfo)); return HRESULT{ hresultToReturn }; } diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h index 6e74d8b7d71bd..3c9a1fcb06cbe 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h @@ -377,7 +377,7 @@ IErrorMarshalTesting : IUnknown /*[in]*/ int hresultToReturn ) = 0; virtual HRESULT STDMETHODCALLTYPE Throw_HResult_HelpLink ( /*[in]*/ int hresultToReturn, - /*[in]*/ BSTR helpLink, + /*[in]*/ LPCWSTR helpLink, /*[in]*/ DWORD helpContext ) = 0; }; From b895defa3033cfb9a4a526d3a4c954ad44e2a8d8 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 11 Jun 2022 14:13:34 +0800 Subject: [PATCH 09/14] Fix #ifdef with ILLink --- src/coreclr/vm/corelib.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 8871e6795a882..e10467dc772d5 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -331,12 +331,11 @@ DEFINE_FIELD_U(_xcode, ExceptionObject, _xcode) DEFINE_FIELD_U(_HResult, ExceptionObject, _HResult) DEFINE_CLASS(EXCEPTION, System, Exception) DEFINE_METHOD(EXCEPTION, INTERNAL_PRESERVE_STACK_TRACE, InternalPreserveStackTrace, IM_RetVoid) -#ifdef FEATURE_COMINTEROP +// Following Exception members are only used when FEATURE_COMINTEROP DEFINE_METHOD(EXCEPTION, GET_CLASS_NAME, GetClassName, IM_RetStr) DEFINE_PROPERTY(EXCEPTION, MESSAGE, Message, Str) DEFINE_PROPERTY(EXCEPTION, SOURCE, Source, Str) DEFINE_METHOD(EXCEPTION, GET_HELP_CONTEXT, GetHelpContext, IM_RefInt_RetStr) -#endif DEFINE_CLASS(SYSTEM_EXCEPTION, System, SystemException) From 4676d9326085b1285841786b97d739a13f872d7d Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 13 Jun 2022 20:29:12 +0800 Subject: [PATCH 10/14] Fix method signature mismatch --- src/coreclr/vm/corelib.h | 2 +- src/coreclr/vm/metasig.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index e10467dc772d5..54eaf1c0d6f60 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -335,7 +335,7 @@ DEFINE_METHOD(EXCEPTION, INTERNAL_PRESERVE_STACK_TRACE, InternalPrese DEFINE_METHOD(EXCEPTION, GET_CLASS_NAME, GetClassName, IM_RetStr) DEFINE_PROPERTY(EXCEPTION, MESSAGE, Message, Str) DEFINE_PROPERTY(EXCEPTION, SOURCE, Source, Str) -DEFINE_METHOD(EXCEPTION, GET_HELP_CONTEXT, GetHelpContext, IM_RefInt_RetStr) +DEFINE_METHOD(EXCEPTION, GET_HELP_CONTEXT, GetHelpContext, IM_RefUInt_RetStr) DEFINE_CLASS(SYSTEM_EXCEPTION, System, SystemException) diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index f581aaea0e5fe..f1e7b7fce43ee 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -544,7 +544,7 @@ DEFINE_METASIG(SM(Str_RetArrStr, s, a(s))) DEFINE_METASIG_T(SM(SyncCtx_ArrIntPtr_Bool_Int_RetInt, C(SYNCHRONIZATION_CONTEXT) a(I) F i, i)) // Exception -DEFINE_METASIG(IM(RefInt_RetStr, r(i), s)) +DEFINE_METASIG(IM(RefUInt_RetStr, r(K), s)) #ifdef FEATURE_COMINTEROP // The signature of the method System.Runtime.InteropServices.ICustomQueryInterface.GetInterface From 70bd67019535f7d46dbbe6c403ced3c3cd5e445a Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 13 Jun 2022 21:32:34 +0800 Subject: [PATCH 11/14] Specify string marshaling --- src/tests/Interop/COM/ServerContracts/Server.Contracts.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs index cd382bababb9e..758c200acaaba 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs @@ -202,7 +202,7 @@ public interface IErrorMarshalTesting [PreserveSig] HResult Return_As_HResult_Struct(int hresultToReturn); - void Throw_HResult_HelpLink(int hresultToReturn, string helpLink, uint helpContext); + void Throw_HResult_HelpLink(int hresultToReturn, [MarshalAs(UnmanagedType.LPWStr)] string helpLink, uint helpContext); } public enum IDispatchTesting_Exception From e896e23d1c47047b7fadc808b40946b55137ed74 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 13 Jun 2022 21:38:22 +0800 Subject: [PATCH 12/14] Throwing test should not test successful HRESULT --- .../Interop/COM/NativeClients/Primitives/ErrorTests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp b/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp index 93e35a78ce499..bd8cdc0e9bdc8 100644 --- a/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp +++ b/src/tests/Interop/COM/NativeClients/Primitives/ErrorTests.cpp @@ -85,11 +85,10 @@ namespace E_NOTIMPL, E_POINTER, E_ACCESSDENIED, + E_OUTOFMEMORY, E_INVALIDARG, E_UNEXPECTED, - HRESULT{-1}, - S_FALSE, - HRESULT{2} + HRESULT{-1} }; LPCWSTR helpLink = L"X:\\NotA\\RealPath\\dummy.hlp"; From 0d4e3851198aa72a56e6f765d263e12bc86db0c1 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 13 Jun 2022 23:10:26 +0800 Subject: [PATCH 13/14] Implement ISupportErrorInfo --- .../Interop/COM/NativeServer/ErrorMarshalTesting.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h index 6e0a1c7393c4e..7c204747f5a47 100644 --- a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h +++ b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h @@ -5,7 +5,7 @@ #include "Servers.h" -class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting +class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting, public ISupportErrorInfo { public: // IErrorMarshalTesting DEF_FUNC(Throw_HResult)( @@ -46,12 +46,18 @@ class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting return HRESULT{ hresultToReturn }; } + DEF_FUNC(InterfaceSupportsErrorInfo)( + /* [in] */ __RPC__in REFIID riid) + { + return S_OK; + } + public: // IUnknown STDMETHOD(QueryInterface)( /* [in] */ REFIID riid, /* [iid_is][out] */ _COM_Outptr_ void __RPC_FAR *__RPC_FAR *ppvObject) { - return DoQueryInterface(riid, ppvObject, static_cast(this)); + return DoQueryInterface(riid, ppvObject, static_cast(this), static_cast(this)); } DEFINE_REF_COUNTING(); From 8fc7de6b43fa0d8b93a018d5c7f79c85530aa228 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 14 Jun 2022 13:06:08 +0800 Subject: [PATCH 14/14] Test interface in InterfaceSupportsErrorInfo --- src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h index 7c204747f5a47..b9c48a95565ea 100644 --- a/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h +++ b/src/tests/Interop/COM/NativeServer/ErrorMarshalTesting.h @@ -49,7 +49,7 @@ class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting, pub DEF_FUNC(InterfaceSupportsErrorInfo)( /* [in] */ __RPC__in REFIID riid) { - return S_OK; + return riid == IID_IErrorMarshalTesting ? S_OK : S_FALSE; } public: // IUnknown