Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 6c12105

Browse files
jkotasjkotas
authored
Detect ByRefLike types using attribute (#15745)
* Detect ByRefLike types using attribute and improve error messages for their invalid use Fixes #11371 and #15458
1 parent 7a78542 commit 6c12105

File tree

9 files changed

+234
-45
lines changed

9 files changed

+234
-45
lines changed

‎src/dlls/mscorrc/mscorrc.rc

+3-2
Original file line numberDiff line numberDiff line change
@@ -1223,8 +1223,9 @@ BEGIN
12231223
IDS_CLASSLOAD_NOTINTERFACE "Could not load type '%1' from assembly '%2' because it attempts to implement a class as an interface."
12241224
IDS_CLASSLOAD_VALUEINSTANCEFIELD "Could not load the value type '%1' from assembly '%2' because it has an instance field of itself."
12251225

1226-
IDS_CLASSLOAD_BYREFLIKE_STATICFIELD "A value type containing a by-ref instance field, such as Span<T>, cannot be used as the type for a static field."
1227-
IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD "A value type containing a by-ref instance field, such as Span<T>, cannot be used as the type for a class instance field."
1226+
IDS_CLASSLOAD_BYREFLIKE_STATICFIELD "A value type containing a ByRef-like instance field cannot be used as the type for a static field."
1227+
IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD "A value type containing a ByRef-like instance field cannot be used as the type for a class instance field."
1228+
IDS_CLASSLOAD_NOTBYREFLIKE "A value type containing a ByRef-like instance field must be ByRef-like type."
12281229

12291230
IDS_CLASSLOAD_BAD_NAME "Type name '%1' from assembly '%2' is invalid."
12301231
IDS_CLASSLOAD_RANK_TOOLARGE "'%1' from assembly '%2' has too many dimensions."

‎src/dlls/mscorrc/resource.h

+1
Original file line numberDiff line numberDiff line change
@@ -893,3 +893,4 @@
893893

894894
#define IDS_CLASSLOAD_BYREFLIKE_STATICFIELD 0x263b
895895
#define IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD 0x263c
896+
#define IDS_CLASSLOAD_NOTBYREFLIKE 0x263d

‎src/mscorlib/src/System/ByReference.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace System
1111
// around lack of first class support for byref fields in C# and IL. The JIT and
1212
// type loader has special handling for it that turns it into a thin wrapper around ref T.
1313
[NonVersionable]
14-
internal struct ByReference<T>
14+
internal ref struct ByReference<T>
1515
{
1616
private IntPtr _value;
1717

‎src/vm/classnames.h

+1
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@
145145

146146
#define g_CompilerServicesFixedAddressValueTypeAttribute "System.Runtime.CompilerServices.FixedAddressValueTypeAttribute"
147147
#define g_CompilerServicesUnsafeValueTypeAttribute "System.Runtime.CompilerServices.UnsafeValueTypeAttribute"
148+
#define g_CompilerServicesIsByRefLikeAttribute "System.Runtime.CompilerServices.IsByRefLikeAttribute"
148149
#define g_CompilerServicesIntrinsicAttribute "System.Runtime.CompilerServices.IntrinsicAttribute"
149150
#define g_UnmanagedFunctionPointerAttribute "System.Runtime.InteropServices.UnmanagedFunctionPointerAttribute"
150151
#define g_DefaultDllImportSearchPathsAttribute "System.Runtime.InteropServices.DefaultDllImportSearchPathsAttribute"

‎src/vm/jitinterface.cpp

+1-8
Original file line numberDiff line numberDiff line change
@@ -6449,20 +6449,13 @@ CorInfoHelpFunc CEEInfo::getBoxHelper(CORINFO_CLASS_HANDLE clsHnd)
64496449
}
64506450
else
64516451
{
6452-
// Dev10 718281 - This has been functionally broken fora very long time (at least 2.0).
6453-
// The recent addition of the check for stack pointers has caused it to now AV instead
6454-
// of gracefully failing with an InvalidOperationException. Since nobody has noticed
6455-
// it being broken, we are choosing not to invest to fix it, and instead explicitly
6456-
// breaking it and failing early and consistently.
64576452
if(VMClsHnd.IsTypeDesc())
6458-
{
64596453
COMPlusThrow(kInvalidOperationException,W("InvalidOperation_TypeCannotBeBoxed"));
6460-
}
64616454

64626455
// we shouldn't allow boxing of types that contains stack pointers
64636456
// csc and vbc already disallow it.
64646457
if (VMClsHnd.AsMethodTable()->IsByRefLike())
6465-
COMPlusThrow(kInvalidProgramException);
6458+
COMPlusThrow(kInvalidProgramException,W("NotSupported_ByRefLike"));
64666459

64676460
result = CORINFO_HELP_BOX;
64686461
}

‎src/vm/methodtablebuilder.cpp

+18-32
Original file line numberDiff line numberDiff line change
@@ -1441,6 +1441,15 @@ MethodTableBuilder::BuildMethodTableThrowing(
14411441
{
14421442
SetUnsafeValueClass();
14431443
}
1444+
1445+
hr = GetMDImport()->GetCustomAttributeByName(bmtInternal->pType->GetTypeDefToken(),
1446+
g_CompilerServicesIsByRefLikeAttribute,
1447+
NULL, NULL);
1448+
IfFailThrow(hr);
1449+
if (hr == S_OK)
1450+
{
1451+
bmtFP->fIsByRefLikeType = true;
1452+
}
14441453
}
14451454

14461455
// Check to see if the class is an enumeration. No fancy checks like the one immediately
@@ -1604,11 +1613,6 @@ MethodTableBuilder::BuildMethodTableThrowing(
16041613
}
16051614
}
16061615

