Skip to content

Commit

Permalink
Use similar types for self-referential generics instead of the exact …
Browse files Browse the repository at this point in the history
…canonical type (#83995)

- Take advantage of work done a few years ago to simplify the interaction with the interop subsystem
- In the problematic case, simulate loads with two different types as instantiations which are unrelated in field layout, and see if they match up. Only enable this code in a few very small isolated parts of the runtime
- Filter more of the type loader logic through the byvalue class cache which should improve performance a bit
- Similarly when considering blittability, tweak the logic to use the special load path

- Support for self-recursive generics is not enabled for static fields, as that requires a somewhat different tweak, and there is less apparent demand. (For that scenario self-referential generics really should support having fields of type T.)
- Support for indirect self-recursive generics is also not enabled. The approach taken here is not practical for that, and there does not appear to be significant demand for that either.

Fixes #6924
  • Loading branch information
davidwrighton authored Apr 4, 2023
1 parent 735ddea commit bc887b3
Show file tree
Hide file tree
Showing 17 changed files with 546 additions and 179 deletions.
4 changes: 3 additions & 1 deletion src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4519,7 +4519,9 @@ void DacDbiInterfaceImpl::EnumerateModulesInAssembly(
// Debugger isn't notified of Resource / Inspection-only modules.
if (pDomainAssembly->GetModule()->IsVisibleToDebugger())
{
_ASSERTE(pDomainAssembly->IsLoaded());
// If domain assembly isn't yet loaded, just return
if (!pDomainAssembly->IsLoaded())
return;

VMPTR_DomainAssembly vmDomainAssembly = VMPTR_DomainAssembly::NullPtr();
vmDomainAssembly.SetHostPtr(pDomainAssembly);
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ BEGIN
IDS_CLASSLOAD_INLINE_ARRAY_LENGTH "InlineArrayAttribute requires that the length argument is greater than 0. Type: '%1'. Assembly: '%2'."
IDS_CLASSLOAD_INLINE_ARRAY_EXPLICIT "InlineArrayAttribute cannot be applied to a type with explicit layout. Type: '%1'. Assembly: '%2'."

IDS_INVALID_RECURSIVE_GENERIC_FIELD_LOAD "Could not load type '%1' from assembly '%2' because of an invalid self-referential generic field."

#if FEATURE_COMINTEROP
IDS_EE_CANNOTCAST_NOMARSHAL "The Windows Runtime Object can only be used in the threading context where it was created, because it implements INoMarshal or has MarshalingBehaviorAttribute(MarshalingType.None) set."
#endif
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscorrc/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
#define IDS_CLASSLOAD_MI_BADRETURNTYPE 0x17a8
#define IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL 0x17a9

#define IDS_INVALID_RECURSIVE_GENERIC_FIELD_LOAD 0x17aa
#define IDS_CLASSLOAD_TOOMANYGENERICARGS 0x17ab

#define IDS_CLASSLOAD_INLINE_ARRAY_FIELD_COUNT 0x17ac
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/vm/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -2230,6 +2230,11 @@ class ApproxFieldDescIterator
SUPPORTS_DAC;
return m_totalFields - m_currField - 1;
}
int GetValueClassCacheIndex()
{
LIMITED_METHOD_CONTRACT;
return m_currField;
}
};

//
Expand Down
39 changes: 34 additions & 5 deletions src/coreclr/vm/classlayoutinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ namespace
IMDInternalImport* pInternalImport,
HENUMInternal* phEnumField,
Module* pModule,
mdTypeDef cl,
ParseNativeTypeFlags nativeTypeFlags,
const SigTypeContext* pTypeContext,
BOOL* fDisqualifyFromManagedSequential,
Expand All @@ -477,6 +478,8 @@ namespace
#endif
)
{
STANDARD_VM_CONTRACT;

HRESULT hr;
mdFieldDef fd;
ULONG maxRid = pInternalImport->GetCountWithTokenKind(mdtFieldDef);
Expand Down Expand Up @@ -533,20 +536,45 @@ namespace
}
#endif
MetaSig fsig(pCOMSignature, cbCOMSignature, pModule, pTypeContext, MetaSig::sigField);
CorElementType corElemType = fsig.NextArgNormalized();
CorElementType corElemType = fsig.NextArg();

