Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add runtime support for ref fields #63985

Merged
merged 27 commits into from
Jan 23, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4ae810d
Add runtime support for ref fields
AaronRobinsonMSFT Jan 19, 2022
34933ac
Update Reflection.Emit tests to validate ref fields.
AaronRobinsonMSFT Jan 19, 2022
6339127
Remove unnecessary addition in ASSERT.
AaronRobinsonMSFT Jan 19, 2022
5fc6174
Mono should throw TypeLoadException for invalid ref field.
AaronRobinsonMSFT Jan 19, 2022
2792c74
coreclr/
AaronRobinsonMSFT Jan 20, 2022
7a2d818
libraries/
AaronRobinsonMSFT Jan 20, 2022
4ff87f9
mono/
AaronRobinsonMSFT Jan 20, 2022
eafc0a8
tests/
AaronRobinsonMSFT Jan 20, 2022
acff92a
mono/
AaronRobinsonMSFT Jan 20, 2022
f2db03b
tests/
AaronRobinsonMSFT Jan 20, 2022
091a48b
mono/
AaronRobinsonMSFT Jan 20, 2022
78fbfd0
Merge remote-tracking branch 'upstream/main' into ref_field_support
AaronRobinsonMSFT Jan 20, 2022
a4604e6
mono/
AaronRobinsonMSFT Jan 21, 2022
f2ec70c
mono/
AaronRobinsonMSFT Jan 21, 2022
3702e57
mono/
AaronRobinsonMSFT Jan 21, 2022
87a3e30
mono/
AaronRobinsonMSFT Jan 21, 2022
4fe2045
mono/
AaronRobinsonMSFT Jan 21, 2022
66e7747
Add test for SRE activation of types with ref fields.
AaronRobinsonMSFT Jan 21, 2022
f493441
libraries/
AaronRobinsonMSFT Jan 22, 2022
9804d2a
coreclr/
AaronRobinsonMSFT Jan 22, 2022
166e4f2
Exclude the RefFields tests from running on llvmfullaot
AaronRobinsonMSFT Jan 22, 2022
e1b2c07
Merge branch 'main' into ref_field_support
AaronRobinsonMSFT Jan 22, 2022
d6e7c0f
Instead of dropping the byref, compute a token from a typespec.
AaronRobinsonMSFT Jan 22, 2022
3059e77
mono/
AaronRobinsonMSFT Jan 22, 2022
8dca52e
coreclr/
AaronRobinsonMSFT Jan 22, 2022
6a5ac57
mono/
AaronRobinsonMSFT Jan 22, 2022
62e06ed
mono/
AaronRobinsonMSFT Jan 22, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/coreclr/utilcode/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1805,9 +1805,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.
Expand Down
8 changes: 6 additions & 2 deletions src/coreclr/vm/field.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand All @@ -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;
Expand All @@ -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));
}
Expand Down Expand Up @@ -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
);
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 25 additions & 1 deletion src/coreclr/vm/methodtablebuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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))]
Expand Down Expand Up @@ -152,6 +144,40 @@ 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));
type.DefineField("Name", typeof(int).MakeByRefType(), FieldAttributes.Public);

// Define type to be ByRefLike
CustomAttributeBuilder ca = new(typeof(IsByRefLikeAttribute).GetConstructors()[0], new object[] { });
type.SetCustomAttribute(ca);

Type createdType = type.CreateTypeInfo().AsType();
FieldInfo[] fields = createdType.GetFields();
Assert.Equal(1, fields.Length);
Assert.True(fields[0].FieldType.IsByRef);
}
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved

[Fact]
public void GetField_TypeNotCreated_ThrowsNotSupportedException()
{
Expand Down
11 changes: 9 additions & 2 deletions src/libraries/System.Reflection.Emit/tests/Utilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ private bool has_ctor_method()
if (fb == null)
continue;
if (fb.FieldType.IsByRef)
throw new COMException();
throw new TypeLoadException();
}
}