1607-
1608-
1609-
// Set the contextful or marshalbyref flag if necessary
1610-
SetContextfulOrByRef();
1611-
16121616
// NOTE: This appears to be the earliest point during class loading that other classes MUST be loaded
16131617
// resolve unresolved interfaces, determine an upper bound on the size of the interface map,
16141618
// and determine the size of the largest interface (in # slots)
@@ -4072,7 +4076,7 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList,
40724076
goto GOT_ELEMENT_TYPE;
40734077
}
40744078

4075-
// Inherit IsByRefLike characteristic from fields
4079+
// Check ByRefLike fields
40764080
if (!IsSelfRef(pByValueClass) && pByValueClass->IsByRefLike())
40774081
{
40784082
if (fIsStatic)
@@ -4085,8 +4089,10 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList,
40854089
// Non-value-classes cannot contain by-ref-like instance fields
40864090
BuildMethodTableThrowException(IDS_CLASSLOAD_BYREFLIKE_NOTVALUECLASSFIELD);
40874091
}
4088-
4089-
bmtFP->fIsByRefLikeType = true;
4092+
if (!bmtFP->fIsByRefLikeType)
4093+
{
4094+
BuildMethodTableThrowException(IDS_CLASSLOAD_NOTBYREFLIKE);
4095+
}
40904096
}
40914097

40924098
if (!IsSelfRef(pByValueClass) && pByValueClass->GetClass()->HasNonPublicFields())
@@ -9522,19 +9528,18 @@ void MethodTableBuilder::CheckForSystemTypes()
95229528
_ASSERTE(g_pByReferenceClass != NULL);
95239529
_ASSERTE(g_pByReferenceClass->IsByRefLike());
95249530

9531+
#ifdef _TARGET_X86_
95259532
if (GetCl() == g_pByReferenceClass->GetCl())
95269533
{
9527-
pMT->SetIsByRefLike();
9528-
#ifdef _TARGET_X86_
95299534
// x86 by default treats the type of ByReference<T> as the actual type of its IntPtr field, see calls to
95309535
// ComputeInternalCorElementTypeForValueType in this file. This is a special case where the struct needs to be
95319536
// treated as a value type so that its field can be considered as a by-ref pointer.
95329537
_ASSERTE(pMT->GetFlag(MethodTable::enum_flag_Category_Mask) == MethodTable::enum_flag_Category_PrimitiveValueType);
95339538
pMT->ClearFlag(MethodTable::enum_flag_Category_Mask);
95349539
pMT->SetInternalCorElementType(ELEMENT_TYPE_VALUETYPE);
9535-
#endif
95369540
return;
95379541
}
9542+
#endif
95389543

95399544
_ASSERTE(g_pNullableClass->IsNullable());
95409545