TypeHandle typeHandleMaybe;
if (corElemType == ELEMENT_TYPE_VALUETYPE) // Only look up the next element in the signature if it is a value type to avoid causing recursive type loads in valid scenarios.
{
typeHandleMaybe = fsig.GetLastTypeHandleThrowing(ClassLoader::LoadTypes,
CLASS_LOAD_APPROXPARENTS,
TRUE);
SigPointer::HandleRecursiveGenericsForFieldLayoutLoad recursiveControl;
recursiveControl.pModuleWithTokenToAvoidIfPossible = pModule;
recursiveControl.tkTypeDefToAvoidIfPossible = cl;
typeHandleMaybe = fsig.GetArgProps().GetTypeHandleThrowing(pModule,
pTypeContext,
ClassLoader::LoadTypes,
CLASS_LOAD_APPROXPARENTS,
TRUE, NULL, NULL, NULL,
&recursiveControl);

if (typeHandleMaybe.IsNull())
{
// Everett C++ compiler can generate a TypeRef with RS=0
// without respective TypeDef for unmanaged valuetypes,
// referenced only by pointers to them.
// In such case, GetTypeHandleThrowing returns null handle,
// and we return E_T_VOID
typeHandleMaybe = TypeHandle(CoreLibBinder::GetElementType(ELEMENT_TYPE_VOID));
}
corElemType = typeHandleMaybe.AsMethodTable()->GetInternalCorElementType();
if (corElemType != ELEMENT_TYPE_VALUETYPE)
typeHandleMaybe = TypeHandle();
}
else if (corElemType == ELEMENT_TYPE_TYPEDBYREF)
{
typeHandleMaybe = TypeHandle(g_TypedReferenceMT);
}

pFieldInfoArrayOut->m_placement = GetFieldPlacementInfo(corElemType, typeHandleMaybe);
*fDisqualifyFromManagedSequential |= TypeHasGCPointers(corElemType, typeHandleMaybe);
*fHasAutoLayoutField |= TypeHasAutoLayoutField(corElemType, typeHandleMaybe);
*fHasInt128Field |= TypeHasInt128Field(corElemType, typeHandleMaybe);

if (!IsFieldBlittable(pModule, fd, fsig.GetArgProps(), pTypeContext, nativeTypeFlags))
if (!IsFieldBlittable(pModule, fd, corElemType, typeHandleMaybe, nativeTypeFlags))
*pIsBlittableOut = FALSE;

(*cInstanceFields)++;
Expand Down Expand Up @@ -705,6 +733,7 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing(
pInternalImport,
phEnumField,
pModule,
cl,
nativeTypeFlags,
pTypeContext,
&fDisqualifyFromManagedSequential,
Expand Down
26 changes: 26 additions & 0 deletions src/coreclr/vm/field.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,32 @@ UINT FieldDesc::LoadSize()
return size;
}

UINT FieldDesc::GetSize(MethodTable *pMTOfValueTypeField)
{
CONTRACTL
{
INSTANCE_CHECK;
NOTHROW;
GC_NOTRIGGER;
MODE_ANY;
FORBID_FAULT;
}
CONTRACTL_END

CorElementType type = GetFieldType();
UINT size = GetSizeForCorElementType(type);
if (size == (UINT) -1)
{
LOG((LF_CLASSLOADER, LL_INFO10000, "FieldDesc::GetSize %s::%s\n", GetApproxEnclosingMethodTable()->GetDebugClassName(), m_debugName));
CONSISTENCY_CHECK(GetFieldType() == ELEMENT_TYPE_VALUETYPE);
TypeHandle t = (pMTOfValueTypeField != NULL) ? TypeHandle(pMTOfValueTypeField) : LookupApproxFieldTypeHandle();
_ASSERTE(!t.IsNull());
size = t.GetMethodTable()->GetNumInstanceFieldBytes();
}

return size;
}

