From 983da443fbeac5bf5139d01ecfb3bae0807dc7ae Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 26 Mar 2024 19:40:50 -0700 Subject: [PATCH 01/11] Support System.Guid marshalling via VARIANT VARIANT marshalling in .NET 5+ requires a TLB for COM records (i.e., ValueType instances). This means that without a runtime provided TLB, users must define their own TLB for runtime types or define their own transfer types. We address this here by deferring to the NetFX mscorlib's TLB. --- src/coreclr/vm/stdinterfaces.cpp | 47 ++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/coreclr/vm/stdinterfaces.cpp b/src/coreclr/vm/stdinterfaces.cpp index 08af895c3baec..9edfc58a92e84 100644 --- a/src/coreclr/vm/stdinterfaces.cpp +++ b/src/coreclr/vm/stdinterfaces.cpp @@ -611,6 +611,46 @@ HRESULT GetITypeLibForAssembly(_In_ Assembly *pAssembly, _Outptr_ ITypeLib **ppT return S_OK; } // HRESULT GetITypeLibForAssembly() +// There are types that are helpful to provide that facilitate porting from +// .NET Framework to .NET 8+. This function is used to acquire their ITypeInfo. +// This should be used narrowly. Types at a minimum should be blittable. +static bool TryDeferToMscorlib(MethodTable* pClass, ITypeInfo** ppTI) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_PREEMPTIVE; + PRECONDITION(pClass != NULL); + PRECONDITION(pClass->IsBlittable()); + PRECONDITION(ppTI != NULL); + } + CONTRACTL_END; + + StackSString className; + pClass->_GetFullyQualifiedNameForClass(className); + + // Marshalling of a System.Guid is such a common scenario, let's see if we can load + // the .NET Framework's TLB. This is a niche scenario, but one that impacts many teams + // porting code to .NET 8+. + if (strcmp(className.GetUTF8(), g_GuidClassName) == 0) + { + SafeComHolder pMscorlibTypeLib = NULL; + + // .NET Frameworks' mscorlib TLB GUID. + const GUID mscorlib = { 0xBED7F4EA, 0x1A96, 0x11D2, { 0x8F, 0x08, 0x00, 0xA0, 0xC9, 0xA6, 0x18, 0x6D } }; + if (SUCCEEDED(::LoadRegTypeLib(mscorlib, 2, 4, 0, &pMscorlibTypeLib))) + { + // Hard-coded GUID for System.Guid. + const GUID guidForSystemGuid = { 0x9C5923E9, 0xDE52, 0x33EA, { 0x88, 0xDE, 0x7E, 0xBC, 0x86, 0x33, 0xB9, 0xCC } }; + if (SUCCEEDED(pMscorlibTypeLib->GetTypeInfoOfGuid(guidForSystemGuid, ppTI))) + return true; + } + } + + return false; +} + HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClassInfo) { CONTRACTL @@ -625,6 +665,7 @@ HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClas GUID clsid; GUID ciid; ComMethodTable *pComMT = NULL; + MethodTable* pOriginalClass = pClass; HRESULT hr = S_OK; SafeComHolder pITLB = NULL; SafeComHolder pTI = NULL; @@ -770,6 +811,12 @@ HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClas { if (!FAILED(hr)) hr = E_FAIL; + + if (pOriginalClass->IsValueType() && pOriginalClass->IsBlittable()) + { + if (TryDeferToMscorlib(pOriginalClass, ppTI)) + hr = S_OK; + } } ReturnHR: From 9fabe5577396d4708fcf71278826b4c01bddcce0 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 27 Mar 2024 16:22:11 -0700 Subject: [PATCH 02/11] Add marshalling of System.Guid in the other direction. --- src/coreclr/vm/olevariant.cpp | 17 ++++++-- src/coreclr/vm/stdinterfaces.cpp | 73 +++++++++++++++++++++++++++----- src/coreclr/vm/stdinterfaces.h | 3 ++ 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/coreclr/vm/olevariant.cpp b/src/coreclr/vm/olevariant.cpp index 888ebdd380dae..5c9b41d6e1cfc 100644 --- a/src/coreclr/vm/olevariant.cpp +++ b/src/coreclr/vm/olevariant.cpp @@ -2573,9 +2573,20 @@ void OleVariant::MarshalRecordVariantOleToCom(VARIANT *pOleVariant, LPVOID pvRecord = V_RECORD(pOleVariant); if (pvRecord) { - // This value type should have been registered through - // a TLB. CoreCLR doesn't support dynamic type mapping. - COMPlusThrow(kArgumentException, IDS_EE_CANNOT_MAP_TO_MANAGED_VC); + MethodTable* pValueClass = GetMethodTableForRecordInfo(pRecInfo); + if (pValueClass == NULL) + { + // This value type should have been registered through + // a TLB. CoreCLR doesn't support dynamic type mapping. + COMPlusThrow(kArgumentException, IDS_EE_CANNOT_MAP_TO_MANAGED_VC); + } + + _ASSERTE(pValueClass->IsBlittable()); + + // Now that we have a blittable value class, allocate an instance of the + // boxed value class and copy the contents of the record into it. + BoxedValueClass = AllocateObject(pValueClass); + memcpyNoGCRefs(BoxedValueClass->GetData(), (BYTE*)pvRecord, pValueClass->GetNativeSize()); } pComVariant->SetObjRef(BoxedValueClass); diff --git a/src/coreclr/vm/stdinterfaces.cpp b/src/coreclr/vm/stdinterfaces.cpp index 9edfc58a92e84..68edd4ec23f65 100644 --- a/src/coreclr/vm/stdinterfaces.cpp +++ b/src/coreclr/vm/stdinterfaces.cpp @@ -611,6 +611,12 @@ HRESULT GetITypeLibForAssembly(_In_ Assembly *pAssembly, _Outptr_ ITypeLib **ppT return S_OK; } // HRESULT GetITypeLibForAssembly() +// .NET Frameworks' mscorlib TLB GUID. +static const GUID s_MscorlibGuid = { 0xBED7F4EA, 0x1A96, 0x11D2, { 0x8F, 0x08, 0x00, 0xA0, 0xC9, 0xA6, 0x18, 0x6D } }; + +// Hard-coded GUID for System.Guid. +static const GUID s_GuidForSystemGuid = { 0x9C5923E9, 0xDE52, 0x33EA, { 0x88, 0xDE, 0x7E, 0xBC, 0x86, 0x33, 0xB9, 0xCC } }; + // There are types that are helpful to provide that facilitate porting from // .NET Framework to .NET 8+. This function is used to acquire their ITypeInfo. // This should be used narrowly. Types at a minimum should be blittable. @@ -627,23 +633,15 @@ static bool TryDeferToMscorlib(MethodTable* pClass, ITypeInfo** ppTI) } CONTRACTL_END; - StackSString className; - pClass->_GetFullyQualifiedNameForClass(className); - // Marshalling of a System.Guid is such a common scenario, let's see if we can load // the .NET Framework's TLB. This is a niche scenario, but one that impacts many teams // porting code to .NET 8+. - if (strcmp(className.GetUTF8(), g_GuidClassName) == 0) + if (pClass == CoreLibBinder::GetClass(CLASS__GUID)) { SafeComHolder pMscorlibTypeLib = NULL; - - // .NET Frameworks' mscorlib TLB GUID. - const GUID mscorlib = { 0xBED7F4EA, 0x1A96, 0x11D2, { 0x8F, 0x08, 0x00, 0xA0, 0xC9, 0xA6, 0x18, 0x6D } }; - if (SUCCEEDED(::LoadRegTypeLib(mscorlib, 2, 4, 0, &pMscorlibTypeLib))) + if (SUCCEEDED(::LoadRegTypeLib(s_MscorlibGuid, 2, 4, 0, &pMscorlibTypeLib))) { - // Hard-coded GUID for System.Guid. - const GUID guidForSystemGuid = { 0x9C5923E9, 0xDE52, 0x33EA, { 0x88, 0xDE, 0x7E, 0xBC, 0x86, 0x33, 0xB9, 0xCC } }; - if (SUCCEEDED(pMscorlibTypeLib->GetTypeInfoOfGuid(guidForSystemGuid, ppTI))) + if (SUCCEEDED(pMscorlibTypeLib->GetTypeInfoOfGuid(s_GuidForSystemGuid, ppTI))) return true; } } @@ -823,6 +821,59 @@ HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClas return hr; } // HRESULT GetITypeInfoForEEClass() +MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo) +{ + CONTRACTL + { + NOTHROW; + GC_TRIGGERS; + MODE_ANY; + PRECONDITION(recInfo != NULL); + } + CONTRACTL_END; + + HRESULT hr; + + // + // Only a narrow set of types are supported. + // See TryDeferToMscorlib() above. + // + + // Verify the associated TypeLib attribute + SafeComHolder typeInfo; + hr = recInfo->GetTypeInfo(&typeInfo); + if (FAILED(hr)) + return NULL; + + SafeComHolder typeLib; + UINT index; + hr = typeInfo->GetContainingTypeLib(&typeLib, &index); + if (FAILED(hr)) + return NULL; + + TLIBATTR* attrs; + hr = typeLib->GetLibAttr(&attrs); + if (FAILED(hr)) + return NULL; + + GUID libGuid = attrs->guid; + typeLib->ReleaseTLibAttr(attrs); + if (s_MscorlibGuid != libGuid) + return NULL; + + // Verify the Guid of the associated type + GUID typeGuid; + hr = recInfo->GetGuid(&typeGuid); + if (FAILED(hr)) + return NULL; + + // Check for supported types. + if (s_GuidForSystemGuid == typeGuid) + return CoreLibBinder::GetClass(CLASS__GUID); + + return NULL; +} + // Returns a NON-ADDREF'd ITypeInfo. HRESULT GetITypeInfoForMT(ComMethodTable *pMT, ITypeInfo **ppTI) { diff --git a/src/coreclr/vm/stdinterfaces.h b/src/coreclr/vm/stdinterfaces.h index 8d6201439657b..517ca810b33ab 100644 --- a/src/coreclr/vm/stdinterfaces.h +++ b/src/coreclr/vm/stdinterfaces.h @@ -183,4 +183,7 @@ IErrorInfo *GetSupportedErrorInfo(IUnknown *iface, REFIID riid); // Helpers to get the ITypeInfo* for a type. HRESULT GetITypeInfoForEEClass(MethodTable *pMT, ITypeInfo **ppTI, bool bClassInfo = false); +// Gets the MethodTable for the associated IRecordInfo. +MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo); + #endif From b7bd5562cc43bfb2de9acc80c10d7479bc2a05ea Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 27 Mar 2024 16:24:28 -0700 Subject: [PATCH 03/11] Adding testing for COM VARIANT marshalling --- src/tests/Interop/CMakeLists.txt | 1 + .../COM/NETClients/MiscTypes/App.manifest | 18 ++ .../MiscTypes/NetClientMiscTypes.csproj | 18 ++ .../COM/NETClients/MiscTypes/Program.cs | 90 +++++++++ .../Interop/COM/NETServer/MiscTypesTesting.cs | 51 ++++++ .../COM/NativeClients/MiscTypes.csproj | 6 + .../COM/NativeClients/MiscTypes/App.manifest | 17 ++ .../NativeClients/MiscTypes/CMakeLists.txt | 22 +++ .../MiscTypes/CoreShim.X.manifest | 16 ++ .../COM/NativeClients/MiscTypes/MiscTypes.cpp | 171 ++++++++++++++++++ .../Primitives/CoreShim.X.manifest | 4 + .../NativeServer/COMNativeServer.X.manifest | 5 + .../COM/NativeServer/MiscTypesTesting.h | 31 ++++ .../Interop/COM/NativeServer/Servers.cpp | 5 + src/tests/Interop/COM/NativeServer/Servers.h | 4 + .../COM/ServerContracts/Server.CoClasses.cs | 33 +++- .../COM/ServerContracts/Server.Contracts.cs | 11 ++ .../COM/ServerContracts/Server.Contracts.h | 12 ++ .../COM/ServerContracts/ServerGuids.cs | 1 + 19 files changed, 509 insertions(+), 7 deletions(-) create mode 100644 src/tests/Interop/COM/NETClients/MiscTypes/App.manifest create mode 100644 src/tests/Interop/COM/NETClients/MiscTypes/NetClientMiscTypes.csproj create mode 100644 src/tests/Interop/COM/NETClients/MiscTypes/Program.cs create mode 100644 src/tests/Interop/COM/NETServer/MiscTypesTesting.cs create mode 100644 src/tests/Interop/COM/NativeClients/MiscTypes.csproj create mode 100644 src/tests/Interop/COM/NativeClients/MiscTypes/App.manifest create mode 100644 src/tests/Interop/COM/NativeClients/MiscTypes/CMakeLists.txt create mode 100644 src/tests/Interop/COM/NativeClients/MiscTypes/CoreShim.X.manifest create mode 100644 src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp create mode 100644 src/tests/Interop/COM/NativeServer/MiscTypesTesting.h diff --git a/src/tests/Interop/CMakeLists.txt b/src/tests/Interop/CMakeLists.txt index 070b4e562eb43..fa3217993a8a7 100644 --- a/src/tests/Interop/CMakeLists.txt +++ b/src/tests/Interop/CMakeLists.txt @@ -81,6 +81,7 @@ if(CLR_CMAKE_TARGET_WIN32) add_subdirectory(COM/NativeClients/DefaultInterfaces) add_subdirectory(COM/NativeClients/Dispatch) add_subdirectory(COM/NativeClients/Events) + add_subdirectory(COM/NativeClients/MiscTypes) add_subdirectory(COM/ComWrappers/MockReferenceTrackerRuntime) add_subdirectory(COM/ComWrappers/WeakReference) diff --git a/src/tests/Interop/COM/NETClients/MiscTypes/App.manifest b/src/tests/Interop/COM/NETClients/MiscTypes/App.manifest new file mode 100644 index 0000000000000..93dcb090e865c --- /dev/null +++ b/src/tests/Interop/COM/NETClients/MiscTypes/App.manifest @@ -0,0 +1,18 @@ + + + + + + + + + + + + diff --git a/src/tests/Interop/COM/NETClients/MiscTypes/NetClientMiscTypes.csproj b/src/tests/Interop/COM/NETClients/MiscTypes/NetClientMiscTypes.csproj new file mode 100644 index 0000000000000..bd343f7dc8f9a --- /dev/null +++ b/src/tests/Interop/COM/NETClients/MiscTypes/NetClientMiscTypes.csproj @@ -0,0 +1,18 @@ + + + + true + App.manifest + true + + + + + + + + + + + + diff --git a/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs b/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs new file mode 100644 index 0000000000000..e602f288a9445 --- /dev/null +++ b/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs @@ -0,0 +1,90 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; +namespace NetClient +{ + using System; + using System.Runtime.InteropServices; + + using TestLibrary; + using Xunit; + using Server.Contract; + using Server.Contract.Servers; + + public unsafe class Program + { + [Fact] + public static int TestEntryPoint() + { + // RegFree COM is not supported on Windows Nano + if (TestLibrary.Utilities.IsWindowsNanoServer) + { + return 100; + } + + try + { + RunTests(); + } + catch (Exception e) + { + Console.WriteLine($"Test object interop failure: {e}"); + return 101; + } + + return 100; + } + + private static void RunTests() + { + var miscTypeTesting = (Server.Contract.Servers.MiscTypesTesting)new Server.Contract.Servers.MiscTypesTestingClass(); + + Console.WriteLine("Validate Primitives <=> VARIANT..."); + { + object expected = null; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = DBNull.Value; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = (sbyte)0x0f; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = (short)0x07ff; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = (int)0x07ffffff; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = (long)0x07ffffffffffffff; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = true; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + { + var expected = false; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + + Console.WriteLine("Validate BSTR <=> VARIANT..."); + { + var expected = "The quick Fox jumped over the lazy Dog."; + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + + Console.WriteLine("Validate System.Guid <=> VARIANT..."); + { + var expected = new Guid("{8EFAD956-B33D-46CB-90F4-45F55BA68A96}"); + Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); + } + } + } +} diff --git a/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs b/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs new file mode 100644 index 0000000000000..d758b6d3e30e0 --- /dev/null +++ b/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs @@ -0,0 +1,51 @@ +// 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.CompilerServices; +using System.Runtime.InteropServices; + +[ComVisible(true)] +[Guid(Server.Contract.Guids.MiscTypesTesting)] +public class MiscTypesTesting : Server.Contract.IMiscTypesTesting +{ + object Server.Contract.IMiscTypesTesting.Marshal_Variant(object obj) + { + if (obj is null) + { + return null; + } + else if (obj is DBNull) + { + return DBNull.Value; + } + else if (obj.GetType().IsValueType) + { + return CallMemberwiseClone(obj); + } + else if (obj is string) + { + return obj; + } + + Environment.FailFast($"Arguments must be ValueTypes or strings: {obj.GetType()}"); + return null; + + // object.MemberwiseClone() will bitwise copy for ValueTypes. + // This is sufficient for the VARIANT marshalling scenario being + // tested here. + [UnsafeAccessor(UnsafeAccessorKind.Method, Name = "MemberwiseClone")] + static extern object CallMemberwiseClone(object obj); + } + + object Server.Contract.IMiscTypesTesting.Marshal_Instance_Variant(string init) + { + if (Guid.TryParse(init, out Guid result)) + { + return result; + } + + Environment.FailFast($"Unknown init value: {init}"); + return null; + } +} \ No newline at end of file diff --git a/src/tests/Interop/COM/NativeClients/MiscTypes.csproj b/src/tests/Interop/COM/NativeClients/MiscTypes.csproj new file mode 100644 index 0000000000000..83409dcfceb26 --- /dev/null +++ b/src/tests/Interop/COM/NativeClients/MiscTypes.csproj @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/tests/Interop/COM/NativeClients/MiscTypes/App.manifest b/src/tests/Interop/COM/NativeClients/MiscTypes/App.manifest new file mode 100644 index 0000000000000..20ffce48d342f --- /dev/null +++ b/src/tests/Interop/COM/NativeClients/MiscTypes/App.manifest @@ -0,0 +1,17 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/src/tests/Interop/COM/NativeClients/MiscTypes/CMakeLists.txt b/src/tests/Interop/COM/NativeClients/MiscTypes/CMakeLists.txt new file mode 100644 index 0000000000000..3dcba4671143e --- /dev/null +++ b/src/tests/Interop/COM/NativeClients/MiscTypes/CMakeLists.txt @@ -0,0 +1,22 @@ +project (COMClientMiscTypes) +include_directories( ${INC_PLATFORM_DIR} ) +include_directories( "../../ServerContracts" ) +include_directories( "../../NativeServer" ) +include_directories("../") +set(SOURCES + MiscTypes.cpp + App.manifest) + +# add the executable +add_executable (COMClientMiscTypes ${SOURCES}) +target_link_libraries(COMClientMiscTypes PRIVATE ${LINK_LIBRARIES_ADDITIONAL}) + +# Copy CoreShim manifest to project output +file(GENERATE OUTPUT $/CoreShim.X.manifest INPUT ${CMAKE_CURRENT_SOURCE_DIR}/CoreShim.X.manifest) + +# add the install targets +install (TARGETS COMClientMiscTypes DESTINATION bin) +# If there's a dynamic ASAN runtime, then copy it to project output. +if (NOT "${ASAN_RUNTIME}" STREQUAL "") + file(COPY "${ASAN_RUNTIME}" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}") +endif() diff --git a/src/tests/Interop/COM/NativeClients/MiscTypes/CoreShim.X.manifest b/src/tests/Interop/COM/NativeClients/MiscTypes/CoreShim.X.manifest new file mode 100644 index 0000000000000..a3c8593ee0676 --- /dev/null +++ b/src/tests/Interop/COM/NativeClients/MiscTypes/CoreShim.X.manifest @@ -0,0 +1,16 @@ + + + + + + + + + + + diff --git a/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp b/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp new file mode 100644 index 0000000000000..c137ef860a592 --- /dev/null +++ b/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp @@ -0,0 +1,171 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include +#include +#include +#include + +// COM headers +#include +#include + +#define COM_CLIENT +#include + +#define THROW_IF_FAILED(exp) { hr = exp; if (FAILED(hr)) { ::printf("FAILURE: 0x%08x = %s\n", hr, #exp); throw hr; } } +#define THROW_FAIL_IF_FALSE(exp) { if (!(exp)) { ::printf("FALSE: %s\n", #exp); throw E_FAIL; } } + +template +struct ComInit +{ + const HRESULT Result; + + ComInit() + : Result{ ::CoInitializeEx(nullptr, TM) } + { } + + ~ComInit() + { + if (SUCCEEDED(Result)) + ::CoUninitialize(); + } +}; + +using ComMTA = ComInit; +void ValidateMiscTypes(); + +int __cdecl main() +{ + if (is_windows_nano() == S_OK) + { + ::puts("RegFree COM is not supported on Windows Nano. Auto-passing this test.\n"); + return 100; + } + ComMTA init; + if (FAILED(init.Result)) + return -1; + + try + { + CoreShimComActivation csact{ W("NETServer"), W("MiscTypesTesting") }; + ValidateMiscTypes(); + } + catch (HRESULT hr) + { + ::printf("Test Failure: 0x%08x\n", hr); + return 101; + } + + return 100; +} + +struct VariantMarshalTest +{ + VARIANT Input; + VARIANT Result; + VariantMarshalTest() + { + ::VariantInit(&Input); + ::VariantInit(&Result); + } + ~VariantMarshalTest() + { + ::VariantClear(&Input); + ::VariantClear(&Result); + } +}; + +void ValidateMiscTypes() +{ + ::printf("MiscTypes test through CoCreateInstance...\n"); + + HRESULT hr; + + IMiscTypesTesting *miscTypesTesting; + THROW_IF_FAILED(::CoCreateInstance(CLSID_MiscTypesTesting, nullptr, CLSCTX_INPROC, IID_IMiscTypesTesting, (void**)&miscTypesTesting)); + + ::printf("Validate Primitives <=> VARIANT...\n"); + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_EMPTY; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_VT(&args.Input) == V_VT(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_NULL; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_VT(&args.Input) == V_VT(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_I1; + V_I1(&args.Input) = 0x0f; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_I1(&args.Input) == V_I1(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_I2; + V_I2(&args.Input) = 0x07ff; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_I2(&args.Input) == V_I2(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_I4; + V_I4(&args.Input) = 0x07ffffff; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_I4(&args.Input) == V_I4(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_I8; + V_I8(&args.Input) = 0x07ffffffffffffff; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_I8(&args.Input) == V_I8(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_BOOL; + V_BOOL(&args.Input) = VARIANT_TRUE; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_BOOL(&args.Input) == V_BOOL(&args.Result)); + } + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_BOOL; + V_BOOL(&args.Input) = VARIANT_FALSE; + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_BOOL(&args.Input) == V_BOOL(&args.Result)); + } + + ::printf("Validate BSTR <=> VARIANT...\n"); + { + VariantMarshalTest args{}; + V_VT(&args.Input) = VT_BSTR; + V_BSTR(&args.Input) = ::SysAllocString(W("The quick Fox jumped over the lazy Dog.")); + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(CompareStringOrdinal(V_BSTR(&args.Input), -1, V_BSTR(&args.Result), -1, FALSE) == CSTR_EQUAL); + } + + ::printf("Validate System.Guid <=> VARIANT...\n"); + { + /* 8EFAD956-B33D-46CB-90F4-45F55BA68A96 */ + const GUID expected = { 0x8EFAD956, 0xB33D, 0x46CB, { 0x90, 0xF4, 0x45, 0xF5, 0x5B, 0xA6, 0x8A, 0x96} }; + + // Get a System.Guid into native + VariantMarshalTest guidVar; + THROW_IF_FAILED(miscTypesTesting->Marshal_Instance_Variant(W("{8EFAD956-B33D-46CB-90F4-45F55BA68A96}"), &guidVar.Result)); + THROW_FAIL_IF_FALSE(V_VT(&guidVar.Result) == VT_RECORD); + THROW_FAIL_IF_FALSE(memcmp(V_RECORD(&guidVar.Result), &expected, sizeof(expected)) == 0); + + // Use the Guid as input. + VariantMarshalTest args{}; + THROW_IF_FAILED(::VariantCopy(&args.Input, &guidVar.Result)); + THROW_IF_FAILED(miscTypesTesting->Marshal_Variant(args.Input, &args.Result)); + THROW_FAIL_IF_FALSE(V_VT(&args.Input) == V_VT(&args.Result)); + THROW_FAIL_IF_FALSE(memcmp(V_RECORD(&args.Input), V_RECORD(&args.Result), sizeof(expected)) == 0); + } +} diff --git a/src/tests/Interop/COM/NativeClients/Primitives/CoreShim.X.manifest b/src/tests/Interop/COM/NativeClients/Primitives/CoreShim.X.manifest index 099f3a36e169b..8b8e6ad135a2e 100644 --- a/src/tests/Interop/COM/NativeClients/Primitives/CoreShim.X.manifest +++ b/src/tests/Interop/COM/NativeClients/Primitives/CoreShim.X.manifest @@ -19,6 +19,10 @@ + + + + + +#include "Servers.h" + +class MiscTypesTesting : public UnknownImpl, public IMiscTypesTesting +{ +public: // IMiscTypesTesting + DEF_FUNC(Marshal_Variant)(_In_ VARIANT obj, _Out_ VARIANT* result) + { + return ::VariantCopy(result, &obj); + } + + DEF_FUNC(Marshal_Instance_Variant)(_In_ LPCWSTR init, _Out_ VARIANT* result) + { + return E_NOTIMPL; + } + +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)); + } + + DEFINE_REF_COUNTING(); +}; \ No newline at end of file diff --git a/src/tests/Interop/COM/NativeServer/Servers.cpp b/src/tests/Interop/COM/NativeServer/Servers.cpp index f2becfe4d0941..05f26be8d4741 100644 --- a/src/tests/Interop/COM/NativeServer/Servers.cpp +++ b/src/tests/Interop/COM/NativeServer/Servers.cpp @@ -162,6 +162,7 @@ STDAPI DllRegisterServer(void) RETURN_IF_FAILED(RegisterClsid(__uuidof(NumericTesting), L"Both")); RETURN_IF_FAILED(RegisterClsid(__uuidof(ArrayTesting), L"Both")); RETURN_IF_FAILED(RegisterClsid(__uuidof(StringTesting), L"Both")); + RETURN_IF_FAILED(RegisterClsid(__uuidof(MiscTypesTesting), L"Both")); RETURN_IF_FAILED(RegisterClsid(__uuidof(ErrorMarshalTesting), L"Both")); RETURN_IF_FAILED(RegisterClsid(__uuidof(DispatchTesting), L"Both")); RETURN_IF_FAILED(RegisterClsid(__uuidof(EventTesting), L"Both")); @@ -180,6 +181,7 @@ STDAPI DllUnregisterServer(void) RETURN_IF_FAILED(RemoveClsid(__uuidof(NumericTesting))); RETURN_IF_FAILED(RemoveClsid(__uuidof(ArrayTesting))); RETURN_IF_FAILED(RemoveClsid(__uuidof(StringTesting))); + RETURN_IF_FAILED(RemoveClsid(__uuidof(MiscTypesTesting))); RETURN_IF_FAILED(RemoveClsid(__uuidof(ErrorMarshalTesting))); RETURN_IF_FAILED(RemoveClsid(__uuidof(DispatchTesting))); RETURN_IF_FAILED(RemoveClsid(__uuidof(EventTesting))); @@ -202,6 +204,9 @@ STDAPI DllGetClassObject(_In_ REFCLSID rclsid, _In_ REFIID riid, _Out_ LPVOID FA if (rclsid == __uuidof(StringTesting)) return ClassFactoryBasic::Create(riid, ppv); + if (rclsid == __uuidof(MiscTypesTesting)) + return ClassFactoryBasic::Create(riid, ppv); + if (rclsid == __uuidof(ErrorMarshalTesting)) return ClassFactoryBasic::Create(riid, ppv); diff --git a/src/tests/Interop/COM/NativeServer/Servers.h b/src/tests/Interop/COM/NativeServer/Servers.h index 7c9ec0300bc67..c87288d2535b1 100644 --- a/src/tests/Interop/COM/NativeServer/Servers.h +++ b/src/tests/Interop/COM/NativeServer/Servers.h @@ -12,6 +12,7 @@ class DECLSPEC_UUID("53169A33-E85D-4E3C-B668-24E438D0929B") NumericTesting; class DECLSPEC_UUID("B99ABE6A-DFF6-440F-BFB6-55179B8FE18E") ArrayTesting; class DECLSPEC_UUID("C73C83E8-51A2-47F8-9B5C-4284458E47A6") StringTesting; +class DECLSPEC_UUID("CCFF894B-A27C-45E0-9B30-6C88D722E843") MiscTypesTesting; class DECLSPEC_UUID("71CF5C45-106C-4B32-B418-43A463C6041F") ErrorMarshalTesting; class DECLSPEC_UUID("0F8ACD0C-ECE0-4F2A-BD1B-6BFCA93A0726") DispatchTesting; class DECLSPEC_UUID("4DBD9B61-E372-499F-84DE-EFC70AA8A009") EventTesting; @@ -25,6 +26,7 @@ class DECLSPEC_UUID("4F54231D-9E11-4C0B-8E0B-2EBD8B0E5811") TrackMyLifetimeTesti #define CLSID_NumericTesting __uuidof(NumericTesting) #define CLSID_ArrayTesting __uuidof(ArrayTesting) #define CLSID_StringTesting __uuidof(StringTesting) +#define CLSID_MiscTypesTesting __uuidof(MiscTypesTesting) #define CLSID_ErrorMarshalTesting __uuidof(ErrorMarshalTesting) #define CLSID_DispatchTesting __uuidof(DispatchTesting) #define CLSID_EventTesting __uuidof(EventTesting) @@ -38,6 +40,7 @@ class DECLSPEC_UUID("4F54231D-9E11-4C0B-8E0B-2EBD8B0E5811") TrackMyLifetimeTesti #define IID_INumericTesting __uuidof(INumericTesting) #define IID_IArrayTesting __uuidof(IArrayTesting) #define IID_IStringTesting __uuidof(IStringTesting) +#define IID_IMiscTypesTesting __uuidof(IMiscTypesTesting) #define IID_IErrorMarshalTesting __uuidof(IErrorMarshalTesting) #define IID_IDispatchTesting __uuidof(IDispatchTesting) #define IID_TestingEvents __uuidof(TestingEvents) @@ -82,6 +85,7 @@ struct CoreShimComActivation #include "NumericTesting.h" #include "ArrayTesting.h" #include "StringTesting.h" + #include "MiscTypesTesting.h" #include "ErrorMarshalTesting.h" #include "DispatchTesting.h" #include "EventTesting.h" diff --git a/src/tests/Interop/COM/ServerContracts/Server.CoClasses.cs b/src/tests/Interop/COM/ServerContracts/Server.CoClasses.cs index 0b6f988f1a7a9..2479e6cd6f083 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.CoClasses.cs +++ b/src/tests/Interop/COM/ServerContracts/Server.CoClasses.cs @@ -10,7 +10,7 @@ namespace Server.Contract.Servers using System.Runtime.InteropServices; /// - /// Managed definition of CoClass + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(NumericTestingClass))] @@ -29,7 +29,7 @@ internal class NumericTestingClass } /// - /// Managed definition of CoClass + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(ArrayTestingClass))] @@ -48,7 +48,7 @@ internal class ArrayTestingClass } /// - /// Managed definition of CoClass + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(StringTestingClass))] @@ -67,7 +67,26 @@ internal class StringTestingClass } /// - /// Managed definition of CoClass + /// Managed definition of CoClass + /// + [ComImport] + [CoClass(typeof(MiscTypesTestingClass))] + [Guid("7FBB8677-BDD0-4E5A-B38B-CA92A4555466")] + internal interface MiscTypesTesting : Server.Contract.IMiscTypesTesting + { + } + + /// + /// Managed activation for CoClass + /// + [ComImport] + [Guid(Server.Contract.Guids.MiscTypesTesting)] + internal class MiscTypesTestingClass + { + } + + /// + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(ErrorMarshalTestingClass))] @@ -86,7 +105,7 @@ internal class ErrorMarshalTestingClass } /// - /// Managed definition of CoClass + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(DispatchTestingClass))] @@ -105,7 +124,7 @@ internal class DispatchTestingClass } /// - /// Managed definition of CoClass + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(AggregationTestingClass))] @@ -124,7 +143,7 @@ internal class AggregationTestingClass } /// - /// Managed definition of CoClass + /// Managed definition of CoClass /// [ComImport] [CoClass(typeof(ColorTestingClass))] diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs index 0bac21e66ee17..dd0f71634e2bd 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.cs @@ -184,6 +184,17 @@ string Add_BStr( void Pass_Through_LCID(out int lcid); } + [ComVisible(true)] + [Guid("7FBB8677-BDD0-4E5A-B38B-CA92A4555466")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + public interface IMiscTypesTesting + { + object Marshal_Variant(object obj); + + // Test API for marshalling an arbitrary type via VARIANT + object Marshal_Instance_Variant([MarshalAs(UnmanagedType.LPWStr)] string init); + } + public struct HResult { public int hr; diff --git a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h index 1eb0528aae4b7..d2c26884589ef 100644 --- a/src/tests/Interop/COM/ServerContracts/Server.Contracts.h +++ b/src/tests/Interop/COM/ServerContracts/Server.Contracts.h @@ -366,6 +366,18 @@ IStringTesting : IUnknown /*[out]*/ LCID* outLcid) = 0; }; +struct __declspec(uuid("7FBB8677-BDD0-4E5A-B38B-CA92A4555466")) +IMiscTypesTesting : IUnknown +{ + virtual HRESULT STDMETHODCALLTYPE Marshal_Variant ( + /*[in]*/ VARIANT obj, + /*[out,retval]*/ VARIANT* result) = 0; + + virtual HRESULT STDMETHODCALLTYPE Marshal_Instance_Variant ( + /*[in]*/ LPCWSTR init, + /*[out,retval]*/ VARIANT* result) = 0; +}; + struct __declspec(uuid("592386a5-6837-444d-9de3-250815d18556")) IErrorMarshalTesting : IUnknown { diff --git a/src/tests/Interop/COM/ServerContracts/ServerGuids.cs b/src/tests/Interop/COM/ServerContracts/ServerGuids.cs index 5336cde54106e..8b0c65a3ce153 100644 --- a/src/tests/Interop/COM/ServerContracts/ServerGuids.cs +++ b/src/tests/Interop/COM/ServerContracts/ServerGuids.cs @@ -11,6 +11,7 @@ internal sealed class Guids public const string NumericTesting = "53169A33-E85D-4E3C-B668-24E438D0929B"; public const string ArrayTesting = "B99ABE6A-DFF6-440F-BFB6-55179B8FE18E"; public const string StringTesting = "C73C83E8-51A2-47F8-9B5C-4284458E47A6"; + public const string MiscTypesTesting = "CCFF894B-A27C-45E0-9B30-6C88D722E843"; public const string ErrorMarshalTesting = "71CF5C45-106C-4B32-B418-43A463C6041F"; public const string DispatchTesting = "0F8ACD0C-ECE0-4F2A-BD1B-6BFCA93A0726"; public const string EventTesting = "4DBD9B61-E372-499F-84DE-EFC70AA8A009"; From c85557f468385be5988cbb96a2ce0beae7a9a257 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 27 Mar 2024 21:26:26 -0700 Subject: [PATCH 04/11] Fix contract --- src/coreclr/vm/stdinterfaces.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/stdinterfaces.cpp b/src/coreclr/vm/stdinterfaces.cpp index 68edd4ec23f65..8d14f1b6f20f7 100644 --- a/src/coreclr/vm/stdinterfaces.cpp +++ b/src/coreclr/vm/stdinterfaces.cpp @@ -825,7 +825,7 @@ MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo) { CONTRACTL { - NOTHROW; + THROWS; GC_TRIGGERS; MODE_ANY; PRECONDITION(recInfo != NULL); From bf2bb940e2ea056dbcca59f86c95585e67cdb92c Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 28 Mar 2024 11:04:55 -0700 Subject: [PATCH 05/11] Fix libraries tests --- src/coreclr/vm/olevariant.cpp | 42 +++++++++++-------- src/coreclr/vm/stdinterfaces.cpp | 2 +- .../Marshal/GetObjectForNativeVariantTests.cs | 26 +++++++++++- 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/coreclr/vm/olevariant.cpp b/src/coreclr/vm/olevariant.cpp index 5c9b41d6e1cfc..40e039a1648ef 100644 --- a/src/coreclr/vm/olevariant.cpp +++ b/src/coreclr/vm/olevariant.cpp @@ -2567,28 +2567,34 @@ void OleVariant::MarshalRecordVariantOleToCom(VARIANT *pOleVariant, if (!pRecInfo) COMPlusThrow(kArgumentException, IDS_EE_INVALID_OLE_VARIANT); - OBJECTREF BoxedValueClass = NULL; - GCPROTECT_BEGIN(BoxedValueClass) + LPVOID pvRecord = V_RECORD(pOleVariant); + if (pvRecord == NULL) { - LPVOID pvRecord = V_RECORD(pOleVariant); - if (pvRecord) - { - MethodTable* pValueClass = GetMethodTableForRecordInfo(pRecInfo); - if (pValueClass == NULL) - { - // This value type should have been registered through - // a TLB. CoreCLR doesn't support dynamic type mapping. - COMPlusThrow(kArgumentException, IDS_EE_CANNOT_MAP_TO_MANAGED_VC); - } + pComVariant->SetObjRef(NULL); + return; + } - _ASSERTE(pValueClass->IsBlittable()); + MethodTable* pValueClass = NULL; + { + GCX_PREEMP(); + pValueClass = GetMethodTableForRecordInfo(pRecInfo); + } - // Now that we have a blittable value class, allocate an instance of the - // boxed value class and copy the contents of the record into it. - BoxedValueClass = AllocateObject(pValueClass); - memcpyNoGCRefs(BoxedValueClass->GetData(), (BYTE*)pvRecord, pValueClass->GetNativeSize()); - } + if (pValueClass == NULL) + { + // This value type should have been registered through + // a TLB. CoreCLR doesn't support dynamic type mapping. + COMPlusThrow(kArgumentException, IDS_EE_CANNOT_MAP_TO_MANAGED_VC); + } + _ASSERTE(pValueClass->IsBlittable()); + OBJECTREF BoxedValueClass = NULL; + GCPROTECT_BEGIN(BoxedValueClass) + { + // Now that we have a blittable value class, allocate an instance of the + // boxed value class and copy the contents of the record into it. + BoxedValueClass = AllocateObject(pValueClass); + memcpyNoGCRefs(BoxedValueClass->GetData(), (BYTE*)pvRecord, pValueClass->GetNativeSize()); pComVariant->SetObjRef(BoxedValueClass); } GCPROTECT_END(); diff --git a/src/coreclr/vm/stdinterfaces.cpp b/src/coreclr/vm/stdinterfaces.cpp index 8d14f1b6f20f7..eeb2bafbb6f50 100644 --- a/src/coreclr/vm/stdinterfaces.cpp +++ b/src/coreclr/vm/stdinterfaces.cpp @@ -827,7 +827,7 @@ MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo) { THROWS; GC_TRIGGERS; - MODE_ANY; + MODE_PREEMPTIVE; PRECONDITION(recInfo != NULL); } CONTRACTL_END; diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetObjectForNativeVariantTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetObjectForNativeVariantTests.cs index c0b68f5d899ec..f4e01dc3d87e4 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetObjectForNativeVariantTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetObjectForNativeVariantTests.cs @@ -246,14 +246,38 @@ public void GetObjectForNativeVariant_InvalidDate_ThrowsArgumentException(double } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))] - public void GetObjectForNativeVariant_NoDataForRecord_ThrowsArgumentException() + public void GetObjectForNativeVariant_NoRecordInfo_ThrowsArgumentException() { Variant variant = CreateVariant(VT_RECORD, new UnionTypes { _record = new Record { _recordInfo = IntPtr.Zero } }); AssertExtensions.Throws(null, () => GetObjectForNativeVariant(variant)); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))] + public void GetObjectForNativeVariant_NoRecordData_ReturnsNull() + { + var recordInfo = new RecordInfo(); + IntPtr pRecordInfo = Marshal.GetComInterfaceForObject(recordInfo); + try + { + Variant variant = CreateVariant(VT_RECORD, new UnionTypes + { + _record = new Record + { + _record = IntPtr.Zero, + _recordInfo = pRecordInfo + } + }); + Assert.Null(GetObjectForNativeVariant(variant)); + } + finally + { + Marshal.Release(pRecordInfo); + } + } + public static IEnumerable GetObjectForNativeVariant_NoSuchGuid_TestData() { + yield return new object[] { typeof(object).GUID }; yield return new object[] { typeof(string).GUID }; yield return new object[] { Guid.Empty }; } From 29ce8d05a472cdb8b866fc95dd880ed0ae07c38d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 4 Apr 2024 08:59:23 -0700 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: Elinor Fung --- src/coreclr/vm/stdinterfaces.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/stdinterfaces.cpp b/src/coreclr/vm/stdinterfaces.cpp index eeb2bafbb6f50..54903df8c59be 100644 --- a/src/coreclr/vm/stdinterfaces.cpp +++ b/src/coreclr/vm/stdinterfaces.cpp @@ -611,7 +611,7 @@ HRESULT GetITypeLibForAssembly(_In_ Assembly *pAssembly, _Outptr_ ITypeLib **ppT return S_OK; } // HRESULT GetITypeLibForAssembly() -// .NET Frameworks' mscorlib TLB GUID. +// .NET Framework's mscorlib TLB GUID. static const GUID s_MscorlibGuid = { 0xBED7F4EA, 0x1A96, 0x11D2, { 0x8F, 0x08, 0x00, 0xA0, 0xC9, 0xA6, 0x18, 0x6D } }; // Hard-coded GUID for System.Guid. @@ -633,9 +633,8 @@ static bool TryDeferToMscorlib(MethodTable* pClass, ITypeInfo** ppTI) } CONTRACTL_END; - // Marshalling of a System.Guid is such a common scenario, let's see if we can load - // the .NET Framework's TLB. This is a niche scenario, but one that impacts many teams - // porting code to .NET 8+. + // Marshalling of System.Guid is a common scenario that impacts many teams porting + // code to .NET 8+. Try to load the .NET Framework's TLB to support this scenario. if (pClass == CoreLibBinder::GetClass(CLASS__GUID)) { SafeComHolder pMscorlibTypeLib = NULL; From 7e268673637408ad4b42588ee4870a606f3ebae9 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 4 Apr 2024 11:29:34 -0700 Subject: [PATCH 07/11] Review feedback --- src/coreclr/vm/stdinterfaces.cpp | 7 ++--- .../COM/NETClients/MiscTypes/Program.cs | 27 +++++++++++++++---- .../COM/NativeClients/MiscTypes/MiscTypes.cpp | 14 +++++----- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/stdinterfaces.cpp b/src/coreclr/vm/stdinterfaces.cpp index 54903df8c59be..3131f33d2892b 100644 --- a/src/coreclr/vm/stdinterfaces.cpp +++ b/src/coreclr/vm/stdinterfaces.cpp @@ -820,6 +820,8 @@ HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClas return hr; } // HRESULT GetITypeInfoForEEClass() +// Only a narrow set of types are supported. +// See TryDeferToMscorlib() above. MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo) { CONTRACTL @@ -833,11 +835,6 @@ MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo) HRESULT hr; - // - // Only a narrow set of types are supported. - // See TryDeferToMscorlib() above. - // - // Verify the associated TypeLib attribute SafeComHolder typeInfo; hr = recInfo->GetTypeInfo(&typeInfo); diff --git a/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs b/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs index e602f288a9445..de4945b5af13b 100644 --- a/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs +++ b/src/tests/Interop/COM/NETClients/MiscTypes/Program.cs @@ -12,6 +12,8 @@ namespace NetClient using Server.Contract; using Server.Contract.Servers; + struct Struct {} + public unsafe class Program { [Fact] @@ -25,7 +27,8 @@ public static int TestEntryPoint() try { - RunTests(); + ValidationTests(); + ValidateNegativeTests(); } catch (Exception e) { @@ -36,11 +39,13 @@ public static int TestEntryPoint() return 100; } - private static void RunTests() + private static void ValidationTests() { + Console.WriteLine($"Running {nameof(ValidationTests)} ..."); + var miscTypeTesting = (Server.Contract.Servers.MiscTypesTesting)new Server.Contract.Servers.MiscTypesTestingClass(); - Console.WriteLine("Validate Primitives <=> VARIANT..."); + Console.WriteLine("-- Primitives <=> VARIANT..."); { object expected = null; Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); @@ -74,17 +79,29 @@ private static void RunTests() Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); } - Console.WriteLine("Validate BSTR <=> VARIANT..."); + Console.WriteLine("-- BSTR <=> VARIANT..."); { var expected = "The quick Fox jumped over the lazy Dog."; Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); } - Console.WriteLine("Validate System.Guid <=> VARIANT..."); + Console.WriteLine("-- System.Guid <=> VARIANT..."); { var expected = new Guid("{8EFAD956-B33D-46CB-90F4-45F55BA68A96}"); Assert.Equal(expected, miscTypeTesting.Marshal_Variant(expected)); } } + + private static void ValidateNegativeTests() + { + Console.WriteLine($"Running {nameof(ValidateNegativeTests)} ..."); + + var miscTypeTesting = (Server.Contract.Servers.MiscTypesTesting)new Server.Contract.Servers.MiscTypesTestingClass(); + + Console.WriteLine("-- User defined ValueType <=> VARIANT..."); + { + Assert.Throws(() => miscTypeTesting.Marshal_Variant(new Struct())); + } + } } } diff --git a/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp b/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp index c137ef860a592..6fb435be6513c 100644 --- a/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp +++ b/src/tests/Interop/COM/NativeClients/MiscTypes/MiscTypes.cpp @@ -33,7 +33,7 @@ struct ComInit }; using ComMTA = ComInit; -void ValidateMiscTypes(); +void ValidationTests(); int __cdecl main() { @@ -49,7 +49,7 @@ int __cdecl main() try { CoreShimComActivation csact{ W("NETServer"), W("MiscTypesTesting") }; - ValidateMiscTypes(); + ValidationTests(); } catch (HRESULT hr) { @@ -76,16 +76,16 @@ struct VariantMarshalTest } }; -void ValidateMiscTypes() +void ValidationTests() { - ::printf("MiscTypes test through CoCreateInstance...\n"); + ::printf(__FUNCTION__ "() through CoCreateInstance...\n"); HRESULT hr; IMiscTypesTesting *miscTypesTesting; THROW_IF_FAILED(::CoCreateInstance(CLSID_MiscTypesTesting, nullptr, CLSCTX_INPROC, IID_IMiscTypesTesting, (void**)&miscTypesTesting)); - ::printf("Validate Primitives <=> VARIANT...\n"); + ::printf("-- Primitives <=> VARIANT...\n"); { VariantMarshalTest args{}; V_VT(&args.Input) = VT_EMPTY; @@ -141,7 +141,7 @@ void ValidateMiscTypes() THROW_FAIL_IF_FALSE(V_BOOL(&args.Input) == V_BOOL(&args.Result)); } - ::printf("Validate BSTR <=> VARIANT...\n"); + ::printf("-- BSTR <=> VARIANT...\n"); { VariantMarshalTest args{}; V_VT(&args.Input) = VT_BSTR; @@ -150,7 +150,7 @@ void ValidateMiscTypes() THROW_FAIL_IF_FALSE(CompareStringOrdinal(V_BSTR(&args.Input), -1, V_BSTR(&args.Result), -1, FALSE) == CSTR_EQUAL); } - ::printf("Validate System.Guid <=> VARIANT...\n"); + ::printf("-- System.Guid <=> VARIANT...\n"); { /* 8EFAD956-B33D-46CB-90F4-45F55BA68A96 */ const GUID expected = { 0x8EFAD956, 0xB33D, 0x46CB, { 0x90, 0xF4, 0x45, 0xF5, 0x5B, 0xA6, 0x8A, 0x96} }; From ee4d6bb3a5607f83e4660d3cee56bd21860925ed Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 4 Apr 2024 12:10:46 -0700 Subject: [PATCH 08/11] Add Guid test for GetObjectForNativeVariant() --- .../Marshal/GetNativeVariantForObjectTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs index c25c59a205c96..bf06309f5d982 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs @@ -167,6 +167,33 @@ public void GetNativeVariantForObject_String_Success(string obj) } } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))] + public unsafe void GetNativeVariantForObject_Guid_Success() + { + var guid = new Guid("0DD3E51B-3162-4D13-B906-030F402C5BA2"); + var v = new Variant(); + IntPtr pNative = Marshal.AllocHGlobal(Marshal.SizeOf(v)); + try + { + Marshal.GetNativeVariantForObject(guid, pNative); + + Variant result = Marshal.PtrToStructure(pNative); + Assert.Equal(VarEnum.VT_RECORD, (VarEnum)result.vt); + Assert.NotEqual(nint.Zero, result.pRecInfo); // We should have an IRecordInfo instance. + + var expectedBytes = new ReadOnlySpan(guid.ToByteArray()); + var actualBytes = new ReadOnlySpan((void*)result.bstrVal, expectedBytes.Length); + Assert.Equal(expectedBytes, actualBytes); + + object o = Marshal.GetObjectForNativeVariant(pNative); + Assert.Equal(guid, o); + } + finally + { + Marshal.FreeHGlobal(pNative); + } + } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))] [InlineData(3.14)] public unsafe void GetNativeVariantForObject_Double_Success(double obj) From 215b58181b2eb78b62473c762011a4a98e3bc42d Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 4 Apr 2024 14:58:10 -0700 Subject: [PATCH 09/11] Handle the Nano server case. --- .../Marshal/GetNativeVariantForObjectTests.cs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs index bf06309f5d982..108a842a3c8b3 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs @@ -175,18 +175,25 @@ public unsafe void GetNativeVariantForObject_Guid_Success() IntPtr pNative = Marshal.AllocHGlobal(Marshal.SizeOf(v)); try { - Marshal.GetNativeVariantForObject(guid, pNative); + if (PlatformDetection.IsWindowsNanoServer) + { + Assert.Throws(() => Marshal.GetNativeVariantForObject(guid, pNative)); + } + else + { + Marshal.GetNativeVariantForObject(guid, pNative); - Variant result = Marshal.PtrToStructure(pNative); - Assert.Equal(VarEnum.VT_RECORD, (VarEnum)result.vt); - Assert.NotEqual(nint.Zero, result.pRecInfo); // We should have an IRecordInfo instance. + Variant result = Marshal.PtrToStructure(pNative); + Assert.Equal(VarEnum.VT_RECORD, (VarEnum)result.vt); + Assert.NotEqual(nint.Zero, result.pRecInfo); // We should have an IRecordInfo instance. - var expectedBytes = new ReadOnlySpan(guid.ToByteArray()); - var actualBytes = new ReadOnlySpan((void*)result.bstrVal, expectedBytes.Length); - Assert.Equal(expectedBytes, actualBytes); + var expectedBytes = new ReadOnlySpan(guid.ToByteArray()); + var actualBytes = new ReadOnlySpan((void*)result.bstrVal, expectedBytes.Length); + Assert.Equal(expectedBytes, actualBytes); - object o = Marshal.GetObjectForNativeVariant(pNative); - Assert.Equal(guid, o); + object o = Marshal.GetObjectForNativeVariant(pNative); + Assert.Equal(guid, o); + } } finally { From b092250eeb911cd3757edde70c847d1074c25722 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 5 Apr 2024 08:46:07 -0700 Subject: [PATCH 10/11] Add test for dynamic scenarios --- .../ComInterop/DynamicVariantExtensions.cs | 2 -- .../Runtime/InteropServices/Marshalling/ComVariant.cs | 5 +++++ src/tests/Interop/COM/Dynamic/BasicTest.cs | 11 +++++++++++ src/tests/Interop/COM/NETServer/MiscTypesTesting.cs | 9 ++++++--- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/DynamicVariantExtensions.cs b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/DynamicVariantExtensions.cs index c8608ce2c9dfa..5b8f45469ddcd 100644 --- a/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/DynamicVariantExtensions.cs +++ b/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/DynamicVariantExtensions.cs @@ -249,8 +249,6 @@ public static unsafe void SetAsByrefVariantIndirect(ref this ComVariant variant, variant.SetAsByrefVariant(ref value); return; case VarEnum.VT_RECORD: - // VT_RECORD's are weird in that regardless of is the VT_BYREF flag is set or not - // they have the same internal representation. variant = ComVariant.CreateRaw(value.VarType | VarEnum.VT_BYREF, value.GetRawDataRef()); break; case VarEnum.VT_DECIMAL: diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs index af1c58b9e97ac..4a1027a525ad8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs @@ -404,7 +404,12 @@ public static unsafe ComVariant CreateRaw(VarEnum vt, T rawValue) (VarEnum.VT_UNKNOWN or VarEnum.VT_DISPATCH or VarEnum.VT_LPSTR or VarEnum.VT_BSTR or VarEnum.VT_LPWSTR or VarEnum.VT_SAFEARRAY or VarEnum.VT_CLSID or VarEnum.VT_STREAM or VarEnum.VT_STREAMED_OBJECT or VarEnum.VT_STORAGE or VarEnum.VT_STORED_OBJECT or VarEnum.VT_CF or VT_VERSIONED_STREAM, _) when sizeof(T) == nint.Size => rawValue, (VarEnum.VT_CY or VarEnum.VT_FILETIME, 8) => rawValue, + + // VT_RECORD's are weird in that regardless of is the VT_BYREF flag is set or not + // they have the same internal representation. (VarEnum.VT_RECORD, _) when sizeof(T) == sizeof(Record) => rawValue, + (VarEnum.VT_RECORD | VarEnum.VT_BYREF, _) when sizeof(T) == sizeof(Record) => rawValue, + _ when vt.HasFlag(VarEnum.VT_BYREF) && sizeof(T) == nint.Size => rawValue, _ when vt.HasFlag(VarEnum.VT_VECTOR) && sizeof(T) == sizeof(Vector) => rawValue, _ when vt.HasFlag(VarEnum.VT_ARRAY) && sizeof(T) == nint.Size => rawValue, diff --git a/src/tests/Interop/COM/Dynamic/BasicTest.cs b/src/tests/Interop/COM/Dynamic/BasicTest.cs index 4ec5d6fbbdccc..0d1125bdfc129 100644 --- a/src/tests/Interop/COM/Dynamic/BasicTest.cs +++ b/src/tests/Interop/COM/Dynamic/BasicTest.cs @@ -43,6 +43,7 @@ public void Run() String(); Date(); + SpecialCasedValueTypes(); ComObject(); Null(); @@ -385,6 +386,16 @@ private void Date() Variant(val, expected); } + private void SpecialCasedValueTypes() + { + { + var val = Guid.NewGuid(); + var expected = val; + // Pass as variant + Variant(val, expected); + } + } + private void ComObject() { Type t = Type.GetTypeFromCLSID(Guid.Parse(ServerGuids.BasicTest)); diff --git a/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs b/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs index d758b6d3e30e0..2a31507d29ab2 100644 --- a/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs +++ b/src/tests/Interop/COM/NETServer/MiscTypesTesting.cs @@ -15,15 +15,18 @@ object Server.Contract.IMiscTypesTesting.Marshal_Variant(object obj) { return null; } - else if (obj is DBNull) + + if (obj is DBNull) { return DBNull.Value; } - else if (obj.GetType().IsValueType) + + if (obj.GetType().IsValueType) { return CallMemberwiseClone(obj); } - else if (obj is string) + + if (obj is string) { return obj; } From ecd9e47a2ff9ea6541b36ac5a4f396e7e2ce35c5 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 5 Apr 2024 09:08:09 -0700 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Elinor Fung --- .../System/Runtime/InteropServices/Marshalling/ComVariant.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs index 4a1027a525ad8..3b9640e285ab7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs @@ -405,10 +405,9 @@ public static unsafe ComVariant CreateRaw(VarEnum vt, T rawValue) or VarEnum.VT_CLSID or VarEnum.VT_STREAM or VarEnum.VT_STREAMED_OBJECT or VarEnum.VT_STORAGE or VarEnum.VT_STORED_OBJECT or VarEnum.VT_CF or VT_VERSIONED_STREAM, _) when sizeof(T) == nint.Size => rawValue, (VarEnum.VT_CY or VarEnum.VT_FILETIME, 8) => rawValue, - // VT_RECORD's are weird in that regardless of is the VT_BYREF flag is set or not + // VT_RECORDs are weird in that regardless of whether the VT_BYREF flag is set or not // they have the same internal representation. - (VarEnum.VT_RECORD, _) when sizeof(T) == sizeof(Record) => rawValue, - (VarEnum.VT_RECORD | VarEnum.VT_BYREF, _) when sizeof(T) == sizeof(Record) => rawValue, + (VarEnum.VT_RECORD or VarEnum.VT_RECORD | VarEnum.VT_BYREF, _) when sizeof(T) == sizeof(Record) => rawValue, _ when vt.HasFlag(VarEnum.VT_BYREF) && sizeof(T) == nint.Size => rawValue, _ when vt.HasFlag(VarEnum.VT_VECTOR) && sizeof(T) == sizeof(Vector) => rawValue,