Add runtime support for ref fields (#63985)

* Add mono and coreclr runtime support for ref fields * Update Reflection.Emit tests to validate ref fields. Add test for TypedReference as a ref field.
dotnet · Jan 23, 2022 · 4a6d169 · 4a6d169
1 parent 5605bdf
commit 4a6d169
Show file tree

Hide file tree

Showing 28 changed files with 391 additions and 79 deletions.
diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs
@@ -233,7 +233,7 @@ private int GetTypeRefNested(Type type, Module? refedModule, string? strRefedMod
                 typeName = UnmangleTypeName(typeName);
             }
 
-            Debug.Assert(!type.IsByRef, "Must not be ByRef.");
+            Debug.Assert(!type.IsByRef, "Must not be ByRef. Get token from TypeSpec.");
             Debug.Assert(!type.IsGenericType || type.IsGenericTypeDefinition, "Must not have generic arguments.");
 
             ModuleBuilder thisModule = this;
@@ -1081,19 +1081,16 @@ private int GetTypeTokenWorkerNoLock(Type type, bool getGenericDefinition)
             // instructions.  Tokens are always relative to the Module.  For example,
             // the token value for System.String is likely to be different from
             // Module to Module.  Calling GetTypeToken will cause a reference to be
-            // added to the Module.  This reference becomes a perminate part of the Module,
-            // multiple calles to this method with the same class have no additional side affects.
-            // This function is optimized to use the TypeDef token if Type is within the same module.
-            // We should also be aware of multiple dynamic modules and multiple implementation of Type!!!
-            if (type.IsByRef)
-            {
-                throw new ArgumentException(SR.Argument_CannotGetTypeTokenForByRef);
-            }
-
-            if ((type.IsGenericType && (!type.IsGenericTypeDefinition || !getGenericDefinition)) ||
-                type.IsGenericParameter ||
-                type.IsArray ||
-                type.IsPointer)
+            // added to the Module.  This reference becomes a permanent part of the Module,
+            // multiple calls to this method with the same class have no additional side-effects.
+            // This function is optimized to use the TypeDef token if the Type is within the
+            // same module. We should also be aware of multiple dynamic modules and multiple
+            // implementations of a Type.
+            if ((type.IsGenericType && (!type.IsGenericTypeDefinition || !getGenericDefinition))
+                || type.IsGenericParameter
+                || type.IsArray
+                || type.IsPointer
+                || type.IsByRef)
             {
                 byte[] sig = SignatureHelper.GetTypeSigToken(this, type).InternalGetSignature(out int length);
                 return GetTokenFromTypeSpec(sig, length);

diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs
@@ -1922,7 +1922,9 @@ private EventBuilder DefineEventNoLock(string name, EventAttributes attributes,
 
             int tkParent = 0;
             if (m_typeParent != null)
+            {
                 tkParent = m_module.GetTypeTokenInternal(m_typeParent);
+            }
 
             ModuleBuilder module = m_module;
 

diff --git a/src/coreclr/utilcode/util.cpp b/src/coreclr/utilcode/util.cpp
@@ -1806,9 +1806,7 @@ HRESULT validateOneArg(
                 // Validate the referenced type.
                 if(FAILED(hr = validateOneArg(tk, pSig, pulNSentinels, pImport, FALSE))) IfFailGo(hr);
                 break;
-            case ELEMENT_TYPE_BYREF:  //fallthru
-                if(TypeFromToken(tk)==mdtFieldDef) IfFailGo(VLDTR_E_SIG_BYREFINFIELD);
-                FALLTHROUGH;
+            case ELEMENT_TYPE_BYREF:
             case ELEMENT_TYPE_PINNED:
             case ELEMENT_TYPE_SZARRAY:
                 // Validate the referenced type.

diff --git a/src/coreclr/vm/field.cpp b/src/coreclr/vm/field.cpp
@@ -61,6 +61,8 @@ VOID FieldDesc::Init(mdFieldDef mb, CorElementType FieldType, DWORD dwMemberAttr
         FieldType == ELEMENT_TYPE_R8 ||
         FieldType == ELEMENT_TYPE_CLASS ||
         FieldType == ELEMENT_TYPE_VALUETYPE ||
+        FieldType == ELEMENT_TYPE_BYREF ||
+        FieldType == ELEMENT_TYPE_TYPEDBYREF ||
         FieldType == ELEMENT_TYPE_PTR ||
         FieldType == ELEMENT_TYPE_FNPTR
         );
@@ -70,7 +72,8 @@ VOID FieldDesc::Init(mdFieldDef mb, CorElementType FieldType, DWORD dwMemberAttr
     m_requiresFullMbValue = 0;
     SetMemberDef(mb);
 
-    m_type = FieldType;
+    // A TypedByRef should be treated like a regular value type.
+    m_type = FieldType != ELEMENT_TYPE_TYPEDBYREF ? FieldType : ELEMENT_TYPE_VALUETYPE;
     m_prot = fdFieldAccessMask & dwMemberAttrs;
     m_isStatic = fIsStatic != 0;
     m_isRVA = fIsRVA != 0;
@@ -81,7 +84,7 @@ VOID FieldDesc::Init(mdFieldDef mb, CorElementType FieldType, DWORD dwMemberAttr
 #endif
 
     _ASSERTE(GetMemberDef() == mb);                 // no truncation
-    _ASSERTE(GetFieldType() == FieldType);
+    _ASSERTE(GetFieldType() == FieldType || (FieldType == ELEMENT_TYPE_TYPEDBYREF && m_type == ELEMENT_TYPE_VALUETYPE));
     _ASSERTE(GetFieldProtection() == (fdFieldAccessMask & dwMemberAttrs));
     _ASSERTE((BOOL) IsStatic() == (fIsStatic != 0));
 }
@@ -152,6 +155,7 @@ TypeHandle FieldDesc::LookupFieldTypeHandle(ClassLoadLevel level, BOOL dropGener
     _ASSERTE(type == ELEMENT_TYPE_CLASS ||
              type == ELEMENT_TYPE_VALUETYPE ||
              type == ELEMENT_TYPE_STRING ||
+             type == ELEMENT_TYPE_TYPEDBYREF ||
              type == ELEMENT_TYPE_SZARRAY ||
              type == ELEMENT_TYPE_VAR
              );

diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp
@@ -9002,9 +9002,6 @@ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd,
     FieldDesc* field = (FieldDesc*) fieldHnd;
     CorElementType type   = field->GetFieldType();
 
-    // <REVISIT_TODO>TODO should not burn the time to do this for anything but Value Classes</REVISIT_TODO>
-    _ASSERTE(type != ELEMENT_TYPE_BYREF);
-
     if (type == ELEMENT_TYPE_I)
     {
         PTR_MethodTable enclosingMethodTable = field->GetApproxEnclosingMethodTable();

diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp
@@ -3935,6 +3935,27 @@ VOID    MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList,
                 break;
             }
 
+        case ELEMENT_TYPE_BYREF:
+            {
+                dwLog2FieldSize = LOG2_PTRSIZE;
+                if (fIsStatic)
+                {
+                    // Byref-like types cannot be used for static fields
+                    BuildMethodTableThrowException(IDS_CLASSLOAD_BYREFLIKE_STATICFIELD);
+                }
+                if (!bmtFP->fIsByRefLikeType)
+                {
+                    // Non-byref-like types cannot contain byref-like instance fields
+                    BuildMethodTableThrowException(IDS_CLASSLOAD_BYREFLIKE_INSTANCEFIELD);
+                }
+                break;
+            }
+
+        case ELEMENT_TYPE_TYPEDBYREF:
+            {
+                goto IS_VALUETYPE;
+            }
+
         // Class type variable (method type variables aren't allowed in fields)
         // These only occur in open types used for verification/reflection.
         case ELEMENT_TYPE_VAR:
@@ -4042,7 +4063,10 @@ VOID    MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList,
                         pByValueClass = (MethodTable *)-1;
                     }
                 } // If 'this' is a value class
-
+            }
+            // TypedReference shares the rest of the code here
+IS_VALUETYPE:
+            {
                 // It's not self-referential so try to load it
                 if (pByValueClass == NULL)
                 {

diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
@@ -903,9 +903,6 @@
   <data name="Argument_CannotCreateTypedReference" xml:space="preserve">
     <value>Cannot use function evaluation to create a TypedReference object.</value>
   </data>
-  <data name="Argument_CannotGetTypeTokenForByRef" xml:space="preserve">
-    <value>Cannot get TypeToken for a ByRef type.</value>
-  </data>
   <data name="Argument_CannotSetParentToInterface" xml:space="preserve">
     <value>Cannot set parent to an interface.</value>
   </data>

diff --git a/src/libraries/System.Reflection.Emit/tests/MethodBuilder/MethodBuilderByRefs.cs b/src/libraries/System.Reflection.Emit/tests/MethodBuilder/MethodBuilderByRefs.cs
@@ -0,0 +1,21 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Linq;
+using Xunit;
+
+namespace System.Reflection.Emit.Tests
+{
+    public class MethodBuilderByRefs
+    {
+        [Fact]
+        public void ByRef_Ldtoken()
+        {
+            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);
+            MethodBuilder method = type.DefineMethod("TestMethod", MethodAttributes.Public, typeof(Type), Type.EmptyTypes);
+            ILGenerator ilg = method.GetILGenerator();
+            ilg.Emit(OpCodes.Ldtoken, typeof(int).MakeByRefType());
+            ilg.Emit(OpCodes.Ret);
+        }
+    }
+}
diff --git a/src/libraries/System.Reflection.Emit/tests/ModuleBuilder/ModuleBuilderDefineEnum.cs b/src/libraries/System.Reflection.Emit/tests/ModuleBuilder/ModuleBuilderDefineEnum.cs
@@ -160,11 +160,11 @@ public void DefineEnum_VoidUnderlyingType_ThrowsArgumentException()
         }
 
         [Fact]
-        public void DefineEnum_ByRefUnderlyingType_ThrowsCOMExceptionOnCreation()
+        public void DefineEnum_ByRefUnderlyingType_ThrowsTypeLoadExceptionOnCreation()
         {
             ModuleBuilder module = Helpers.DynamicModule();
             EnumBuilder enumBuilder = module.DefineEnum("Name", TypeAttributes.Public, typeof(int).MakeByRefType());
-            Assert.Throws<COMException>(() => enumBuilder.CreateTypeInfo());
+            Assert.Throws<TypeLoadException>(() => enumBuilder.CreateTypeInfo());
         }
 
         [Theory]

diff --git a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj
@@ -33,6 +33,7 @@
     <Compile Include="GenericTypeParameterBuilder\GenericTypeParameterBuilderSetCustomAttribute.cs" />
     <Compile Include="GenericTypeParameterBuilder\GenericTypeParameterBuilderSetGenericParameterAttributes.cs" />
     <Compile Include="GenericTypeParameterBuilder\GenericTypeParameterBuilderSetInterfaceConstraints.cs" />
+    <Compile Include="MethodBuilder\MethodBuilderByRefs.cs" />
     <Compile Include="MethodBuilder\MethodBuilderContainsGenericParameters.cs" />
     <Compile Include="MethodBuilder\MethodBuilderDefineGenericParameters.cs" />
     <Compile Include="MethodBuilder\MethodBuilderDefineParameter.cs" />

diff --git a/...ibraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderAddInterfaceImplementaion.cs b/...ibraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderAddInterfaceImplementaion.cs
@@ -61,13 +61,6 @@ public void AddInterfaceImplementation_NullInterfaceType_ThrowsArgumentNullExcep
             AssertExtensions.Throws<ArgumentNullException>("interfaceType", () => type.AddInterfaceImplementation(null));
         }
 
-        [Fact]
-        public void AddInterfaceImplementation_ByRefInterfaceType_ThrowsArgumentExceptioN()
-        {
-            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);
-            AssertExtensions.Throws<ArgumentException>(null, () => type.AddInterfaceImplementation(typeof(int).MakeByRefType()));
-        }
-
         [Fact]
         public void AddInterfaceImplementation_TypeAlreadyCreated_ThrowsInvalidOperationException()
         {

diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineEvent.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineEvent.cs
@@ -106,11 +106,11 @@ public void DefineEvent_Invalid(string name, Type eventType, Type exceptionType)
         }
 
         [Fact]
-        public void DefineEvent_ByRefEventType_ThrowsArgumentException()
+        public void DefineEvent_ByRefEventType()
         {
             TypeBuilder type = Helpers.DynamicType(TypeAttributes.Class | TypeAttributes.Public);
-
-            AssertExtensions.Throws<ArgumentException>(null, () => type.DefineEvent("Name", EventAttributes.None, typeof(int).MakeByRefType()));
+            type.DefineEvent("Name", EventAttributes.None, typeof(int).MakeByRefType());
+            type.CreateTypeInfo().AsType();
         }
 
         [Fact]

diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineField.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineField.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using Xunit;
 
@@ -112,15 +113,6 @@ public void DefineField_VoidFieldType_ThrowsArgumentException()
             AssertExtensions.Throws<ArgumentException>(null, () => type.DefineField("Name", typeof(void), FieldAttributes.Public));
         }
 
