Skip to content

Commit ee33cff

Browse files
AaronRobinsonMSFTelinor-fung
authored andcommitted
Special casing System.Guid for COM VARIANT marshalling (dotnet#100377)
* 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. Co-authored-by: Elinor Fung <elfung@microsoft.com>
1 parent daf19d5 commit ee33cff

File tree

27 files changed

+726
-19
lines changed

27 files changed

+726
-19
lines changed

src/coreclr/vm/olevariant.cpp

+25-8
Original file line numberDiff line numberDiff line change
@@ -2567,17 +2567,34 @@ void OleVariant::MarshalRecordVariantOleToCom(VARIANT *pOleVariant,
25672567
if (!pRecInfo)
25682568
COMPlusThrow(kArgumentException, IDS_EE_INVALID_OLE_VARIANT);
25692569

2570+
LPVOID pvRecord = V_RECORD(pOleVariant);
2571+
if (pvRecord == NULL)
2572+
{
2573+
pComVariant->SetObjRef(NULL);
2574+
return;
2575+
}
2576+
2577+
MethodTable* pValueClass = NULL;
2578+
{
2579+
GCX_PREEMP();
2580+
pValueClass = GetMethodTableForRecordInfo(pRecInfo);
2581+
}
2582+
2583+
if (pValueClass == NULL)
2584+
{
2585+
// This value type should have been registered through
2586+
// a TLB. CoreCLR doesn't support dynamic type mapping.
2587+
COMPlusThrow(kArgumentException, IDS_EE_CANNOT_MAP_TO_MANAGED_VC);
2588+
}
2589+
_ASSERTE(pValueClass->IsBlittable());
2590+
25702591
OBJECTREF BoxedValueClass = NULL;
25712592
GCPROTECT_BEGIN(BoxedValueClass)
25722593
{
2573-
LPVOID pvRecord = V_RECORD(pOleVariant);
2574-
if (pvRecord)
2575-
{
2576-
// This value type should have been registered through
2577-
// a TLB. CoreCLR doesn't support dynamic type mapping.
2578-
COMPlusThrow(kArgumentException, IDS_EE_CANNOT_MAP_TO_MANAGED_VC);
2579-
}
2580-
2594+
// Now that we have a blittable value class, allocate an instance of the
2595+
// boxed value class and copy the contents of the record into it.
2596+
BoxedValueClass = AllocateObject(pValueClass);
2597+
memcpyNoGCRefs(BoxedValueClass->GetData(), (BYTE*)pvRecord, pValueClass->GetNativeSize());
25812598
pComVariant->SetObjRef(BoxedValueClass);
25822599
}
25832600
GCPROTECT_END();

src/coreclr/vm/stdinterfaces.cpp

+94
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,43 @@ HRESULT GetITypeLibForAssembly(_In_ Assembly *pAssembly, _Outptr_ ITypeLib **ppT
611611
return S_OK;
612612
} // HRESULT GetITypeLibForAssembly()
613613

614+
// .NET Framework's mscorlib TLB GUID.
615+
static const GUID s_MscorlibGuid = { 0xBED7F4EA, 0x1A96, 0x11D2, { 0x8F, 0x08, 0x00, 0xA0, 0xC9, 0xA6, 0x18, 0x6D } };
616+
617+
// Hard-coded GUID for System.Guid.
618+
static const GUID s_GuidForSystemGuid = { 0x9C5923E9, 0xDE52, 0x33EA, { 0x88, 0xDE, 0x7E, 0xBC, 0x86, 0x33, 0xB9, 0xCC } };
619+
620+
// There are types that are helpful to provide that facilitate porting from
621+
// .NET Framework to .NET 8+. This function is used to acquire their ITypeInfo.
622+
// This should be used narrowly. Types at a minimum should be blittable.
623+
static bool TryDeferToMscorlib(MethodTable* pClass, ITypeInfo** ppTI)
624+
{
625+
CONTRACTL
626+
{
627+
THROWS;
628+
GC_TRIGGERS;
629+
MODE_PREEMPTIVE;
630+
PRECONDITION(pClass != NULL);
631+
PRECONDITION(pClass->IsBlittable());
632+
PRECONDITION(ppTI != NULL);
633+
}
634+
CONTRACTL_END;
635+
636+
// Marshalling of System.Guid is a common scenario that impacts many teams porting
637+
// code to .NET 8+. Try to load the .NET Framework's TLB to support this scenario.
638+
if (pClass == CoreLibBinder::GetClass(CLASS__GUID))
639+
{
640+
SafeComHolder<ITypeLib> pMscorlibTypeLib = NULL;
641+
if (SUCCEEDED(::LoadRegTypeLib(s_MscorlibGuid, 2, 4, 0, &pMscorlibTypeLib)))
642+
{
643+
if (SUCCEEDED(pMscorlibTypeLib->GetTypeInfoOfGuid(s_GuidForSystemGuid, ppTI)))
644+
return true;
645+
}
646+
}
647+
648+
return false;
649+
}
650+
614651
HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClassInfo)
615652
{
616653
CONTRACTL
@@ -625,6 +662,7 @@ HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClas
625662
GUID clsid;
626663
GUID ciid;
627664
ComMethodTable *pComMT = NULL;
665+
MethodTable* pOriginalClass = pClass;
628666
HRESULT hr = S_OK;
629667
SafeComHolder<ITypeLib> pITLB = NULL;
630668
SafeComHolder<ITypeInfo> pTI = NULL;
@@ -770,12 +808,68 @@ HRESULT GetITypeInfoForEEClass(MethodTable *pClass, ITypeInfo **ppTI, bool bClas
770808
{
771809
if (!FAILED(hr))
772810
hr = E_FAIL;
811+
812+
if (pOriginalClass->IsValueType() && pOriginalClass->IsBlittable())
813+
{
814+
if (TryDeferToMscorlib(pOriginalClass, ppTI))
815+
hr = S_OK;
816+
}
773817
}
774818

775819
ReturnHR:
776820
return hr;
777821
} // HRESULT GetITypeInfoForEEClass()
778822

823+
// Only a narrow set of types are supported.
824+
// See TryDeferToMscorlib() above.
825+
MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo)
826+
{
827+
CONTRACTL
828+
{
829+
THROWS;
830+
GC_TRIGGERS;
831+
MODE_PREEMPTIVE;
832+
PRECONDITION(recInfo != NULL);
833+
}
834+
CONTRACTL_END;
835+
836+
HRESULT hr;
837+
838+
// Verify the associated TypeLib attribute
839+
SafeComHolder<ITypeInfo> typeInfo;
840+
hr = recInfo->GetTypeInfo(&typeInfo);
841+
if (FAILED(hr))
842+
return NULL;
843+
844+
SafeComHolder<ITypeLib> typeLib;
845+
UINT index;
846+
hr = typeInfo->GetContainingTypeLib(&typeLib, &index);
847+
if (FAILED(hr))
848+
return NULL;
849+
850+
TLIBATTR* attrs;
851+
hr = typeLib->GetLibAttr(&attrs);
852+
if (FAILED(hr))
853+
return NULL;
854+
855+
GUID libGuid = attrs->guid;
856+
typeLib->ReleaseTLibAttr(attrs);
857+
if (s_MscorlibGuid != libGuid)
858+
return NULL;
859+
860+
// Verify the Guid of the associated type
861+
GUID typeGuid;
862+
hr = recInfo->GetGuid(&typeGuid);
863+
if (FAILED(hr))
864+
return NULL;
865+
866+
// Check for supported types.
867+
if (s_GuidForSystemGuid == typeGuid)
868+
return CoreLibBinder::GetClass(CLASS__GUID);
869+
870+
return NULL;
871+
}
872+
779873
// Returns a NON-ADDREF'd ITypeInfo.
780874
HRESULT GetITypeInfoForMT(ComMethodTable *pMT, ITypeInfo **ppTI)
781875
{

src/coreclr/vm/stdinterfaces.h

+3
Original file line numberDiff line numberDiff line change
@@ -183,4 +183,7 @@ IErrorInfo *GetSupportedErrorInfo(IUnknown *iface, REFIID riid);
183183
// Helpers to get the ITypeInfo* for a type.
184184
HRESULT GetITypeInfoForEEClass(MethodTable *pMT, ITypeInfo **ppTI, bool bClassInfo = false);
185185

186+
// Gets the MethodTable for the associated IRecordInfo.
187+
MethodTable* GetMethodTableForRecordInfo(IRecordInfo* recInfo);
188+
186189
#endif

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

-2
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,6 @@ public static unsafe void SetAsByrefVariantIndirect(ref this ComVariant variant,
249249
variant.SetAsByrefVariant(ref value);
250250
return;
251251
case VarEnum.VT_RECORD:
252-
// VT_RECORD's are weird in that regardless of is the VT_BYREF flag is set or not
253-
// they have the same internal representation.
254252
variant = ComVariant.CreateRaw(value.VarType | VarEnum.VT_BYREF, value.GetRawDataRef<Record>());
255253
break;
256254
case VarEnum.VT_DECIMAL:

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ComVariant.cs

+5-1
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,11 @@ public static unsafe ComVariant CreateRaw<T>(VarEnum vt, T rawValue)
404404
(VarEnum.VT_UNKNOWN or VarEnum.VT_DISPATCH or VarEnum.VT_LPSTR or VarEnum.VT_BSTR or VarEnum.VT_LPWSTR or VarEnum.VT_SAFEARRAY
405405
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,
406406
(VarEnum.VT_CY or VarEnum.VT_FILETIME, 8) => rawValue,
407-
(VarEnum.VT_RECORD, _) when sizeof(T) == sizeof(Record) => rawValue,
407+
408+
// VT_RECORDs are weird in that regardless of whether the VT_BYREF flag is set or not
409+
// they have the same internal representation.
410+
(VarEnum.VT_RECORD or VarEnum.VT_RECORD | VarEnum.VT_BYREF, _) when sizeof(T) == sizeof(Record) => rawValue,
411+
408412
_ when vt.HasFlag(VarEnum.VT_BYREF) && sizeof(T) == nint.Size => rawValue,
409413
_ when vt.HasFlag(VarEnum.VT_VECTOR) && sizeof(T) == sizeof(Vector<byte>) => rawValue,
410414
_ when vt.HasFlag(VarEnum.VT_ARRAY) && sizeof(T) == nint.Size => rawValue,

src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetNativeVariantForObjectTests.cs

+34
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,40 @@ public void GetNativeVariantForObject_String_Success(string obj)
167167
}
168168
}
169169

170+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))]
171+
public unsafe void GetNativeVariantForObject_Guid_Success()
172+
{
173+
var guid = new Guid("0DD3E51B-3162-4D13-B906-030F402C5BA2");
174+
var v = new Variant();
175+
IntPtr pNative = Marshal.AllocHGlobal(Marshal.SizeOf(v));
176+
try
177+
{
178+
if (PlatformDetection.IsWindowsNanoServer)
179+
{
180+
Assert.Throws<NotSupportedException>(() => Marshal.GetNativeVariantForObject(guid, pNative));
181+
}
182+
else
183+
{
184+
Marshal.GetNativeVariantForObject(guid, pNative);
185+
186+
Variant result = Marshal.PtrToStructure<Variant>(pNative);
187+
Assert.Equal(VarEnum.VT_RECORD, (VarEnum)result.vt);
188+
Assert.NotEqual(nint.Zero, result.pRecInfo); // We should have an IRecordInfo instance.
189+
190+
var expectedBytes = new ReadOnlySpan<byte>(guid.ToByteArray());
191+
var actualBytes = new ReadOnlySpan<byte>((void*)result.bstrVal, expectedBytes.Length);
192+
Assert.Equal(expectedBytes, actualBytes);
193+
194+
object o = Marshal.GetObjectForNativeVariant(pNative);
195+
Assert.Equal(guid, o);
196+
}
197+
}
198+
finally
199+
{
200+
Marshal.FreeHGlobal(pNative);
201+
}
202+
}
203+
170204
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))]
171205
[InlineData(3.14)]
172206
public unsafe void GetNativeVariantForObject_Double_Success(double obj)

