Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable array support for the COM variant wrapper classes when built-in COM is disabled. #55756

Merged
merged 10 commits into from
Jul 18, 2021
Merged
4 changes: 4 additions & 0 deletions eng/testing/linker/project.csproj.template
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
<_ExtraTrimmerArgs>{ExtraTrimmerArgs} $(_ExtraTrimmerArgs)</_ExtraTrimmerArgs>
</PropertyGroup>

<ItemGroup>
{RuntimeHostConfigurationOptions}
</ItemGroup>

<ItemGroup>
{AdditionalProjectReferences}
</ItemGroup>
Expand Down
10 changes: 10 additions & 0 deletions eng/testing/linker/trimmingTests.targets
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@
<_additionalProjectSourceFiles Include="%(TestConsoleApps.AdditionalSourceFiles)" />
</ItemGroup>

<ItemGroup>
<_switchesAsItems Include="%(TestConsoleApps.DisabledFeatureSwitches)" Value="false" />
<_switchesAsItems Include="%(TestConsoleApps.EnabledFeatureSwitches)" Value="true" />
</ItemGroup>

<PropertyGroup>
<_runtimeHostConfigurationOptionsString>@(_switchesAsItems->'&lt;RuntimeHostConfigurationOption Include=&quot;%(Identity)&quot; Value=&quot;%(Value)&quot; Trim=&quot;true&quot; /&gt;', '%0a ')</_runtimeHostConfigurationOptionsString>
</PropertyGroup>

<MakeDir Directories="$(_projectDir)" />
<WriteLinesToFile File="$(_projectFile)"
Lines="$([System.IO.File]::ReadAllText('$(ProjectTemplate)')
Expand All @@ -79,6 +88,7 @@
.Replace('{UseMonoRuntime}','$(UseMonoRuntime)')
.Replace('{MicrosoftNETILLinkTasksVersion}', '$(MicrosoftNETILLinkTasksVersion)')
.Replace('{ExtraTrimmerArgs}', '%(TestConsoleApps.ExtraTrimmerArgs)')
.Replace('{RuntimeHostConfigurationOptions}', '$(_runtimeHostConfigurationOptionsString)')
.Replace('{AdditionalProjectReferences}', '$(_additionalProjectReferencesString)')
.Replace('{RepositoryEngineeringDir}', '$(RepositoryEngineeringDir)')
.Replace('{MonoAOTCompilerDir}', '$(MonoAOTCompilerDir)')
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/mlinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3863,7 +3863,7 @@ void ArrayMarshalInfo::InitElementInfo(CorNativeType arrayNativeType, MarshalInf
}
}
#ifdef FEATURE_COMINTEROP
else if (m_thElement == TypeHandle(CoreLibBinder::GetClass(CLASS__ERROR_WRAPPER)))
else if (g_pConfig->IsBuiltInCOMSupported() && m_thElement == TypeHandle(CoreLibBinder::GetClass(CLASS__ERROR_WRAPPER)))
{
m_vtElement = VT_ERROR;
}
Expand Down
56 changes: 37 additions & 19 deletions src/coreclr/vm/olevariant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,20 +329,24 @@ VARTYPE OleVariant::GetVarTypeForTypeHandle(TypeHandle type)
#endif

#ifdef FEATURE_COMINTEROP
if (CoreLibBinder::IsClass(pMT, CLASS__DISPATCH_WRAPPER))
return VT_DISPATCH;
if (CoreLibBinder::IsClass(pMT, CLASS__UNKNOWN_WRAPPER))
return VT_UNKNOWN;
if (CoreLibBinder::IsClass(pMT, CLASS__ERROR_WRAPPER))
return VT_ERROR;
if (CoreLibBinder::IsClass(pMT, CLASS__CURRENCY_WRAPPER))
return VT_CY;
if (CoreLibBinder::IsClass(pMT, CLASS__BSTR_WRAPPER))
return VT_BSTR;
// The wrapper types are only available when built-in COM is supported.
if (g_pConfig->IsBuiltInCOMSupported())
{
if (CoreLibBinder::IsClass(pMT, CLASS__DISPATCH_WRAPPER))
return VT_DISPATCH;
if (CoreLibBinder::IsClass(pMT, CLASS__UNKNOWN_WRAPPER))
return VT_UNKNOWN;
if (CoreLibBinder::IsClass(pMT, CLASS__ERROR_WRAPPER))
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
return VT_ERROR;
if (CoreLibBinder::IsClass(pMT, CLASS__CURRENCY_WRAPPER))
return VT_CY;
if (CoreLibBinder::IsClass(pMT, CLASS__BSTR_WRAPPER))
return VT_BSTR;

// VariantWrappers cannot be stored in VARIANT's.
if (CoreLibBinder::IsClass(pMT, CLASS__VARIANT_WRAPPER))
COMPlusThrow(kArgumentException, IDS_EE_COM_UNSUPPORTED_SIG);
// VariantWrappers cannot be stored in VARIANT's.
if (CoreLibBinder::IsClass(pMT, CLASS__VARIANT_WRAPPER))
COMPlusThrow(kArgumentException, IDS_EE_COM_UNSUPPORTED_SIG);
}
#endif // FEATURE_COMINTEROP

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a more surgical fix than I was thinking and thanks for doing this!

I assume the code under the other #FEATURE_COMINTEROP in this method will not cause a problem?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other ones won't cause problems.