@@ -9590,18 +9595,17 @@ void MethodTableBuilder::CheckForSystemTypes()
95909595
{
95919596
pMT->SetIsNullable();
95929597
}
9598+
#ifdef _TARGET_X86_
95939599
else if (strcmp(name, g_ByReferenceName) == 0)
95949600
{
9595-
pMT->SetIsByRefLike();
9596-
#ifdef _TARGET_X86_
95979601
// x86 by default treats the type of ByReference<T> as the actual type of its IntPtr field, see calls to
95989602
// ComputeInternalCorElementTypeForValueType in this file. This is a special case where the struct needs to be
95999603
// treated as a value type so that its field can be considered as a by-ref pointer.
96009604
_ASSERTE(pMT->GetFlag(MethodTable::enum_flag_Category_Mask) == MethodTable::enum_flag_Category_PrimitiveValueType);
96019605
pMT->ClearFlag(MethodTable::enum_flag_Category_Mask);
96029606
pMT->SetInternalCorElementType(ELEMENT_TYPE_VALUETYPE);
9603-
#endif
96049607
}
9608+
#endif
96059609
else if (strcmp(name, g_ArgIteratorName) == 0)
96069610
{
96079611
// Mark the special types that have embeded stack pointers in them
@@ -11153,24 +11157,6 @@ BOOL MethodTableBuilder::NeedsAlignedBaseOffset()
1115311157
}
1115411158
#endif // FEATURE_READYTORUN
1115511159

11156-
//*******************************************************************************
11157-
//
11158-
// Used by BuildMethodTable
11159-
//
11160-
// Set the contextful or marshaledbyref flag on the attributes of the class
11161-
//
11162-
VOID MethodTableBuilder::SetContextfulOrByRef()
11163-
{
11164-
CONTRACTL
11165-
{
11166-
STANDARD_VM_CHECK;
11167-
PRECONDITION(CheckPointer(this));
11168-
PRECONDITION(CheckPointer(bmtInternal));
11169-
11170-
}
11171-
CONTRACTL_END;
11172-
11173-
}
1117411160
//*******************************************************************************
1117511161
//
1117611162
// Used by BuildMethodTable

‎src/vm/methodtablebuilder.h

-2
Original file line numberDiff line numberDiff line change
@@ -2875,8 +2875,6 @@ class MethodTableBuilder
28752875

28762876
VOID CheckForSpecialTypes();
28772877

2878-
VOID SetContextfulOrByRef();
2879-
28802878
#ifdef FEATURE_READYTORUN
28812879

28822880
VOID CheckLayoutDependsOnOtherModules(MethodTable * pDependencyMT);

‎tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.il