UINT FieldDesc::GetSize()
{
CONTRACTL
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ class FieldDesc
// Return -1 if the type isn't loaded yet (i.e. if LookupFieldTypeHandle() would return null)
UINT GetSize();

// If the field is a valuetype, then either pMTOfValueTypeField must not be NULL or LookupFieldTypeHandle() must not return null
UINT GetSize(MethodTable *pMTOfValueTypeField);

// These routines encapsulate the operation of getting and setting
// fields.
void GetInstanceField(OBJECTREF o, VOID * pOutVal);
Expand Down
122 changes: 56 additions & 66 deletions src/coreclr/vm/fieldmarshaler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,13 @@ VOID ParseNativeType(Module* pModule,
bool IsFieldBlittable(
Module* pModule,
mdFieldDef fd,
SigPointer fieldSig,
const SigTypeContext* pTypeContext,
CorElementType corElemType,
TypeHandle valueTypeHandle,
ParseNativeTypeFlags flags
)
{
STANDARD_VM_CONTRACT;

PCCOR_SIGNATURE marshalInfoSig;
ULONG marshalInfoSigLength;

Expand All @@ -218,75 +220,63 @@ bool IsFieldBlittable(

bool isBlittable = false;

EX_TRY
switch (corElemType)
{
TypeHandle valueTypeHandle;
CorElementType corElemType = fieldSig.PeekElemTypeNormalized(pModule, pTypeContext, &valueTypeHandle);

switch (corElemType)
case ELEMENT_TYPE_CHAR:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT && flags != ParseNativeTypeFlags::IsAnsi) || (nativeType == NATIVE_TYPE_I2) || (nativeType == NATIVE_TYPE_U2);
break;
case ELEMENT_TYPE_I1:
case ELEMENT_TYPE_U1:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I1) || (nativeType == NATIVE_TYPE_U1);
break;
case ELEMENT_TYPE_I2:
case ELEMENT_TYPE_U2:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I2) || (nativeType == NATIVE_TYPE_U2);
break;
case ELEMENT_TYPE_I4:
case ELEMENT_TYPE_U4:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I4) || (nativeType == NATIVE_TYPE_U4) || (nativeType == NATIVE_TYPE_ERROR);
break;
case ELEMENT_TYPE_I8:
case ELEMENT_TYPE_U8:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I8) || (nativeType == NATIVE_TYPE_U8);
break;
case ELEMENT_TYPE_R4:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_R4);
break;
case ELEMENT_TYPE_R8:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_R8);
break;
case ELEMENT_TYPE_I:
case ELEMENT_TYPE_U:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_INT) || (nativeType == NATIVE_TYPE_UINT);
break;
case ELEMENT_TYPE_PTR:
isBlittable = nativeType == NATIVE_TYPE_DEFAULT;
break;
case ELEMENT_TYPE_FNPTR:
isBlittable = nativeType == NATIVE_TYPE_DEFAULT || nativeType == NATIVE_TYPE_FUNC;
break;
case ELEMENT_TYPE_VALUETYPE:
if (nativeType != NATIVE_TYPE_DEFAULT && nativeType != NATIVE_TYPE_STRUCT)
{
case ELEMENT_TYPE_CHAR:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT && flags != ParseNativeTypeFlags::IsAnsi) || (nativeType == NATIVE_TYPE_I2) || (nativeType == NATIVE_TYPE_U2);
break;
case ELEMENT_TYPE_I1:
case ELEMENT_TYPE_U1:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I1) || (nativeType == NATIVE_TYPE_U1);
break;
case ELEMENT_TYPE_I2:
case ELEMENT_TYPE_U2:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I2) || (nativeType == NATIVE_TYPE_U2);
break;
case ELEMENT_TYPE_I4:
case ELEMENT_TYPE_U4:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I4) || (nativeType == NATIVE_TYPE_U4) || (nativeType == NATIVE_TYPE_ERROR);
break;
case ELEMENT_TYPE_I8:
case ELEMENT_TYPE_U8:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_I8) || (nativeType == NATIVE_TYPE_U8);
break;
case ELEMENT_TYPE_R4:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_R4);
break;
case ELEMENT_TYPE_R8:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_R8);
break;
case ELEMENT_TYPE_I:
case ELEMENT_TYPE_U:
isBlittable = (nativeType == NATIVE_TYPE_DEFAULT) || (nativeType == NATIVE_TYPE_INT) || (nativeType == NATIVE_TYPE_UINT);
break;
case ELEMENT_TYPE_PTR:
isBlittable = nativeType == NATIVE_TYPE_DEFAULT;
break;
case ELEMENT_TYPE_FNPTR:
isBlittable = nativeType == NATIVE_TYPE_DEFAULT || nativeType == NATIVE_TYPE_FUNC;
break;
case ELEMENT_TYPE_VALUETYPE:
if (nativeType != NATIVE_TYPE_DEFAULT && nativeType != NATIVE_TYPE_STRUCT)
{
isBlittable = false;
}
else if (valueTypeHandle.GetMethodTable() == CoreLibBinder::GetClass(CLASS__DECIMAL))
{
// The alignment requirements of the managed System.Decimal type do not match the native DECIMAL type.
// As a result, a field of type System.Decimal can't be blittable.
isBlittable = false;
}
else
{
isBlittable = valueTypeHandle.GetMethodTable()->IsBlittable();
}
break;
default:
isBlittable = false;
break;
}
else if (valueTypeHandle.GetMethodTable() == CoreLibBinder::GetClass(CLASS__DECIMAL))
{
// The alignment requirements of the managed System.Decimal type do not match the native DECIMAL type.
// As a result, a field of type System.Decimal can't be blittable.
isBlittable = false;
}
else
{
isBlittable = valueTypeHandle.GetMethodTable()->IsBlittable();
}
break;
default:
isBlittable = false;
break;
}
EX_CATCH
{
// We were unable to determine the native type, likely because there is a mutually recursive type reference
// in this field's type. A mutually recursive object would never be blittable, so we don't need to do anything.
}
EX_END_CATCH(RethrowTerminalExceptions);
return isBlittable;
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/fieldmarshaler.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ BOOL IsStructMarshalable(TypeHandle th);
bool IsFieldBlittable(
Module* pModule,
mdFieldDef fd,
SigPointer fieldSig,
const SigTypeContext* pTypeContext,
CorElementType corElemType,
TypeHandle valueTypeHandle,
ParseNativeTypeFlags flags
);

Expand Down
Loading

0 comments on commit bc887b3

Please sign in to comment.