src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetObjectForNativeVariantTests.cs

+25-1
Original file line numberDiff line numberDiff line change
@@ -246,14 +246,38 @@ public void GetObjectForNativeVariant_InvalidDate_ThrowsArgumentException(double
246246
}
247247

248248
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))]
249-
public void GetObjectForNativeVariant_NoDataForRecord_ThrowsArgumentException()
249+
public void GetObjectForNativeVariant_NoRecordInfo_ThrowsArgumentException()
250250
{
251251
Variant variant = CreateVariant(VT_RECORD, new UnionTypes { _record = new Record { _recordInfo = IntPtr.Zero } });
252252
AssertExtensions.Throws<ArgumentException>(null, () => GetObjectForNativeVariant(variant));
253253
}
254254

255+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))]
256+
public void GetObjectForNativeVariant_NoRecordData_ReturnsNull()
257+
{
258+
var recordInfo = new RecordInfo();
259+
IntPtr pRecordInfo = Marshal.GetComInterfaceForObject<RecordInfo, IRecordInfo>(recordInfo);
260+
try
261+
{
262+
Variant variant = CreateVariant(VT_RECORD, new UnionTypes
263+
{
264+
_record = new Record
265+
{
266+
_record = IntPtr.Zero,
267+
_recordInfo = pRecordInfo
268+
}
269+
});
270+
Assert.Null(GetObjectForNativeVariant(variant));
271+
}
272+
finally
273+
{
274+
Marshal.Release(pRecordInfo);
275+
}
276+
}
277+
255278
public static IEnumerable<object[]> GetObjectForNativeVariant_NoSuchGuid_TestData()
256279
{
280+
yield return new object[] { typeof(object).GUID };
257281
yield return new object[] { typeof(string).GUID };
258282
yield return new object[] { Guid.Empty };
259283
}

