From 2d801010e2e3cea69c49b961a72f03f6d391715e Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 18 Nov 2020 09:38:30 -0800 Subject: [PATCH] GetUninitializedObject fixes & improved tests (#44843) * Improve unit test coverage for GetUninitializedObject * Normalize most checks between coreclr and mono --- src/coreclr/src/vm/reflectioninvocation.cpp | 4 +- .../TestUtilities/System/PlatformDetection.cs | 1 + .../CompilerServices/RuntimeHelpersTests.cs | 120 +++++++++++++++++++-- src/mono/mono/metadata/icall.c | 10 ++ 4 files changed, 122 insertions(+), 13 deletions(-) diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index 3dfa8ca..e8bc4be 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2193,8 +2193,8 @@ FCIMPL1(Object*, ReflectionSerialization::GetUninitializedObject, ReflectClassBa TypeHandle type = objType->GetType(); - // Don't allow arrays, pointers, byrefs or function pointers. - if (type.IsTypeDesc() || type.IsArray()) + // Don't allow void, arrays, pointers, byrefs or function pointers. + if (type.IsTypeDesc() || type.IsArray() || type.GetSignatureCorElementType() == ELEMENT_TYPE_VOID) COMPlusThrow(kArgumentException, W("Argument_InvalidValue")); MethodTable *pMT = type.AsMethodTable(); diff --git a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs index 313e619..3bb73df 100644 --- a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs +++ b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs @@ -77,6 +77,7 @@ namespace System } public static bool IsInContainer => GetIsInContainer(); + public static bool SupportsComInterop => IsWindows && IsNotMonoRuntime; // matches definitions in clr.featuredefines.props public static bool SupportsSsl3 => GetSsl3Support(); public static bool SupportsSsl2 => IsWindows && !PlatformDetection.IsWindows10Version1607OrGreater; diff --git a/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs b/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs index d3097d9..12620e9 100644 --- a/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs +++ b/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs @@ -1,11 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Reflection; using System.Collections; using System.Collections.Generic; -using System.Runtime.CompilerServices; +using System.IO; +using System.Reflection; using System.Runtime.InteropServices; using Xunit; @@ -192,16 +191,107 @@ namespace System.Runtime.CompilerServices.Tests } } + public static IEnumerable GetUninitializedObject_NegativeTestCases() + { + // TODO: Test actual function pointer types when typeof(delegate*<...>) support is available + + yield return new[] { typeof(string), typeof(ArgumentException) }; // variable-length type + yield return new[] { typeof(int[]), typeof(ArgumentException) }; // variable-length type + yield return new[] { typeof(int[,]), typeof(ArgumentException) }; // variable-length type + yield return new[] { Array.CreateInstance(typeof(int), new[] { 1 }, new[] { 1 }).GetType(), typeof(ArgumentException) }; // variable-length type (non-szarray) + yield return new[] { typeof(Array), typeof(MemberAccessException) }; // abstract type + yield return new[] { typeof(Enum), typeof(MemberAccessException) }; // abstract type + + yield return new[] { typeof(Stream), typeof(MemberAccessException) }; // abstract type + yield return new[] { typeof(Buffer), typeof(MemberAccessException) }; // static type (runtime sees it as abstract) + yield return new[] { typeof(IDisposable), typeof(MemberAccessException) }; // interface type + + yield return new[] { typeof(List<>), typeof(MemberAccessException) }; // open generic type + yield return new[] { typeof(List<>).GetGenericArguments()[0], PlatformDetection.IsMonoRuntime ? typeof(MemberAccessException) : typeof(ArgumentException) }; // 'T' placeholder typedesc + + yield return new[] { typeof(Delegate), typeof(MemberAccessException) }; // abstract type + + yield return new[] { typeof(void), typeof(ArgumentException) }; // explicit block in place + yield return new[] { typeof(int).MakePointerType(), typeof(ArgumentException) }; // pointer typedesc + yield return new[] { typeof(int).MakeByRefType(), typeof(ArgumentException) }; // byref typedesc + + yield return new[] { typeof(ReadOnlySpan), typeof(NotSupportedException) }; // byref type + yield return new[] { typeof(ArgIterator), typeof(NotSupportedException) }; // byref type + + Type canonType = typeof(object).Assembly.GetType("System.__Canon", throwOnError: false); + if (canonType != null) + { + yield return new[] { typeof(List<>).MakeGenericType(canonType), typeof(NotSupportedException) }; // shared by generic instantiations + } + + Type comObjType = typeof(object).Assembly.GetType("System.__ComObject", throwOnError: false); + if (comObjType != null) + { + yield return new[] { comObjType, typeof(NotSupportedException) }; // COM type + } + + if (PlatformDetection.SupportsComInterop) + { + yield return new[] { typeof(WbemContext), typeof(NotSupportedException) }; // COM type + } + } + + // This type definition is lifted from System.Management, just for testing purposes + [ClassInterface((short)0x0000)] + [Guid("674B6698-EE92-11D0-AD71-00C04FD8FDFF")] + [ComImport] + internal class WbemContext + { + } + + internal class ClassWithBeforeFieldInitCctor + { + private static readonly int _theInt = GetInt(); + + private static int GetInt() + { + AppDomain.CurrentDomain.SetData("ClassWithBeforeFieldInitCctor_CctorRan", true); + return 0; + } + } + + internal class ClassWithNormalCctor + { +#pragma warning disable CS0414 // unused private field + private static readonly int _theInt; +#pragma warning restore CS0414 + + static ClassWithNormalCctor() + { + AppDomain.CurrentDomain.SetData("ClassWithNormalCctor_CctorRan", true); + _theInt = 0; + } + } + [Fact] - public static void GetUninitializedObject_InvalidArguments_ThrowsException() + public static void GetUninitalizedObject_DoesNotRunBeforeFieldInitCctors() { - AssertExtensions.Throws("type", () => RuntimeHelpers.GetUninitializedObject(null)); + object o = RuntimeHelpers.GetUninitializedObject(typeof(ClassWithBeforeFieldInitCctor)); + Assert.IsType(o); - AssertExtensions.Throws(null, () => RuntimeHelpers.GetUninitializedObject(typeof(string))); // special type - Assert.Throws(() => RuntimeHelpers.GetUninitializedObject(typeof(System.IO.Stream))); // abstract type - Assert.Throws(() => RuntimeHelpers.GetUninitializedObject(typeof(System.Collections.IEnumerable))); // interface - Assert.Throws(() => RuntimeHelpers.GetUninitializedObject(typeof(System.Collections.Generic.List<>))); // generic definition - Assert.Throws(() => RuntimeHelpers.GetUninitializedObject(typeof(TypedReference))); // byref-like type + Assert.Null(AppDomain.CurrentDomain.GetData("ClassWithBeforeFieldInitCctor_CctorRan")); + } + + [ActiveIssue("https://github.com/dotnet/runtime/issues/44852", TestRuntimes.Mono)] + [Fact] + public static void GetUninitalizedObject_RunsNormalStaticCtors() + { + object o = RuntimeHelpers.GetUninitializedObject(typeof(ClassWithNormalCctor)); + Assert.IsType(o); + + Assert.Equal(true, AppDomain.CurrentDomain.GetData("ClassWithNormalCctor_CctorRan")); + } + + [Theory] + [MemberData(nameof(GetUninitializedObject_NegativeTestCases))] + public static void GetUninitializedObject_InvalidArguments_ThrowsException(Type typeToInstantiate, Type expectedExceptionType) + { + Assert.Throws(expectedExceptionType, () => RuntimeHelpers.GetUninitializedObject(typeToInstantiate)); } [Fact] @@ -212,10 +302,18 @@ namespace System.Runtime.CompilerServices.Tests } [Fact] + public static void GetUninitializedObject_Struct() + { + object o = RuntimeHelpers.GetUninitializedObject(typeof(Guid)); + Assert.Equal(Guid.Empty, Assert.IsType(o)); + } + + [Fact] public static void GetUninitializedObject_Nullable() { // Nullable returns the underlying type instead - Assert.Equal(typeof(int), RuntimeHelpers.GetUninitializedObject(typeof(Nullable)).GetType()); + object o = RuntimeHelpers.GetUninitializedObject(typeof(int?)); + Assert.Equal(0, Assert.IsType(o)); } private class ObjectWithDefaultCtor diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 2d70ba3..d54276c 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -1381,6 +1381,16 @@ ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetUninitializedObjectI return NULL_HANDLE; } + if (mono_class_is_array (klass) || mono_class_is_pointer (klass) || handle->byref) { + mono_error_set_argument (error, NULL, NULL); + return NULL_HANDLE; + } + + if (MONO_TYPE_IS_VOID (handle)) { + mono_error_set_argument (error, NULL, NULL); + return NULL_HANDLE; + } + if (m_class_is_abstract (klass) || m_class_is_interface (klass) || m_class_is_gtd (klass)) { mono_error_set_member_access (error, NULL, NULL); return NULL_HANDLE; -- 2.7.4