From 1fe1fb10c64dd87dea1cdabbc81194f26f00206c Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 13 Jul 2022 08:28:55 -0700 Subject: [PATCH] Fix error handling for generic arguments constrained as Enums Fixes #71884 --- .../RuntimeHelpers.CoreCLR.cs | 6 ++ .../src/System/RuntimeType.CoreCLR.cs | 58 ++++++++++++++++++- .../src/System/TypedReference.CoreCLR.cs | 2 +- .../src/System/RuntimeType.cs | 13 +++-- .../System.Private.CoreLib/src/System/Enum.cs | 4 +- .../src/System/RuntimeType.cs | 27 +++------ .../System.Runtime/tests/System/EnumTests.cs | 15 +++++ .../src/System/RuntimeType.Mono.cs | 15 +++++ 8 files changed, 111 insertions(+), 29 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 66e90085e2a7e..a158321d5dbe5 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -650,6 +650,12 @@ public bool IsTypeDesc return (MethodTable*)m_asTAddr; } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static TypeHandle TypeHandleOf() + { + return new TypeHandle((void*)RuntimeTypeHandle.GetValueInternal(typeof(T).TypeHandle)); + } } // Helper structs used for tail calls via helper. diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 1b57a97d18c01..3ae8e5f385034 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -2403,6 +2403,11 @@ private static bool FilterApplyMethodBase( #region Private\Internal Members + internal unsafe TypeHandle GetNativeTypeHandle() + { + return new TypeHandle((void*)m_handle); + } + internal IntPtr GetUnderlyingNativeHandle() { return m_handle; @@ -3360,7 +3365,56 @@ public override Guid GUID [MethodImpl(MethodImplOptions.InternalCall)] private extern void GetGUID(ref Guid result); - internal bool IsDelegate() => GetBaseType() == typeof(MulticastDelegate); + protected override unsafe bool IsValueTypeImpl() + { + TypeHandle th = GetNativeTypeHandle(); + + // We need to return true for generic parameters with the ValueType constraint. + if (th.IsTypeDesc) + return IsSubclassOf(typeof(ValueType)); + + bool isValueType = th.AsMethodTable()->IsValueType; + GC.KeepAlive(this); + return isValueType; + } + + public override unsafe bool IsEnum + { + get + { + TypeHandle th = GetNativeTypeHandle(); + + // We need to return true for generic parameters with the Enum constraint. + if (th.IsTypeDesc) + return IsSubclassOf(typeof(Enum)); + + bool isEnum = th.AsMethodTable()->ParentMethodTable == System.Runtime.CompilerServices.TypeHandle.TypeHandleOf().AsMethodTable(); + GC.KeepAlive(this); + return isEnum; + } + } + + // This returns true for actual enum types only. + internal unsafe bool IsActualEnum + { + get + { + TypeHandle th = GetNativeTypeHandle(); + + bool isEnum = !th.IsTypeDesc && th.AsMethodTable()->ParentMethodTable == System.Runtime.CompilerServices.TypeHandle.TypeHandleOf().AsMethodTable(); + GC.KeepAlive(this); + return isEnum; + } + } + + internal unsafe bool IsDelegate() + { + TypeHandle th = GetNativeTypeHandle(); + + bool isDelegate = !th.IsTypeDesc && th.AsMethodTable()->ParentMethodTable == System.Runtime.CompilerServices.TypeHandle.TypeHandleOf().AsMethodTable(); + GC.KeepAlive(this); + return isDelegate; + } public override GenericParameterAttributes GenericParameterAttributes { @@ -3756,7 +3810,7 @@ internal bool TryByRefFastPath(ref object arg, ref bool isValueType) internal static CorElementType GetUnderlyingType(RuntimeType type) { - if (type.IsEnum) + if (type.IsActualEnum) { type = (RuntimeType)Enum.GetUnderlyingType(type); } diff --git a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs index dc7e3ee58693f..46b9d2bd9fb2e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs @@ -29,7 +29,7 @@ public ref partial struct TypedReference // for System.UIntPtr without inspecting the type desc any further. Otherwise, the type handle // is just wrapping a method table pointer, so return that directly with a reinterpret cast. MethodTable* pMethodTable = typeHandle.IsTypeDesc - ? (MethodTable*)RuntimeTypeHandle.GetValueInternal(typeof(UIntPtr).TypeHandle) + ? TypeHandle.TypeHandleOf().AsMethodTable() : typeHandle.AsMethodTable(); Debug.Assert(pMethodTable is not null); diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.cs index 487715c2c05ab..cce84ae6ed1f8 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.cs @@ -21,7 +21,7 @@ public abstract class RuntimeType : TypeInfo // For desktop compatibility, do not bounce an incoming integer that's the wrong size. // Do a value-preserving cast of both it and the enum values and do a 64-bit compare. - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum); return Enum.GetEnumName(this, rawValue); @@ -29,7 +29,7 @@ public abstract class RuntimeType : TypeInfo public sealed override string[] GetEnumNames() { - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); string[] ret = Enum.InternalGetNames(this); @@ -40,7 +40,7 @@ public sealed override string[] GetEnumNames() public sealed override Type GetEnumUnderlyingType() { - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); return Enum.InternalGetUnderlyingType(this); @@ -51,7 +51,7 @@ public sealed override bool IsEnumDefined(object value) if (value == null) throw new ArgumentNullException(nameof(value)); - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); if (value is string valueAsString) @@ -94,7 +94,7 @@ public sealed override bool IsEnumDefined(object value) [RequiresDynamicCode("It might not be possible to create an array of the enum type at runtime. Use the GetValues overload instead.")] public sealed override Array GetEnumValues() { - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); Array values = Enum.GetEnumInfo(this).ValuesAsUnderlyingType; @@ -110,5 +110,8 @@ public sealed override Array GetEnumValues() Array.Copy(values, result, values.Length); return result; } + + internal bool IsActualEnum + => TryGetEEType(out EETypePtr eeType) && eeType.IsEnum; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Enum.cs b/src/libraries/System.Private.CoreLib/src/System/Enum.cs index 194f0b6906835..e3f994148b55d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Enum.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Enum.cs @@ -1486,10 +1486,10 @@ private static RuntimeType ValidateRuntimeType(Type enumType) { ArgumentNullException.ThrowIfNull(enumType); - if (!enumType.IsEnum) - throw new ArgumentException(SR.Arg_MustBeEnum, nameof(enumType)); if (enumType is not RuntimeType rtType) throw new ArgumentException(SR.Arg_MustBeType, nameof(enumType)); + if (!rtType.IsActualEnum) + throw new ArgumentException(SR.Arg_MustBeEnum, nameof(enumType)); #if NATIVEAOT // Check for the unfortunate "typeof(Outer<>.InnerEnum)" corner case. // https://github.com/dotnet/runtime/issues/7976 diff --git a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs index 1af83dc8ce592..0182163de6aef 100644 --- a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs @@ -97,9 +97,9 @@ public override MemberInfo[] GetDefaultMembers() { ArgumentNullException.ThrowIfNull(value); - Type valueType = value.GetType(); + RuntimeType valueType = (RuntimeType)value.GetType(); - if (!(valueType.IsEnum || IsIntegerType(valueType))) + if (!(valueType.IsActualEnum || IsIntegerType(valueType))) throw new ArgumentException(SR.Arg_MustBeEnumBaseTypeOrEnum, nameof(value)); ulong ulValue = Enum.ToUInt64(value); @@ -109,7 +109,7 @@ public override MemberInfo[] GetDefaultMembers() public override string[] GetEnumNames() { - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); string[] ret = Enum.InternalGetNames(this); @@ -121,7 +121,7 @@ public override string[] GetEnumNames() [RequiresDynamicCode("It might not be possible to create an array of the enum type at runtime. Use the GetValues overload instead.")] public override Array GetEnumValues() { - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); // Get all of the values @@ -141,7 +141,7 @@ public override Array GetEnumValues() public override Type GetEnumUnderlyingType() { - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); return Enum.InternalGetUnderlyingType(this); @@ -202,7 +202,7 @@ protected override TypeCode GetTypeCodeImpl() typeCode = TypeCode.Decimal; else if (ReferenceEquals(this, typeof(DateTime))) typeCode = TypeCode.DateTime; - else if (IsEnum) + else if (IsActualEnum) typeCode = GetTypeCode(Enum.InternalGetUnderlyingType(this)); else typeCode = TypeCode.Object; @@ -251,14 +251,14 @@ public override bool IsEnumDefined(object value) { ArgumentNullException.ThrowIfNull(value); - if (!IsEnum) + if (!IsActualEnum) throw new ArgumentException(SR.Arg_MustBeEnum, "enumType"); // Check if both of them are of the same type RuntimeType valueType = (RuntimeType)value.GetType(); // If the value is an Enum then we need to extract the underlying value from it - if (valueType.IsEnum) + if (valueType.IsActualEnum) { if (!valueType.IsEquivalentTo(this)) throw new ArgumentException(SR.Format(SR.Arg_EnumAndObjectMustBeSameType, valueType, this)); @@ -292,17 +292,6 @@ public override bool IsEnumDefined(object value) } } - protected override bool IsValueTypeImpl() - { - // We need to return true for generic parameters with the ValueType constraint. - // So we cannot use the faster RuntimeTypeHandle.IsValueType because it returns - // false for all generic parameters. - if (this == typeof(ValueType) || this == typeof(Enum)) - return false; - - return IsSubclassOf(typeof(ValueType)); - } - protected override bool IsByRefImpl() => RuntimeTypeHandle.IsByRef(this); protected override bool IsPrimitiveImpl() => RuntimeTypeHandle.IsPrimitive(this); diff --git a/src/libraries/System.Runtime/tests/System/EnumTests.cs b/src/libraries/System.Runtime/tests/System/EnumTests.cs index a764555bd93fd..8ca1c7351f60c 100644 --- a/src/libraries/System.Runtime/tests/System/EnumTests.cs +++ b/src/libraries/System.Runtime/tests/System/EnumTests.cs @@ -1599,6 +1599,21 @@ public void GetValues_EnumTypeNotEnum_ThrowsArgumentException(Type enumType) AssertExtensions.Throws("enumType", () => Enum.GetValues(enumType)); } + private class ClassWithEnumConstraint where T : Enum { } + + [Fact] + public void EnumConstraint_ThrowsArgumentException() + { + Type genericArgumentWithEnumConstraint = typeof(ClassWithEnumConstraint<>).GetGenericArguments()[0]; + Assert.True(genericArgumentWithEnumConstraint.IsEnum); + + Assert.Throws(() => Enum.GetUnderlyingType(genericArgumentWithEnumConstraint)); + Assert.Throws(() => Enum.IsDefined(genericArgumentWithEnumConstraint, 1)); + Assert.Throws(() => Enum.GetName(genericArgumentWithEnumConstraint, 1)); + Assert.Throws(() => Enum.GetNames(genericArgumentWithEnumConstraint)); + Assert.Throws(() => Enum.GetValues(genericArgumentWithEnumConstraint)); + } + public static IEnumerable ToString_Format_TestData() { // Format "D": the decimal equivalent of the value is returned. diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index d5c707c87031b..eed3d75c60bd5 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1262,8 +1262,23 @@ internal bool IsDelegate() return GetBaseType() == typeof(System.MulticastDelegate); } + protected override bool IsValueTypeImpl() + { + // We need to return true for generic parameters with the ValueType constraint. + // So we cannot use the faster RuntimeTypeHandle.IsValueType because it returns + // false for all generic parameters. + if (this == typeof(ValueType) || this == typeof(Enum)) + return false; + + return IsSubclassOf(typeof(ValueType)); + } + + // Returns true for generic parameters with the Enum constraint. public override bool IsEnum => GetBaseType() == EnumType; + // Returns true for actual enum types only. + internal bool IsActualEnum => RuntimeTypeHandle.GetBaseType(this) == EnumType; + public override GenericParameterAttributes GenericParameterAttributes { get