Expand Down
92 changes: 92 additions & 0 deletions src/tests/Loader/classloader/RefFields/InvalidCSharp.il
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

.assembly extern System.Runtime { .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) }

.assembly InvalidCSharp { }

.class public auto ansi sealed beforefieldinit InvalidCSharp.InvalidStructWithRefField
extends [System.Runtime]System.ValueType
{
// Type requires IsByRefLikeAttribute to be valid.
.field public string& invalid
}

// This is invalid metadata and is unable to be loaded.
// - [field sig] (0x80131815 (VER_E_FIELD_SIG))
//
// .class public auto ansi sealed beforefieldinit InvalidCSharp.InvalidStructWithStaticRefField
// extends [System.Runtime]System.ValueType
// {
// .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
// 01 00 00 00
// )
// .field public static string& invalid
// }

.class public auto ansi sealed beforefieldinit InvalidCSharp.WithRefField
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
01 00 00 00
)
.field public string& Str

.method public hidebysig specialname rtspecialname
instance void .ctor (
string&
) cil managed
{
ldarg.0
ldarg.1
stfld string& InvalidCSharp.WithRefField::Str
ret
}

.method public hidebysig
instance bool ConfirmFieldInstance (
string
) cil managed
{
ldarg.0
ldfld string& InvalidCSharp.WithRefField::Str
ldind.ref
ldarg.1
ceq
ret
}
}

.class public auto ansi sealed beforefieldinit InvalidCSharp.WithRefStructField
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
01 00 00 00
)
.field public valuetype InvalidCSharp.WithRefField& Field

.method public hidebysig specialname rtspecialname
instance void .ctor (
valuetype InvalidCSharp.WithRefField&
) cil managed
{
ldarg.0
ldarg.1
stfld valuetype InvalidCSharp.WithRefField& InvalidCSharp.WithRefStructField::Field
ret
}

.method public hidebysig
instance bool ConfirmFieldInstance (
valuetype InvalidCSharp.WithRefField&
) cil managed
{
ldarg.0
ldfld valuetype InvalidCSharp.WithRefField& InvalidCSharp.WithRefStructField::Field
ldind.ref
ldarg.1
ldind.ref
ceq
ret
}
}
8 changes: 8 additions & 0 deletions src/tests/Loader/classloader/RefFields/InvalidCSharp.ilproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk.IL">
<PropertyGroup>
<OutputType>Library</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="InvalidCSharp.il" />
</ItemGroup>
</Project>
50 changes: 50 additions & 0 deletions src/tests/Loader/classloader/RefFields/Validate.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// 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.IO;
using InvalidCSharp;

using Xunit;

class Validate
{
[Fact]
[SkipOnMono("Mono doesn't validate ref field state during type load")]
public static void Validate_Invalid_RefField_Fails()
{
Console.WriteLine($"{nameof(Validate_Invalid_RefField_Fails)}...");
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidStructWithRefField); });
}

[Fact]
public static void Validate_RefStructWithRefField_Load()
{
Console.WriteLine($"{nameof(Validate_RefStructWithRefField_Load)}...");
var t = typeof(WithRefField);
}

[Fact]
public static void Validate_Create_RefField()
{
var str = nameof(Validate_Create_RefField);
Console.WriteLine($"{str}...");

WithRefField s = new(ref str);
Assert.True(s.ConfirmFieldInstance(str));

string newStr = new(str);
Assert.False(s.ConfirmFieldInstance(newStr));
}

[Fact]
public static void Validate_Create_RefStructField()
{
var str = nameof(Validate_Create_RefStructField);
Console.WriteLine($"{str}...");

WithRefField s = new(ref str);
WithRefStructField t = new(ref s);
Assert.True(t.ConfirmFieldInstance(ref s));
}
}
12 changes: 12 additions & 0 deletions src/tests/Loader/classloader/RefFields/Validate.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<OutputType>Exe</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="Validate.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="InvalidCSharp.ilproj" />
</ItemGroup>
</Project>