Skip to content

Commit fea7ff2

Browse files
davidwrightondavidwrighton
authored
Fix issues with virtuals and mdarrays (#53400)
- Change up the definition of an MDArray which doesn't need a special descriptor in the embedded signature data to match the type of mdarray thay everyone always uses. This fixes the virtual function handling behavior for all reasonable cases. (All cases where the MDArray is used for a base type in the expected fashion.) - Add a devirtualization test for this specific scenario Fixes #52444
1 parent 8538a75 commit fea7ff2

File tree

6 files changed

+361
-5
lines changed

6 files changed

+361
-5
lines changed

‎src/coreclr/tools/Common/TypeSystem/Ecma/EcmaSignatureParser.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,16 @@ private TypeDesc ParseTypeImpl(SignatureTypeCode typeCode)
169169

170170
var lowerBoundsCount = _reader.ReadCompressedInteger();
171171
int []lowerBounds = lowerBoundsCount > 0 ? new int[lowerBoundsCount] : Array.Empty<int>();
172+
bool nonZeroLowerBounds = false;
172173
for (int j = 0; j < lowerBoundsCount; j++)
173-
lowerBounds[j] = _reader.ReadCompressedSignedInteger();
174+
{
175+
int loBound = _reader.ReadCompressedSignedInteger();
176+
if (loBound != 0)
177+
nonZeroLowerBounds = true;
178+
lowerBounds[j] = loBound;
179+
}
174180

175-
if (boundsCount != 0 || lowerBoundsCount != 0)
181+
if (boundsCount != 0 || lowerBoundsCount != rank || nonZeroLowerBounds)
176182
{
177183
StringBuilder arrayShapeString = new StringBuilder();
178184
arrayShapeString.Append(string.Join(",", bounds));

‎src/coreclr/tools/Common/TypeSystem/MetadataEmitter/TypeSystemMetadataEmitter.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,10 +424,29 @@ public void EmitArrayShapeAtCurrentIndexStack(BlobBuilder signatureBuilder, int
424424

425425
if (!emittedWithShape)
426426
{
427-
shapeEncoder.Shape(rank, ImmutableArray<int>.Empty, ImmutableArray<int>.Empty);
427+
shapeEncoder.Shape(rank, ImmutableArray<int>.Empty, GetZeroedImmutableArrayOfSize(rank));
428428
}
429429
}
430430

431+
private static ImmutableArray<int>[] ImmutableArraysFilledWithZeroes = CreateStaticArrayOfImmutableArraysFilledWithZeroes(33); // The max rank of an array is 32
432+
433+
private static ImmutableArray<int> GetZeroedImmutableArrayOfSize(int rank)
434+
{
435+
if (rank < ImmutableArraysFilledWithZeroes.Length)
436+
return ImmutableArraysFilledWithZeroes[rank];
437+
438+
return new int[rank].ToImmutableArray();
439+
}
440+
private static ImmutableArray<int>[] CreateStaticArrayOfImmutableArraysFilledWithZeroes(int count)
441+
{
442+
ImmutableArray<int>[] result = new ImmutableArray<int>[count];
443+
for (int i = 0; i < result.Length; i++)
444+
{
445+
result[i] = new int[i].ToImmutableArray();
446+
}
447+
return result;
448+
}
449+
431450
public void EmitAtCurrentIndexStack(BlobBuilder signatureBuilder)
432451
{
433452
if (!Complete)

‎src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILTestAssembly/Signature.il‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@
3434
{
3535
ret
3636
}
37+
38+
.method public hidebysig instance int32 [0...,0...] Method5(int32 [0...,], int32 [,]) cil managed
39+
{
40+
ret
41+
}
3742
}
3843

3944
.class private auto ansi beforefieldinit Atom

‎src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/SignatureTests.cs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,27 @@ public void TestSignatureMatchesForArrayShapeDetails()
9595
MetadataType modOptTester = _testModule.GetType("", "ModOptTester");
9696
MethodSignature methodWithModOpt = modOptTester.GetMethods().Single(m => string.Equals(m.Name, "Method4")).Signature;
9797

98-
Assert.Equal(6, methodWithModOpt.GetEmbeddedSignatureData().Length);
98+
_output.WriteLine($"Found ModOptData '{GetModOptMethodSignatureInfo(methodWithModOpt)}'");
99+
Assert.Equal(7, methodWithModOpt.GetEmbeddedSignatureData().Length);
99100
Assert.Equal(MethodSignature.GetIndexOfCustomModifierOnPointedAtTypeByParameterIndex(0), methodWithModOpt.GetEmbeddedSignatureData()[0].index);
100101
Assert.Equal(MethodSignature.GetIndexOfCustomModifierOnPointedAtTypeByParameterIndex(1), methodWithModOpt.GetEmbeddedSignatureData()[2].index);
101102
Assert.Equal(MethodSignature.GetIndexOfCustomModifierOnPointedAtTypeByParameterIndex(2), methodWithModOpt.GetEmbeddedSignatureData()[4].index);
102-
Assert.Equal("OptionalCustomModifier0.1.1.2.1.1VoidArrayShape1.2.1.1|3,4|0,1<null>OptionalCustomModifier0.1.1.2.2.1FooModifierArrayShape1.2.2.1|0,9|2,0<null>OptionalCustomModifier0.1.1.2.3.1FooModifierArrayShape1.2.3.1||0<null>", GetModOptMethodSignatureInfo(methodWithModOpt));
103+
Assert.Equal("OptionalCustomModifier0.1.1.2.1.1VoidArrayShape1.2.1.1|3,4|0,1<null>OptionalCustomModifier0.1.1.2.2.1FooModifierArrayShape1.2.2.1|0,9|2,0<null>OptionalCustomModifier0.1.1.2.3.1FooModifierArrayShape1.2.3.1||0<null>ArrayShape1.2.4.1||<null>", GetModOptMethodSignatureInfo(methodWithModOpt));
104+
}
105+
106+
[Fact]
107+
public void TestSignatureMatchesForArrayShapeDetails_HandlingOfCasesWhichDoNotNeedEmbeddeSignatureData()
108+
{
109+
// Test that ensure the typical case (where the loBounds is 0, and the hibounds is unspecified) doesn't produce an
110+
// EmbeddedSignatureData, but that other cases do. This isn't a complete test as ilasm won't actually properly generate the metadata for many of these cases
111+
MetadataType modOptTester = _testModule.GetType("", "ModOptTester");
112+
MethodSignature methodWithModOpt = modOptTester.GetMethods().Single(m => string.Equals(m.Name, "Method5")).Signature;
113+
114+
_output.WriteLine($"Found ModOptData '{GetModOptMethodSignatureInfo(methodWithModOpt)}'");
115+
Assert.Equal(2, methodWithModOpt.GetEmbeddedSignatureData().Length);
116+
Assert.EndsWith(methodWithModOpt.GetEmbeddedSignatureData()[0].index.Split('|')[0], MethodSignature.GetIndexOfCustomModifierOnPointedAtTypeByParameterIndex(1));
117+
Assert.EndsWith(methodWithModOpt.GetEmbeddedSignatureData()[1].index.Split('|')[0], MethodSignature.GetIndexOfCustomModifierOnPointedAtTypeByParameterIndex(2));
118+
Assert.Equal("ArrayShape1.2.2.1||0<null>ArrayShape1.2.3.1||<null>", GetModOptMethodSignatureInfo(methodWithModOpt));
103119
}
104120

105121
[Fact]

‎src/tests/JIT/opt/Devirtualization/GenericArrayOverride.il‎

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
2+
// Licensed to the .NET Foundation under one or more agreements.
3+
// The .NET Foundation licenses this file to you under the MIT license.
4+
5+
6+
// Metadata version: v4.0.30319
7+
.assembly extern System.Runtime
8+
{
9+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
10+
.ver 5:0:0:0
11+
}
12+
.assembly extern System.Collections
13+
{
14+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
15+
.ver 5:0:0:0
16+
}
17+
.assembly extern System.Console
18+
{
19+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
20+
.ver 5:0:0:0
21+
}
22+
.assembly GenericArrayOverride
23+
{
24+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 )
25+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78 // ....T..WrapNonEx
26+
63 65 70 74 69 6F 6E 54 68 72 6F 77 73 01 ) // ceptionThrows.
27+
28+
// --- The following custom attribute is added automatically, do not uncomment -------
29+
// .custom instance void [System.Runtime]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.DebuggableAttribute/DebuggingModes) = ( 01 00 07 01 00 00 00 00 )
30+
31+
.custom instance void [System.Runtime]System.Runtime.Versioning.TargetFrameworkAttribute::.ctor(string) = ( 01 00 18 2E 4E 45 54 43 6F 72 65 41 70 70 2C 56 // ....NETCoreApp,V
32+
65 72 73 69 6F 6E 3D 76 35 2E 30 01 00 54 0E 14 // ersion=v5.0..T..
33+
46 72 61 6D 65 77 6F 72 6B 44 69 73 70 6C 61 79 // FrameworkDisplay
34+
4E 61 6D 65 00 ) // Name.
35+
.custom instance void [System.Runtime]System.Reflection.AssemblyCompanyAttribute::.ctor(string) = ( 01 00 14 47 65 6E 65 72 69 63 41 72 72 61 79 4F // ...GenericArrayO
36+
76 65 72 72 69 64 65 00 00 ) // verride..
37+
.custom instance void [System.Runtime]System.Reflection.AssemblyConfigurationAttribute::.ctor(string) = ( 01 00 05 44 65 62 75 67 00 00 ) // ...Debug..
38+
.custom instance void [System.Runtime]System.Reflection.AssemblyFileVersionAttribute::.ctor(string) = ( 01 00 07 31 2E 30 2E 30 2E 30 00 00 ) // ...1.0.0.0..
39+
.custom instance void [System.Runtime]System.Reflection.AssemblyInformationalVersionAttribute::.ctor(string) = ( 01 00 05 31 2E 30 2E 30 00 00 ) // ...1.0.0..
40+
.custom instance void [System.Runtime]System.Reflection.AssemblyProductAttribute::.ctor(string) = ( 01 00 14 47 65 6E 65 72 69 63 41 72 72 61 79 4F // ...GenericArrayO
41+
76 65 72 72 69 64 65 00 00 ) // verride..
42+
.custom instance void [System.Runtime]System.Reflection.AssemblyTitleAttribute::.ctor(string) = ( 01 00 14 47 65 6E 65 72 69 63 41 72 72 61 79 4F // ...GenericArrayO
43+
76 65 72 72 69 64 65 00 00 ) // verride..
44+
.hash algorithm 0x00008004
45+
.ver 1:0:0:0
46+
}
47+
.module GenericArrayOverride.dll
48+
// MVID: {36F29C33-E2D3-47BA-B888-D30052D5C374}
49+
.imagebase 0x00400000
50+
.file alignment 0x00000200
51+
.stackreserve 0x00100000
52+
.subsystem 0x0003 // WINDOWS_CUI
53+
.corflags 0x00000001 // ILONLY
54+
// Image base: 0x06DD0000
55+
56+
57+
// =============== CLASS MEMBERS DECLARATION ===================
58+
59+
.class private auto ansi beforefieldinit GenericArrayOverride.Base`1<T>
60+
extends [System.Runtime]System.Object
61+
{
62+
.method public hidebysig newslot virtual
63+
instance string Function(!T param) cil managed
64+
{
65+
// Code size 11 (0xb)
66+
.maxstack 1
67+
.locals init (string V_0)
68+
IL_0000: nop
69+
IL_0001: ldstr "Base"
70+
IL_0006: stloc.0
71+
IL_0007: br.s IL_0009
72+
73+
IL_0009: ldloc.0
74+
IL_000a: ret
75+
} // end of method Base`1::Function
76+
77+
.method public hidebysig specialname rtspecialname
78+
instance void .ctor() cil managed
79+
{
80+
// Code size 8 (0x8)
81+
.maxstack 8
82+
IL_0000: ldarg.0
83+
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
84+
IL_0006: nop
85+
IL_0007: ret
86+
} // end of method Base`1::.ctor
87+
88+
} // end of class GenericArrayOverride.Base`1
89+
90+
.class interface private abstract auto ansi GenericArrayOverride.IFace`1<T>
91+
{
92+
.method public hidebysig newslot abstract virtual
93+
instance string IFaceFunction(class [System.Collections]System.Collections.Generic.List`1<!T> param) cil managed
94+
{
95+
} // end of method IFace`1::IFaceFunction
96+
97+
} // end of class GenericArrayOverride.IFace`1
98+
99+
.class private auto ansi beforefieldinit GenericArrayOverride.Derived
100+
extends class GenericArrayOverride.Base`1<class GenericArrayOverride.Derived[0...,0...,0...,0...]>
101+
implements class GenericArrayOverride.IFace`1<class GenericArrayOverride.Derived[0...,0...]>
102+
{
103+
.method public hidebysig virtual instance string
104+
Function(class GenericArrayOverride.Derived[1...,0...,0...,0...] param) cil managed
105+
{
106+
// Code size 11 (0xb)
107+
.maxstack 1
108+
.locals init (string V_0)
109+
IL_0000: nop
110+
IL_0001: ldstr "DerivedWrong1"
111+
IL_0006: stloc.0
112+
IL_0007: br.s IL_0009
113+
114+
IL_0009: ldloc.0
115+
IL_000a: ret
116+
} // end of method Derived::Function
117+
118+
.method public hidebysig virtual instance string
119+
Function(class GenericArrayOverride.Derived[0...,0...,0...,0...] param) cil managed
120+
{
121+
// Code size 11 (0xb)
122+
.maxstack 1
123+
.locals init (string V_0)
124+
IL_0000: nop
125+
IL_0001: ldstr "Derived"
126+
IL_0006: stloc.0
127+
IL_0007: br.s IL_0009
128+
129+
IL_0009: ldloc.0
130+
IL_000a: ret
131+
} // end of method Derived::Function
132+
133+
.method public hidebysig virtual instance string
134+
Function(class GenericArrayOverride.Derived[2...,0...,0...,0...] param) cil managed
135+
{
136+
// Code size 11 (0xb)
137+
.maxstack 1
138+
.locals init (string V_0)
139+
IL_0000: nop
140+
IL_0001: ldstr "DerivedWrong2"
141+
IL_0006: stloc.0
142+
IL_0007: br.s IL_0009
143+
144+
IL_0009: ldloc.0
145+
IL_000a: ret
146+
} // end of method Derived::Function
147+
148+
.method public hidebysig newslot virtual
149+
instance string IFaceFunction(class [System.Collections]System.Collections.Generic.List`1<class GenericArrayOverride.Derived[1...,0...]> param) cil managed
150+
{
151+
// Code size 11 (0xb)
152+
.maxstack 1
153+
.locals init (string V_0)
154+
IL_0000: nop
155+
IL_0001: ldstr "IFaceFuncWrong1"
156+
IL_0006: stloc.0
157+
IL_0007: br.s IL_0009
158+
159+
IL_0009: ldloc.0
160+
IL_000a: ret
161+
} // end of method Derived::IFaceFunction
162+
163+
.method public hidebysig newslot virtual
164+
instance string IFaceFunction(class [System.Collections]System.Collections.Generic.List`1<class GenericArrayOverride.Derived[0...,0...]> param) cil managed
165+
{
166+
// Code size 11 (0xb)
167+
.maxstack 1
168+
.locals init (string V_0)
169+
IL_0000: nop
170+
IL_0001: ldstr "IFaceFunc"
171+
IL_0006: stloc.0
172+
IL_0007: br.s IL_0009
173+
174+
IL_0009: ldloc.0
175+
IL_000a: ret
176+
} // end of method Derived::IFaceFunction
177+
178+
.method public hidebysig newslot virtual
179+
instance string IFaceFunction(class [System.Collections]System.Collections.Generic.List`1<class GenericArrayOverride.Derived[2...,0...]> param) cil managed
180+
{
181+
// Code size 11 (0xb)
182+
.maxstack 1
183+
.locals init (string V_0)
184+
IL_0000: nop
185+
IL_0001: ldstr "IFaceFuncWrong2"
186+
IL_0006: stloc.0
187+
IL_0007: br.s IL_0009
188+
189+
IL_0009: ldloc.0
190+
IL_000a: ret
191+
} // end of method Derived::IFaceFunction
192+
193+
.method public hidebysig specialname rtspecialname
194+
instance void .ctor() cil managed
195+
{
196+
// Code size 8 (0x8)
197+
.maxstack 8
198+
IL_0000: ldarg.0
199+
IL_0001: call instance void class GenericArrayOverride.Base`1<class GenericArrayOverride.Derived[0...,0...,0...,0...]>::.ctor()
200+
IL_0006: nop
201+
IL_0007: ret
202+
} // end of method Derived::.ctor
203+
204+
} // end of class GenericArrayOverride.Derived
205+
206+
.class private auto ansi beforefieldinit GenericArrayOverride.Program
207+
extends [System.Runtime]System.Object
208+
{
209+
.method private hidebysig static int32
210+
Main() cil managed
211+
{
212+
.entrypoint
213+
// Code size 120 (0x78)
214+
.maxstack 5
215+
.locals init (class GenericArrayOverride.Base`1<class GenericArrayOverride.Derived[0...,0...,0...,0...]> V_0,
216+
class GenericArrayOverride.IFace`1<class GenericArrayOverride.Derived[0...,0...]> V_1,
217+
bool V_2,
218+
int32 V_3,
219+
bool V_4)
220+
IL_0000: nop
221+
IL_0001: newobj instance void GenericArrayOverride.Derived::.ctor()
222+
IL_0006: stloc.0
223+
IL_0007: ldloc.0
224+
IL_0008: castclass class GenericArrayOverride.IFace`1<class GenericArrayOverride.Derived[0...,0...]>
225+
IL_000d: stloc.1
226+
IL_000e: ldloc.0
227+
IL_000f: ldc.i4.1
228+
IL_0010: ldc.i4.1
229+
IL_0011: ldc.i4.1
230+
IL_0012: ldc.i4.1
231+
IL_0013: newobj instance void class GenericArrayOverride.Derived[0...,0...,0...,0...]::.ctor(int32,
232+
int32,
233+
int32,
234+
int32)
235+
IL_0018: callvirt instance string class GenericArrayOverride.Base`1<class GenericArrayOverride.Derived[0...,0...,0...,0...]>::Function(!0)
236+
IL_001d: ldstr "Derived"
237+
IL_0022: call bool [System.Runtime]System.String::op_Inequality(string,
238+
string)
239+
IL_0027: stloc.2
240+
IL_0028: ldloc.2
241+
IL_0029: brfalse.s IL_003b
242+
243+
IL_002b: nop
244+
IL_002c: ldstr "Virtual override failed"
245+
IL_0031: call void [System.Console]System.Console::WriteLine(string)
246+
IL_0036: nop
247+
IL_0037: ldc.i4.1
248+
IL_0038: stloc.3
249+
IL_0039: br.s IL_0076
250+
251+
IL_003b: ldloc.1
252+
IL_003c: newobj instance void class [System.Collections]System.Collections.Generic.List`1<class GenericArrayOverride.Derived[0...,0...]>::.ctor()
253+
IL_0041: callvirt instance string class GenericArrayOverride.IFace`1<class GenericArrayOverride.Derived[0...,0...]>::IFaceFunction(class [System.Collections]System.Collections.Generic.List`1<!0>)
254+
IL_0046: ldstr "IFaceFunc"
255+
IL_004b: call bool [System.Runtime]System.String::op_Inequality(string,
256+
string)
257+
IL_0050: stloc.s V_4
258+
IL_0052: ldloc.s V_4
259+
IL_0054: brfalse.s IL_0066
260+
261+
IL_0056: nop
262+
IL_0057: ldstr "Interface override failed"
263+
IL_005c: call void [System.Console]System.Console::WriteLine(string)
264+
IL_0061: nop
265+
IL_0062: ldc.i4.2
266+
IL_0063: stloc.3
267+
IL_0064: br.s IL_0076
268+
269+
IL_0066: ldstr "PASS"
270+
IL_006b: call void [System.Console]System.Console::WriteLine(string)
271+
IL_0070: nop
272+
IL_0071: ldc.i4.s 100
273+
IL_0073: stloc.3
274+
IL_0074: br.s IL_0076
275+
276+
IL_0076: ldloc.3
277+
IL_0077: ret
278+
} // end of method Program::Main
279+
280+
.method public hidebysig specialname rtspecialname
281+
instance void .ctor() cil managed
282+
{
283+
// Code size 8 (0x8)
284+
.maxstack 8
285+
IL_0000: ldarg.0
286+
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
287+
IL_0006: nop
288+
IL_0007: ret
289+
} // end of method Program::.ctor
290+
291+
} // end of class GenericArrayOverride.Program
292+
293+
294+
// =============================================================
295+
296+
// *********** DISASSEMBLY COMPLETE ***********************
297+
// WARNING: Created Win32 resource file C:\git\runtime\src\tests\JIT\opt\Devirtualization\GenericArrayOverride.res

0 commit comments

Comments
 (0)