We'll always have SafeHandle and CriticalHandle (which arrays of are always unsupported in interop).

And then the rest is based on RCW/CCW support having been used to create a wrapper, which is already blocked previously.

if (pMT->IsEnum())
Expand Down Expand Up @@ -4847,7 +4851,7 @@ void OleVariant::TransposeArrayData(BYTE *pDestData, BYTE *pSrcData, SIZE_T dwNu
}
}

BOOL OleVariant::IsArrayOfWrappers(BASEARRAYREF *pArray, BOOL *pbOfInterfaceWrappers)
BOOL OleVariant::IsArrayOfWrappers(_In_ BASEARRAYREF *pArray, _Out_opt_ BOOL *pbOfInterfaceWrappers)
{
CONTRACTL
{
Expand All @@ -4857,27 +4861,40 @@ BOOL OleVariant::IsArrayOfWrappers(BASEARRAYREF *pArray, BOOL *pbOfInterfaceWrap
}
CONTRACTL_END;

if (!g_pConfig->IsBuiltInCOMSupported())
{
return FALSE;
}

TypeHandle hndElemType = (*pArray)->GetArrayElementTypeHandle();

if (!hndElemType.IsTypeDesc())
{
if (hndElemType == TypeHandle(CoreLibBinder::GetClass(CLASS__DISPATCH_WRAPPER)) ||
hndElemType == TypeHandle(CoreLibBinder::GetClass(CLASS__UNKNOWN_WRAPPER)))
{
*pbOfInterfaceWrappers = TRUE;
if (pbOfInterfaceWrappers)
{
*pbOfInterfaceWrappers = TRUE;
}
return TRUE;
}

if (hndElemType == TypeHandle(CoreLibBinder::GetClass(CLASS__ERROR_WRAPPER)) ||
hndElemType == TypeHandle(CoreLibBinder::GetClass(CLASS__CURRENCY_WRAPPER)) ||
hndElemType == TypeHandle(CoreLibBinder::GetClass(CLASS__BSTR_WRAPPER)))
{
*pbOfInterfaceWrappers = FALSE;
if (pbOfInterfaceWrappers)
{
*pbOfInterfaceWrappers = FALSE;
}
return TRUE;
}
}

*pbOfInterfaceWrappers = FALSE;
if (pbOfInterfaceWrappers)
*pbOfInterfaceWrappers = FALSE;

return FALSE;
}

Expand All @@ -4889,6 +4906,7 @@ BASEARRAYREF OleVariant::ExtractWrappedObjectsFromArray(BASEARRAYREF *pArray)
GC_TRIGGERS;
MODE_COOPERATIVE;
PRECONDITION(CheckPointer(pArray));
PRECONDITION(IsArrayOfWrappers(pArray, NULL));
}
CONTRACTL_END;

Expand Down Expand Up @@ -5022,6 +5040,7 @@ TypeHandle OleVariant::GetWrappedArrayElementType(BASEARRAYREF *pArray)
GC_TRIGGERS;
MODE_COOPERATIVE;
PRECONDITION(CheckPointer(pArray));
PRECONDITION(IsArrayOfWrappers(pArray, NULL));
}
CONTRACTL_END;

Expand Down Expand Up @@ -5067,8 +5086,7 @@ TypeHandle OleVariant::GetArrayElementTypeWrapperAware(BASEARRAYREF *pArray)
}
CONTRACTL_END;

BOOL bArrayOfInterfaceWrappers;
if (IsArrayOfWrappers(pArray, &bArrayOfInterfaceWrappers))
if (IsArrayOfWrappers(pArray, nullptr))
{
return GetWrappedArrayElementType(pArray);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
as this test will ensure exceptions are using Resource keys -->
<ExtraTrimmerArgs>--feature System.Resources.UseSystemResourceKeys true</ExtraTrimmerArgs>
</TestConsoleAppSourceFiles>
<TestConsoleAppSourceFiles Include="TypeBuilderComDisabled.cs">
<DisabledFeatureSwitches>System.Runtime.InteropServices.BuiltInComInterop.IsSupported</DisabledFeatureSwitches>
</TestConsoleAppSourceFiles>
</ItemGroup>

<Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.targets))" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
elinor-fung marked this conversation as resolved.
Show resolved Hide resolved
using System.Reflection;
using System.Reflection.Emit;

AssemblyBuilder assembly = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("GeneratedAssembly"), AssemblyBuilderAccess.Run);
ModuleBuilder module = assembly.DefineDynamicModule("GeneratedModule");

string typeName = "GeneratedType";
TypeBuilder genericType = module.DefineType(typeName);
genericType.DefineField("_int", typeof(int), FieldAttributes.Private);
genericType.DefineProperty("Prop", PropertyAttributes.None, typeof(string), null);

Type generatedType = genericType.CreateType();
eerhardt marked this conversation as resolved.
Show resolved Hide resolved
if (generatedType.Name != typeName)
{
Console.WriteLine($"Unexpected name for generated type. Expected: {typeName}, Actual: {generatedType.Name}");
return -1;
}

object obj = Activator.CreateInstance(generatedType);
string objAsString = obj.ToString();
if (objAsString != typeName)
{
Console.WriteLine($"Unexpected result of ToString() for instance of generated type. Expected: {typeName}, Actual: {objAsString}");
return -2;
}

return 100;