src/tests/Interop/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ if(CLR_CMAKE_TARGET_WIN32)
8181
add_subdirectory(COM/NativeClients/DefaultInterfaces)
8282
add_subdirectory(COM/NativeClients/Dispatch)
8383
add_subdirectory(COM/NativeClients/Events)
84+
add_subdirectory(COM/NativeClients/MiscTypes)
8485
add_subdirectory(COM/ComWrappers/MockReferenceTrackerRuntime)
8586
add_subdirectory(COM/ComWrappers/WeakReference)
8687

src/tests/Interop/COM/Dynamic/BasicTest.cs

+11
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public void Run()
4343

4444
String();
4545
Date();
46+
SpecialCasedValueTypes();
4647
ComObject();
4748
Null();
4849

@@ -385,6 +386,16 @@ private void Date()
385386
Variant<DateTime>(val, expected);
386387
}
387388

389+
private void SpecialCasedValueTypes()
390+
{
391+
{
392+
var val = Guid.NewGuid();
393+
var expected = val;
394+
// Pass as variant
395+
Variant<Guid>(val, expected);
396+
}
397+
}
398+
388399
private void ComObject()
389400
{
390401
Type t = Type.GetTypeFromCLSID(Guid.Parse(ServerGuids.BasicTest));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
3+
<assemblyIdentity
4+
type="win32"
5+
name="NetClientMiscTypes"
6+
version="1.0.0.0" />
7+
8+
<dependency>
9+
<dependentAssembly>
10+
<!-- RegFree COM -->
11+
<assemblyIdentity
12+
type="win32"
13+
name="COMNativeServer.X"
14+
version="1.0.0.0"/>
15+
</dependentAssembly>
16+
</dependency>
17+
18+
</assembly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<!-- Needed for CMakeProjectReference, GC.WaitForPendingFinalizers -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
<ApplicationManifest>App.manifest</ApplicationManifest>
6+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
7+
</PropertyGroup>
8+
<ItemGroup>
9+
<Compile Include="Program.cs" />
10+
<Compile Include="../../ServerContracts/Server.CoClasses.cs" />
11+
<Compile Include="../../ServerContracts/Server.Contracts.cs" />
12+
<Compile Include="../../ServerContracts/ServerGuids.cs" />
13+
</ItemGroup>
14+
<ItemGroup>
15+
<CMakeProjectReference Include="../../NativeServer/CMakeLists.txt" />
16+
<ProjectReference Include="$(TestLibraryProjectPath)" />
17+
</ItemGroup>
18+
</Project>

0 commit comments

Comments
 (0)