-        [Fact]
-        public void DefineField_ByRefFieldType_ThrowsCOMExceptionOnCreation()
-        {
-            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);
-            type.DefineField("Name", typeof(int).MakeByRefType(), FieldAttributes.Public);
-
-            Assert.Throws<COMException>(() => type.CreateTypeInfo());
-        }
-
         [Theory]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/2389", TestRuntimes.Mono)]
         [InlineData((FieldAttributes)(-1), (FieldAttributes)(-38145))]
@@ -152,6 +144,69 @@ public void DefineField_DynamicFieldTypeNotCreated_ThrowsTypeLoadException()
             Assert.Equal(createdFieldType, field.FieldType);
         }
 
+        [Fact]
+        public void DefineByRefField_Class_ThrowsTypeLoadExceptionOnCreation()
+        {
+            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);
+            type.DefineField("Name", typeof(int).MakeByRefType(), FieldAttributes.Public);
+
+            Assert.Throws<TypeLoadException>(() => type.CreateTypeInfo());
+        }
+
+        [Fact]
+        public void DefineByRefField_ValueType_NonByRefLike_ThrowsTypeLoadExceptionOnCreation()
+        {
+            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public, baseType: typeof(ValueType));
+            type.DefineField("Name", typeof(int).MakeByRefType(), FieldAttributes.Public);
+
+            Assert.Throws<TypeLoadException>(() => type.CreateTypeInfo());
+        }
+
+        [Fact]
+        public void DefineByRefField_ValueType_ByRefLike()
+        {
+            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public, baseType: typeof(ValueType));
+
+            // Define type to be ByRefLike
+            CustomAttributeBuilder ca = new(typeof(IsByRefLikeAttribute).GetConstructors()[0], new object[] { });
+            type.SetCustomAttribute(ca);
+
+            type.DefineField("Name", typeof(int).MakeByRefType(), FieldAttributes.Public);
+
+            Type createdType = type.CreateTypeInfo().AsType();
+            FieldInfo[] fields = createdType.GetFields();
+            Assert.Equal(1, fields.Length);
+            Assert.True(fields[0].FieldType.IsByRef);
+        }
+
+        [Fact]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/45152")]
+        public void Instantiate_ValueType_With_ByRefField()
+        {
+            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public, baseType: typeof(ValueType));
+
+            // Define type to be ByRefLike
+            CustomAttributeBuilder ca = new(typeof(IsByRefLikeAttribute).GetConstructors()[0], new object[] { });
+            type.SetCustomAttribute(ca);
+
+            var field = type.DefineField("Name", typeof(int).MakeByRefType(), FieldAttributes.Public);
+
+            var ctor = type.DefineConstructor(MethodAttributes.Public, CallingConventions.HasThis, new Type[] { typeof(string) });
+            {
+                ILGenerator il = ctor.GetILGenerator();
+                il.Emit(OpCodes.Ldarg_0);
+                il.Emit(OpCodes.Ldarga_S, 1);
+                il.Emit(OpCodes.Stfld, field);
+                il.Emit(OpCodes.Ret);
+            }
+
+            Type createdType = type.CreateTypeInfo().AsType();
+
+            var ctorToCall = createdType.GetConstructor(BindingFlags.Public | BindingFlags.Instance, new[] { typeof(string) });
+            var str = "12345";
+            ctorToCall.Invoke(new[] { str });
+        }
+
         [Fact]
         public void GetField_TypeNotCreated_ThrowsNotSupportedException()
         {

diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineNestedType.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineNestedType.cs
@@ -210,15 +210,9 @@ public void DefineNestedType_NullInterface_ThrowsArgumentNullException()
             AssertExtensions.Throws<ArgumentNullException>("interfaces", () => type.DefineNestedType("Name", TypeAttributes.NestedPublic, typeof(object), new Type[] { null }));
         }
 
