Skip to content

Commit

Permalink
Call custom parameterless constructor on structs through Activator
Browse files Browse the repository at this point in the history
Fixes #6843
- Disabled caching struct types that have custom parameterless constructors in `ActivatorCache`
- Removed some things relevant to security, which don't apply to CoreCLR
  • Loading branch information
kouvel committed Apr 6, 2017
1 parent 4abb8b6 commit 7aa4f28
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 71 deletions.
13 changes: 3 additions & 10 deletions src/mscorlib/src/System/RtType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4701,15 +4701,12 @@ private class ActivatorCacheEntry
internal volatile CtorDelegate m_ctor;
internal readonly RuntimeMethodHandleInternal m_hCtorMethodHandle;
internal readonly MethodAttributes m_ctorAttributes;
// Is a security check needed before this constructor is invoked?
internal readonly bool m_bNeedSecurityCheck;
// Lazy initialization was performed
internal volatile bool m_bFullyInitialized;

internal ActivatorCacheEntry(RuntimeType t, RuntimeMethodHandleInternal rmh, bool bNeedSecurityCheck)
internal ActivatorCacheEntry(RuntimeType t, RuntimeMethodHandleInternal rmh)
{
m_type = t;
m_bNeedSecurityCheck = bNeedSecurityCheck;
m_hCtorMethodHandle = rmh;
if (!m_hCtorMethodHandle.IsNullHandle())
m_ctorAttributes = RuntimeMethodHandle.GetAttributes(m_hCtorMethodHandle);
Expand Down Expand Up @@ -4779,16 +4776,12 @@ internal void SetEntry(ActivatorCacheEntry ace)
internal Object CreateInstanceSlow(bool publicOnly, bool skipCheckThis, bool fillCache, ref StackCrawlMark stackMark)
{
RuntimeMethodHandleInternal runtime_ctor = default(RuntimeMethodHandleInternal);
bool bNeedSecurityCheck = true;
bool bCanBeCached = false;
bool bSecurityCheckOff;

if (!skipCheckThis)
CreateInstanceCheckThis();

bSecurityCheckOff = true; // CoreCLR does not use security at all.

Object instance = RuntimeTypeHandle.CreateInstance(this, publicOnly, bSecurityCheckOff, ref bCanBeCached, ref runtime_ctor, ref bNeedSecurityCheck);
Object instance = RuntimeTypeHandle.CreateInstance(this, publicOnly, ref bCanBeCached, ref runtime_ctor);

if (bCanBeCached && fillCache)
{
Expand All @@ -4801,7 +4794,7 @@ internal Object CreateInstanceSlow(bool publicOnly, bool skipCheckThis, bool fil
}

// cache the ctor
ActivatorCacheEntry ace = new ActivatorCacheEntry(this, runtime_ctor, bNeedSecurityCheck);
ActivatorCacheEntry ace = new ActivatorCacheEntry(this, runtime_ctor);
activatorCache.SetEntry(ace);
}

Expand Down
2 changes: 1 addition & 1 deletion src/mscorlib/src/System/RuntimeHandles.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ internal static IntPtr[] CopyRuntimeTypeHandles(Type[] inHandles, out int length
}

[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal static extern Object CreateInstance(RuntimeType type, bool publicOnly, bool noCheck, ref bool canBeCached, ref RuntimeMethodHandleInternal ctor, ref bool bNeedSecurityCheck);
internal static extern Object CreateInstance(RuntimeType type, bool publicOnly, ref bool canBeCached, ref RuntimeMethodHandleInternal ctor);

[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal static extern Object CreateCaInstance(RuntimeType type, IRuntimeMethodInfo ctor);
Expand Down
64 changes: 8 additions & 56 deletions src/vm/reflectioninvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,21 +470,17 @@ FCIMPL1(Object*, RuntimeTypeHandle::Allocate, ReflectClassBaseObject* pTypeUNSAF
}//Allocate
FCIMPLEND

FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refThisUNSAFE,
FCIMPL4(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refThisUNSAFE,
CLR_BOOL publicOnly,
CLR_BOOL securityOff,
CLR_BOOL* pbCanBeCached,
MethodDesc** pConstructor,
CLR_BOOL *pbNeedSecurityCheck) {
MethodDesc** pConstructor) {
CONTRACTL {
FCALL_CHECK;
PRECONDITION(CheckPointer(refThisUNSAFE));
PRECONDITION(CheckPointer(pbCanBeCached));
PRECONDITION(CheckPointer(pbNeedSecurityCheck));
PRECONDITION(CheckPointer(pConstructor));
PRECONDITION(*pbCanBeCached == false);
PRECONDITION(*pConstructor == NULL);
PRECONDITION(*pbNeedSecurityCheck == true);
}
CONTRACTL_END;

Expand All @@ -508,7 +504,6 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
HELPER_METHOD_FRAME_BEGIN_RET_2(rv, refThis);

MethodTable* pVMT;
bool bNeedAccessCheck;

// Get the type information associated with refThis
if (thisTH.IsNull() || thisTH.IsTypeDesc())
Expand All @@ -518,8 +513,6 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT

pVMT->EnsureInstanceActive();

bNeedAccessCheck = false;

#ifdef FEATURE_COMINTEROP
// If this is __ComObject then create the underlying COM object.
if (IsComObjectClass(refThis->GetType())) {
Expand Down Expand Up @@ -577,34 +570,16 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
COMPlusThrow(kMissingMethodException,W("Arg_NoDefCTor"));
}

if (!securityOff)
{
{
// Public critical types cannot be accessed by transparent callers
bNeedAccessCheck = !pVMT->IsExternallyVisible() || Security::TypeRequiresTransparencyCheck(pVMT);
}

if (bNeedAccessCheck)
{
RefSecContext sCtx(InvokeUtil::GetInvocationAccessCheckType());
InvokeUtil::CanAccessClass(&sCtx, pVMT, TRUE);
}
}

// Handle the nullable<T> special case
// Handle the nullable<T> special case
if (Nullable::IsNullableType(thisTH)) {
rv = Nullable::BoxedNullableNull(thisTH);
}
else
rv = pVMT->Allocate();

// Since no security checks will be performed on cached value types without default ctors,
// we cannot cache those types that require access checks.
// In fact, we don't even need to set pbNeedSecurityCheck to false here.
if (!pVMT->Collectible() && !bNeedAccessCheck)
if (!pVMT->Collectible())
{
*pbCanBeCached = true;
*pbNeedSecurityCheck = false;
}
}
else // !pVMT->HasDefaultConstructor()
Expand All @@ -617,30 +592,6 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
if (!IsMdPublic(attr) && publicOnly)
COMPlusThrow(kMissingMethodException,W("Arg_NoDefCTor"));

if (!securityOff)
{
// If the type is critical or the constructor we're using is critical, we need to ensure that
// the caller is allowed to invoke it.
bool needsTransparencyCheck = Security::TypeRequiresTransparencyCheck(pVMT) ||
(Security::IsMethodCritical(pMeth) && !Security::IsMethodSafeCritical(pMeth));

// We also need to do a check if the method or type is not public
bool needsVisibilityCheck = !IsMdPublic(attr) || !pVMT->IsExternallyVisible();

// If the visiblity, transparency, or legacy LinkDemands on the type or constructor dictate that
// we need to check the caller, then do that now.
bNeedAccessCheck = needsTransparencyCheck ||
needsVisibilityCheck ||
pMeth->RequiresLinktimeCheck();

if (bNeedAccessCheck)
{
// this security context will be used in cast checking as well
RefSecContext sCtx(InvokeUtil::GetInvocationAccessCheckType());
InvokeUtil::CanAccessMethod(pMeth, pVMT, NULL, &sCtx);
}
}

// We've got the class, lets allocate it and call the constructor
OBJECTREF o;
bool remoting = false;
Expand All @@ -663,12 +614,13 @@ FCIMPL6(Object*, RuntimeTypeHandle::CreateInstance, ReflectClassBaseObject* refT
rv = o;
GCPROTECT_END();

// No need to set these if they cannot be cached
if (!remoting && !pVMT->Collectible())
// No need to set these if they cannot be cached. In particular, if the type is a value type with a custom
// parameterless constructor, don't allow caching and have subsequent calls come back here to allocate an object and
// call the constructor.
if (!remoting && !pVMT->Collectible() && !pVMT->IsValueType())
{
*pbCanBeCached = true;
*pConstructor = pMeth;
*pbNeedSecurityCheck = bNeedAccessCheck;
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/vm/runtimehandles.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,10 @@ class RuntimeTypeHandle {

// Static method on RuntimeTypeHandle
static FCDECL1(Object*, Allocate, ReflectClassBaseObject *refType) ; //A.CI work
static FCDECL6(Object*, CreateInstance, ReflectClassBaseObject* refThisUNSAFE,
static FCDECL4(Object*, CreateInstance, ReflectClassBaseObject* refThisUNSAFE,
CLR_BOOL publicOnly,
CLR_BOOL securityOff,
CLR_BOOL *pbCanBeCached,
MethodDesc** pConstructor,
CLR_BOOL *pbNeedSecurityCheck);
MethodDesc** pConstructor);

static FCDECL2(Object*, CreateCaInstance, ReflectClassBaseObject* refCaType, ReflectMethodObject* pCtorUNSAFE);

Expand Down
107 changes: 107 additions & 0 deletions tests/src/reflection/ActivateStructDefCtor/ActivateStructDefCtor.il
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
.assembly extern mscorlib
{
.publickeytoken = (B7 7A 5C 56 19 34 E0 89 )
.ver 2:0:0:0
}
.assembly ActivateStructDefCtor
{
}

// =============== CLASS MEMBERS DECLARATION ===================

.class private abstract auto ansi sealed beforefieldinit Program
extends [System.Runtime]System.Object
{
.class sequential ansi sealed nested private beforefieldinit Foo
extends [System.Runtime]System.ValueType
{
.field public int32 x

.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 8 (0x8)
.maxstack 8
IL_0000: ldarg.0
IL_0001: ldc.i4.1
IL_0002: stfld int32 Program/Foo::x
IL_0007: ret
} // end of method Foo::.ctor
} // end of class Foo

.field private static int32 Pass
.field private static int32 Fail

.method public hidebysig static int32 Main() cil managed
{
.entrypoint
// Code size 89 (0x59)
.maxstack 2
.locals init ([0] int32 testIndex,
[1] int32 i,
[2] valuetype Program/Foo foo)
IL_0000: ldc.i4.m1
IL_0001: stloc.0
IL_0002: ldc.i4.0
IL_0003: stloc.1
IL_0004: br.s IL_004f

IL_0006: ldloc.0
IL_0007: ldc.i4.1
IL_0008: add
IL_0009: stloc.0
IL_000a: call !!0 [System.Runtime]System.Activator::CreateInstance<valuetype Program/Foo>()
IL_000f: stloc.2
IL_0010: ldloc.2
IL_0011: ldfld int32 Program/Foo::x
IL_0016: ldc.i4.1
IL_0017: beq.s IL_0021

IL_0019: ldsfld int32 Program::Fail
IL_001e: ldloc.0
IL_001f: add
IL_0020: ret

IL_0021: ldloc.0
IL_0022: ldc.i4.1
IL_0023: add
IL_0024: stloc.0
IL_0025: ldtoken Program/Foo
IL_002a: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_002f: call object [System.Runtime]System.Activator::CreateInstance(class [System.Runtime]System.Type)
IL_0034: unbox.any Program/Foo
IL_0039: stloc.2
IL_003a: ldloc.2
IL_003b: ldfld int32 Program/Foo::x
IL_0040: ldc.i4.1
IL_0041: beq.s IL_004b

IL_0043: ldsfld int32 Program::Fail
IL_0048: ldloc.0
IL_0049: add
IL_004a: ret

IL_004b: ldloc.1
IL_004c: ldc.i4.1
IL_004d: add
IL_004e: stloc.1
IL_004f: ldloc.1
IL_0050: ldc.i4.2
IL_0051: blt.s IL_0006

IL_0053: ldsfld int32 Program::Pass
IL_0058: ret
} // end of method Program::Main

.method private hidebysig specialname rtspecialname static
void .cctor() cil managed
{
// Code size 15 (0xf)
.maxstack 8
IL_0000: ldc.i4.s 100
IL_0002: stsfld int32 Program::Pass
IL_0007: ldc.i4.s 101
IL_0009: stsfld int32 Program::Fail
IL_000e: ret
} // end of method Program::.cctor
} // end of class Program
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
<ProjectGuid>{F8D9C0E2-F86F-493F-8D18-0D19ADF6E3FD}</ProjectGuid>
<OutputType>Library</OutputType>
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
</PropertyGroup>
<!-- Default configurations to help VS understand the configurations -->
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "/>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "/>
<ItemGroup>
<Compile Include="ActivateStructDefCtor.il" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
</PropertyGroup>
</Project>

0 comments on commit 7aa4f28

Please sign in to comment.