+186
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
.assembly extern System.Console { }
6+
.assembly extern System.Runtime { }
7+
.assembly Negative_ByRefLikeType { }
8+
9+
.class sequential ansi sealed beforefieldinit MyByRefLikeType
10+
extends [System.Runtime]System.ValueType
11+
{
12+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( 01 00 00 00 )
13+
}
14+
15+
.class sequential ansi sealed beforefieldinit ByRefLikeStaticField
16+
{
17+
.field private static valuetype MyByRefLikeType s
18+
}
19+
20+
.class sequential ansi sealed beforefieldinit ByRefLikeFieldInNonValueType
21+
{
22+
.field private valuetype MyByRefLikeType f
23+
}
24+
25+
.class sequential ansi sealed beforefieldinit ByRefLikeFieldInNonByRefLikeType
26+
{
27+
.field private valuetype MyByRefLikeType f
28+
}
29+
30+
.class public auto ansi beforefieldinit Test
31+
extends [System.Runtime]System.Object
32+
{
33+
.method private hidebysig static void ByRefLikeBoxing() cil managed
34+
{
35+
.maxstack 1
36+
.locals init (valuetype MyByRefLikeType V_0)
37+
ldloc.0
38+
box valuetype MyByRefLikeType
39+
pop
40+
ret
41+
}
42+
43+
.method private hidebysig static void ByRefLikeStaticField() cil managed
44+
{
45+
.maxstack 1
46+
ldsfld valuetype MyByRefLikeType ByRefLikeStaticField::s
47+
pop
48+
ret
49+
}
50+
51+
.method private hidebysig static void ByRefLikeFieldInNonValueType() cil managed
52+
{
53+
.maxstack 1
54+
ldnull
55+
ldfld valuetype MyByRefLikeType ByRefLikeFieldInNonValueType::f
56+
pop
57+
ret
58+
}
59+
60+
.method private hidebysig static void ByRefLikeFieldInNonByRefLikeType() cil managed
61+
{
62+
.maxstack 1
63+
.locals init (valuetype ByRefLikeFieldInNonByRefLikeType V_0)
64+
ldloc.0
65+
ldfld valuetype MyByRefLikeType ByRefLikeFieldInNonValueType::f
66+
pop
67+
ret
68+
}
69+
70+
.method private hidebysig static void ByRefLikeArray() cil managed
71+
{
72+
.maxstack 1
73+
ldc.i4.1
74+
newarr valuetype MyByRefLikeType
75+
pop
76+
ret
77+
}
78+
79+
.method private hidebysig static void ByRefLikeGenericInstantiation() cil managed
80+
{
81+
.maxstack 1
82+
newobj instance void class [System.Runtime]System.Collections.Generic.List`1<valuetype MyByRefLikeType>::.ctor()
83+
pop
84+
ret
85+
}
86+
87+
.method public hidebysig static int32 Main() cil managed
88+
{
89+
.entrypoint
90+
.maxstack 1
91+
92+
ldstr "ByRefLikeBoxing"
93+
call void [System.Console]System.Console::WriteLine(string)
94+
.try
95+
{
96+
call void Test::ByRefLikeBoxing()
97+
leave TestFailed
98+
}
99+
catch [System.Runtime]System.InvalidProgramException
100+
{
101+
pop
102+
leave ByRefLikeBoxing_Done
103+
}
104+
ByRefLikeBoxing_Done:
105+
106+
ldstr "ByRefLikeStaticField"
107+
call void [System.Console]System.Console::WriteLine(string)
108+
.try
109+
{
110+
call void Test::ByRefLikeStaticField()
111+
leave TestFailed
112+
}
113+
catch [System.Runtime]System.TypeLoadException
114+
{
115+
pop
116+
leave ByRefLikeStaticField_Done
117+
}
118+
ByRefLikeStaticField_Done:
119+
120+
ldstr "ByRefLikeFieldInNonValueType"
121+
call void [System.Console]System.Console::WriteLine(string)
122+
.try
123+
{
124+
call void Test::ByRefLikeFieldInNonValueType()
125+
leave TestFailed
126+
}
127+
catch [System.Runtime]System.TypeLoadException
128+
{
129+
pop
130+
leave ByRefLikeFieldInNonValueType_Done
131+
}
132+
ByRefLikeFieldInNonValueType_Done:
133+
134+
ldstr "ByRefLikeFieldInNonByRefLikeType"
135+
call void [System.Console]System.Console::WriteLine(string)
136+
.try
137+
{
138+
call void Test::ByRefLikeFieldInNonByRefLikeType()
139+
leave TestFailed
140+
}
141+
catch [System.Runtime]System.TypeLoadException
142+
{
143+
pop
144+
leave ByRefLikeFieldInNonByRefLikeType_Done
145+
}
146+
ByRefLikeFieldInNonByRefLikeType_Done:
147+
148+
ldstr "ByRefLikeArray"
149+
call void [System.Console]System.Console::WriteLine(string)
150+
.try
151+
{
152+
call void Test::ByRefLikeArray()
153+
leave TestFailed
154+
}
155+
catch [System.Runtime]System.TypeLoadException
156+
{
157+
pop
158+
leave ByRefLikeArray_Done
159+
}
160+
ByRefLikeArray_Done:
161+
162+
ldstr "ByRefLikeGenericInstantiation"
163+
call void [System.Console]System.Console::WriteLine(string)
164+
.try
165+
{
166+
call void Test::ByRefLikeGenericInstantiation()
167+
leave TestFailed
168+
}
169+
catch [System.Runtime]System.TypeLoadException
170+
{
171+
pop
172+
leave ByRefLikeGenericInstantiation_Done
173+
}
174+
ByRefLikeGenericInstantiation_Done:
175+
176+
ldstr "All Tests Passed"
177+
call void [System.Console]System.Console::WriteLine(string)
178+
ldc.i4.s 100
179+
ret
180+
TestFailed:
181+
ldstr "Test Failed"
182+
call void [System.Console]System.Console::WriteLine(string)
183+
ldc.i4.1
184+
ret
185+
}
186+
}

‎tests/src/Loader/classloader/regressions/GitHub_11371/Negative_ByRefLikeType.ilproj

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<AssemblyName>Negative_ByRefLikeType</AssemblyName>
6+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
7+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
10+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
11+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
12+
<OutputType>Exe</OutputType>
13+
<CLRTestKind>BuildAndRun</CLRTestKind>
14+
<CLRTestPriority>0</CLRTestPriority>
15+
</PropertyGroup>
16+
<ItemGroup>
17+
<Compile Include="Negative_ByRefLikeType.il" />
18+
</ItemGroup>
19+
<ItemGroup>
20+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
21+
</ItemGroup>
22+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
23+
</Project>

0 commit comments

Comments
 (0)