-        [Fact]
-        public void DefineNestedType_ByRefInterfaceType_ThrowsArgumentException()
-        {
-            TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);
-            AssertExtensions.Throws<ArgumentException>(null, () => type.DefineNestedType("Name", TypeAttributes.NestedPublic, typeof(object), new Type[] { typeof(int).MakeByRefType() }));
-        }
-
         public static IEnumerable<object[]> InvalidInterfaceType_TestData()
         {
+            yield return new object[] { typeof(int).MakeByRefType() };
             yield return new object[] { typeof(EmptyNonGenericClass) };
             yield return new object[] { typeof(EmptyNonGenericStruct) };
             yield return new object[] { typeof(EmptyGenericClass<int>) };

diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetParent.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderSetParent.cs
@@ -56,14 +56,14 @@ public void SetParent_InterfaceType_ThrowsArgumentException(TypeAttributes attri
         }
 
         [Fact]
-        public void SetParent_ByRefType_ThrowsArgumentExceptionOnCreation()
+        public void SetParent_ByRefType_ThrowsNotSupportedExceptionOnCreation()
         {
             TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);
 
             type.SetParent(typeof(int).MakeByRefType());
             Assert.Equal(typeof(int).MakeByRefType(), type.BaseType);
 
-            AssertExtensions.Throws<ArgumentException>(null, () => type.CreateTypeInfo());
+            Assert.Throws<NotSupportedException>(() => type.CreateTypeInfo());
         }
 
         [Fact]

