From 6c12105bb8cc1821ba5d5c3d36aad609a44308e0 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 5 Jan 2018 14:44:18 -0800 Subject: [PATCH] Detect ByRefLike types using attribute (#15745) * Detect ByRefLike types using attribute and improve error messages for their invalid use Fixes #11371 and #15458 --- src/dlls/mscorrc/mscorrc.rc | 5 +- src/dlls/mscorrc/resource.h | 1 + src/mscorlib/src/System/ByReference.cs | 2 +- src/vm/classnames.h | 1 + src/vm/jitinterface.cpp | 9 +- src/vm/methodtablebuilder.cpp | 50 ++---- src/vm/methodtablebuilder.h | 2 - .../GitHub_11371/Negative_ByRefLikeType.il | 186 +++++++++++++++++++++ .../GitHub_11371/Negative_ByRefLikeType.ilproj | 23 +++ 9 files changed, 234 insertions(+), 45 deletions(-) create mode 100644 tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.il create mode 100644 tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.ilproj diff --git a/src/dlls/mscorrc/mscorrc.rc b/src/dlls/mscorrc/mscorrc.rc index bbf5a29..1f2423b 100644 --- a/src/dlls/mscorrc/mscorrc.rc +++ b/src/dlls/mscorrc/mscorrc.rc @@ -1223,8 +1223,9 @@ BEGIN IDS_CLASSLOAD_NOTINTERFACE "Could not load type '%1' from assembly '%2' because it attempts to implement a class as an interface." IDS_CLASSLOAD_VALUEINSTANCEFIELD "Could not load the value type '%1' from assembly '%2' because it has an instance field of itself." - IDS_CLASSLOAD_BYREFLIKE_STATICFIELD "A value type containing a by-ref instance field, such as Span, cannot be used as the type for a static field." - IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD "A value type containing a by-ref instance field, such as Span, cannot be used as the type for a class instance field." + IDS_CLASSLOAD_BYREFLIKE_STATICFIELD "A value type containing a ByRef-like instance field cannot be used as the type for a static field." + IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD "A value type containing a ByRef-like instance field cannot be used as the type for a class instance field." + IDS_CLASSLOAD_NOTBYREFLIKE "A value type containing a ByRef-like instance field must be ByRef-like type." IDS_CLASSLOAD_BAD_NAME "Type name '%1' from assembly '%2' is invalid." IDS_CLASSLOAD_RANK_TOOLARGE "'%1' from assembly '%2' has too many dimensions." diff --git a/src/dlls/mscorrc/resource.h b/src/dlls/mscorrc/resource.h index 053b1b4..45ddd2e 100644 --- a/src/dlls/mscorrc/resource.h +++ b/src/dlls/mscorrc/resource.h @@ -893,3 +893,4 @@ #define IDS_CLASSLOAD_BYREFLIKE_STATICFIELD 0x263b #define IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD 0x263c +#define IDS_CLASSLOAD_NOTBYREFLIKE 0x263d diff --git a/src/mscorlib/src/System/ByReference.cs b/src/mscorlib/src/System/ByReference.cs index 8079605..3e1fa8a 100644 --- a/src/mscorlib/src/System/ByReference.cs +++ b/src/mscorlib/src/System/ByReference.cs @@ -11,7 +11,7 @@ namespace System // around lack of first class support for byref fields in C# and IL. The JIT and // type loader has special handling for it that turns it into a thin wrapper around ref T. [NonVersionable] - internal struct ByReference + internal ref struct ByReference { private IntPtr _value; diff --git a/src/vm/classnames.h b/src/vm/classnames.h index fc08737..6af229e 100644 --- a/src/vm/classnames.h +++ b/src/vm/classnames.h @@ -145,6 +145,7 @@ #define g_CompilerServicesFixedAddressValueTypeAttribute "System.Runtime.CompilerServices.FixedAddressValueTypeAttribute" #define g_CompilerServicesUnsafeValueTypeAttribute "System.Runtime.CompilerServices.UnsafeValueTypeAttribute" +#define g_CompilerServicesIsByRefLikeAttribute "System.Runtime.CompilerServices.IsByRefLikeAttribute" #define g_CompilerServicesIntrinsicAttribute "System.Runtime.CompilerServices.IntrinsicAttribute" #define g_UnmanagedFunctionPointerAttribute "System.Runtime.InteropServices.UnmanagedFunctionPointerAttribute" #define g_DefaultDllImportSearchPathsAttribute "System.Runtime.InteropServices.DefaultDllImportSearchPathsAttribute" diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp index 671f7e8..af7e903 100644 --- a/src/vm/jitinterface.cpp +++ b/src/vm/jitinterface.cpp @@ -6449,20 +6449,13 @@ CorInfoHelpFunc CEEInfo::getBoxHelper(CORINFO_CLASS_HANDLE clsHnd) } else { - // Dev10 718281 - This has been functionally broken fora very long time (at least 2.0). - // The recent addition of the check for stack pointers has caused it to now AV instead - // of gracefully failing with an InvalidOperationException. Since nobody has noticed - // it being broken, we are choosing not to invest to fix it, and instead explicitly - // breaking it and failing early and consistently. if(VMClsHnd.IsTypeDesc()) - { COMPlusThrow(kInvalidOperationException,W("InvalidOperation_TypeCannotBeBoxed")); - } // we shouldn't allow boxing of types that contains stack pointers // csc and vbc already disallow it. if (VMClsHnd.AsMethodTable()->IsByRefLike()) - COMPlusThrow(kInvalidProgramException); + COMPlusThrow(kInvalidProgramException,W("NotSupported_ByRefLike")); result = CORINFO_HELP_BOX; } diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp index c13e220..4e8dd96 100644 --- a/src/vm/methodtablebuilder.cpp +++ b/src/vm/methodtablebuilder.cpp @@ -1441,6 +1441,15 @@ MethodTableBuilder::BuildMethodTableThrowing( { SetUnsafeValueClass(); } + + hr = GetMDImport()->GetCustomAttributeByName(bmtInternal->pType->GetTypeDefToken(), + g_CompilerServicesIsByRefLikeAttribute, + NULL, NULL); + IfFailThrow(hr); + if (hr == S_OK) + { + bmtFP->fIsByRefLikeType = true; + } } // Check to see if the class is an enumeration. No fancy checks like the one immediately @@ -1604,11 +1613,6 @@ MethodTableBuilder::BuildMethodTableThrowing( } } - - - // Set the contextful or marshalbyref flag if necessary - SetContextfulOrByRef(); - // NOTE: This appears to be the earliest point during class loading that other classes MUST be loaded // resolve unresolved interfaces, determine an upper bound on the size of the interface map, // and determine the size of the largest interface (in # slots) @@ -4072,7 +4076,7 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, goto GOT_ELEMENT_TYPE; } - // Inherit IsByRefLike characteristic from fields + // Check ByRefLike fields if (!IsSelfRef(pByValueClass) && pByValueClass->IsByRefLike()) { if (fIsStatic) @@ -4085,8 +4089,10 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, // Non-value-classes cannot contain by-ref-like instance fields BuildMethodTableThrowException(IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD); } - - bmtFP->fIsByRefLikeType = true; + if (!bmtFP->fIsByRefLikeType) + { + BuildMethodTableThrowException(IDS_CLASSLOAD_NOTBYREFLIKE); + } } if (!IsSelfRef(pByValueClass) && pByValueClass->GetClass()->HasNonPublicFields()) @@ -9522,19 +9528,18 @@ void MethodTableBuilder::CheckForSystemTypes() _ASSERTE(g_pByReferenceClass != NULL); _ASSERTE(g_pByReferenceClass->IsByRefLike()); +#ifdef _TARGET_X86_ if (GetCl() == g_pByReferenceClass->GetCl()) { - pMT->SetIsByRefLike(); -#ifdef _TARGET_X86_ // x86 by default treats the type of ByReference as the actual type of its IntPtr field, see calls to // ComputeInternalCorElementTypeForValueType in this file. This is a special case where the struct needs to be // treated as a value type so that its field can be considered as a by-ref pointer. _ASSERTE(pMT->GetFlag(MethodTable::enum_flag_Category_Mask) == MethodTable::enum_flag_Category_PrimitiveValueType); pMT->ClearFlag(MethodTable::enum_flag_Category_Mask); pMT->SetInternalCorElementType(ELEMENT_TYPE_VALUETYPE); -#endif return; } +#endif _ASSERTE(g_pNullableClass->IsNullable()); @@ -9590,18 +9595,17 @@ void MethodTableBuilder::CheckForSystemTypes() { pMT->SetIsNullable(); } +#ifdef _TARGET_X86_ else if (strcmp(name, g_ByReferenceName) == 0) { - pMT->SetIsByRefLike(); -#ifdef _TARGET_X86_ // x86 by default treats the type of ByReference as the actual type of its IntPtr field, see calls to // ComputeInternalCorElementTypeForValueType in this file. This is a special case where the struct needs to be // treated as a value type so that its field can be considered as a by-ref pointer. _ASSERTE(pMT->GetFlag(MethodTable::enum_flag_Category_Mask) == MethodTable::enum_flag_Category_PrimitiveValueType); pMT->ClearFlag(MethodTable::enum_flag_Category_Mask); pMT->SetInternalCorElementType(ELEMENT_TYPE_VALUETYPE); -#endif } +#endif else if (strcmp(name, g_ArgIteratorName) == 0) { // Mark the special types that have embeded stack pointers in them @@ -11157,24 +11161,6 @@ BOOL MethodTableBuilder::NeedsAlignedBaseOffset() // // Used by BuildMethodTable // -// Set the contextful or marshaledbyref flag on the attributes of the class -// -VOID MethodTableBuilder::SetContextfulOrByRef() -{ - CONTRACTL - { - STANDARD_VM_CHECK; - PRECONDITION(CheckPointer(this)); - PRECONDITION(CheckPointer(bmtInternal)); - - } - CONTRACTL_END; - -} -//******************************************************************************* -// -// Used by BuildMethodTable -// // Set the HasFinalizer and HasCriticalFinalizer flags // VOID MethodTableBuilder::SetFinalizationSemantics() diff --git a/src/vm/methodtablebuilder.h b/src/vm/methodtablebuilder.h index 9a05843..d736368 100644 --- a/src/vm/methodtablebuilder.h +++ b/src/vm/methodtablebuilder.h @@ -2875,8 +2875,6 @@ private: VOID CheckForSpecialTypes(); - VOID SetContextfulOrByRef(); - #ifdef FEATURE_READYTORUN VOID CheckLayoutDependsOnOtherModules(MethodTable * pDependencyMT); diff --git a/tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.il b/tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.il new file mode 100644 index 0000000..a231285 --- /dev/null +++ b/tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.il @@ -0,0 +1,186 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +.assembly extern System.Console { } +.assembly extern System.Runtime { } +.assembly Negative_ByRefLikeType { } + +.class sequential ansi sealed beforefieldinit MyByRefLikeType + extends [System.Runtime]System.ValueType +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( 01 00 00 00 ) +} + +.class sequential ansi sealed beforefieldinit ByRefLikeStaticField +{ + .field private static valuetype MyByRefLikeType s +} + +.class sequential ansi sealed beforefieldinit ByRefLikeFieldInNonValueType +{ + .field private valuetype MyByRefLikeType f +} + +.class sequential ansi sealed beforefieldinit ByRefLikeFieldInNonByRefLikeType +{ + .field private valuetype MyByRefLikeType f +} + +.class public auto ansi beforefieldinit Test + extends [System.Runtime]System.Object +{ + .method private hidebysig static void ByRefLikeBoxing() cil managed + { + .maxstack 1 + .locals init (valuetype MyByRefLikeType V_0) + ldloc.0 + box valuetype MyByRefLikeType + pop + ret + } + + .method private hidebysig static void ByRefLikeStaticField() cil managed + { + .maxstack 1 + ldsfld valuetype MyByRefLikeType ByRefLikeStaticField::s + pop + ret + } + + .method private hidebysig static void ByRefLikeFieldInNonValueType() cil managed + { + .maxstack 1 + ldnull + ldfld valuetype MyByRefLikeType ByRefLikeFieldInNonValueType::f + pop + ret + } + + .method private hidebysig static void ByRefLikeFieldInNonByRefLikeType() cil managed + { + .maxstack 1 + .locals init (valuetype ByRefLikeFieldInNonByRefLikeType V_0) + ldloc.0 + ldfld valuetype MyByRefLikeType ByRefLikeFieldInNonValueType::f + pop + ret + } + + .method private hidebysig static void ByRefLikeArray() cil managed + { + .maxstack 1 + ldc.i4.1 + newarr valuetype MyByRefLikeType + pop + ret + } + + .method private hidebysig static void ByRefLikeGenericInstantiation() cil managed + { + .maxstack 1 + newobj instance void class [System.Runtime]System.Collections.Generic.List`1::.ctor() + pop + ret + } + + .method public hidebysig static int32 Main() cil managed + { + .entrypoint + .maxstack 1 + + ldstr "ByRefLikeBoxing" + call void [System.Console]System.Console::WriteLine(string) + .try + { + call void Test::ByRefLikeBoxing() + leave TestFailed + } + catch [System.Runtime]System.InvalidProgramException + { + pop + leave ByRefLikeBoxing_Done + } +ByRefLikeBoxing_Done: + + ldstr "ByRefLikeStaticField" + call void [System.Console]System.Console::WriteLine(string) + .try + { + call void Test::ByRefLikeStaticField() + leave TestFailed + } + catch [System.Runtime]System.TypeLoadException + { + pop + leave ByRefLikeStaticField_Done + } +ByRefLikeStaticField_Done: + + ldstr "ByRefLikeFieldInNonValueType" + call void [System.Console]System.Console::WriteLine(string) + .try + { + call void Test::ByRefLikeFieldInNonValueType() + leave TestFailed + } + catch [System.Runtime]System.TypeLoadException + { + pop + leave ByRefLikeFieldInNonValueType_Done + } +ByRefLikeFieldInNonValueType_Done: + + ldstr "ByRefLikeFieldInNonByRefLikeType" + call void [System.Console]System.Console::WriteLine(string) + .try + { + call void Test::ByRefLikeFieldInNonByRefLikeType() + leave TestFailed + } + catch [System.Runtime]System.TypeLoadException + { + pop + leave ByRefLikeFieldInNonByRefLikeType_Done + } +ByRefLikeFieldInNonByRefLikeType_Done: + + ldstr "ByRefLikeArray" + call void [System.Console]System.Console::WriteLine(string) + .try + { + call void Test::ByRefLikeArray() + leave TestFailed + } + catch [System.Runtime]System.TypeLoadException + { + pop + leave ByRefLikeArray_Done + } +ByRefLikeArray_Done: + + ldstr "ByRefLikeGenericInstantiation" + call void [System.Console]System.Console::WriteLine(string) + .try + { + call void Test::ByRefLikeGenericInstantiation() + leave TestFailed + } + catch [System.Runtime]System.TypeLoadException + { + pop + leave ByRefLikeGenericInstantiation_Done + } +ByRefLikeGenericInstantiation_Done: + + ldstr "All Tests Passed" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.s 100 + ret + TestFailed: + ldstr "Test Failed" + call void [System.Console]System.Console::WriteLine(string) + ldc.i4.1 + ret + } +} diff --git a/tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.ilproj b/tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.ilproj new file mode 100644 index 0000000..deca7f1 --- /dev/null +++ b/tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.ilproj @@ -0,0 +1,23 @@ + + + + + Negative_ByRefLikeType + Debug + AnyCPU + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + true + Exe + BuildAndRun + 0 + + + + + + + + + -- 2.7.4