diff --git a/src/libraries/System.Reflection.Emit/tests/Utilities.cs b/src/libraries/System.Reflection.Emit/tests/Utilities.cs
@@ -54,9 +54,16 @@ public static ModuleBuilder DynamicModule(string assemblyName = "TestAssembly",
             return DynamicAssembly(assemblyName).DefineDynamicModule(moduleName);
         }
 
-        public static TypeBuilder DynamicType(TypeAttributes attributes, string assemblyName = "TestAssembly", string moduleName = "TestModule", string typeName = "TestType")
+        public static TypeBuilder DynamicType(TypeAttributes attributes, string assemblyName = "TestAssembly", string moduleName = "TestModule", string typeName = "TestType", Type? baseType = null)
         {
-            return DynamicModule(assemblyName, moduleName).DefineType(typeName, attributes);
+            if (baseType is null)
+            {
+                return DynamicModule(assemblyName, moduleName).DefineType(typeName, attributes);
+            }
+            else
+            {
+                return DynamicModule(assemblyName, moduleName).DefineType(typeName, attributes, baseType);
+            }
         }
 
         public static EnumBuilder DynamicEnum(TypeAttributes visibility, Type underlyingType, string enumName = "TestEnum", string assemblyName = "TestAssembly", string moduleName = "TestModule")

diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/ILGenerator.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/ILGenerator.Mono.cs
@@ -819,9 +819,6 @@ public virtual void Emit(OpCode opcode, string str)
 
         public virtual void Emit(OpCode opcode, Type cls)
         {
-            if (cls != null && cls.IsByRef)
-                throw new ArgumentException("Cannot get TypeToken for a ByRef type.");
-
             make_room(6);
             ll_emit(opcode);
             int token = token_gen.GetToken(cls!, opcode != OpCodes.Ldtoken);