Skip to content

Commit 260d02b

Browse files
Do not look at unboxing bit when looking up constructed generic methods (#118211)
Fixes #118072. In the bug we were ending up in a situation where we remember generic dictionary for a `MethodDesc` with `UnboxingStub` bit set to true and then try to look up a generic dictionary for a `MethodDesc` with unboxing stub bit set to false (the false is hardcoded in the code I'm updating). This is all sorts of wrong. The unboxing MethodDescs in the runtime type system were a mistake. We get by without them in the compiler. This is really difficult to hit because the runtime type system is mutable (this is yet another mistake) and if we still have a cached version of this mutable type system around, this lookup is not used. The regression test I added is really annoying and takes like 40 seconds to run in checked builds. I hate it, but this is not hittable in any other way. I'm not sure if this one is 100% reliable at triggering the type system flush either.
1 parent abc4c65 commit 260d02b

File tree

3 files changed

+91
-8
lines changed

3 files changed

+91
-8
lines changed

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericMethodsLookup.cs

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,28 +146,53 @@ internal override bool MatchParsedEntry(ref NativeParser entryParser, ref Extern
146146
TypeSystemContext context = _methodToLookup.Context;
147147

148148
RuntimeTypeHandle parsedDeclaringTypeHandle = externalReferencesLookup.GetRuntimeTypeHandleFromIndex(entryParser.GetUnsigned());
149+
TypeDesc parsedDeclaringType = context.ResolveRuntimeTypeHandle(parsedDeclaringTypeHandle);
150+
if (_methodToLookup.OwningType != parsedDeclaringType)
151+
return false;
149152

150153
// Hash table names / sigs are indirected through to the native layout info
151154
MethodNameAndSignature nameAndSignature = TypeLoaderEnvironment.GetMethodNameAndSignatureFromToken(moduleHandle, entryParser.GetUnsigned());
155+
if (!_methodToLookup.NameAndSignature.Equals(nameAndSignature))
156+
return false;
152157

153158
RuntimeTypeHandle[] parsedArgsHandles = GetTypeSequence(ref externalReferencesLookup, ref entryParser);
159+
if (parsedArgsHandles.Length != _methodToLookup.Instantiation.Length)
160+
return false;
154161

155-
DefType parsedDeclaringType = context.ResolveRuntimeTypeHandle(parsedDeclaringTypeHandle) as DefType;
156-
Instantiation parsedArgs = context.ResolveRuntimeTypeHandles(parsedArgsHandles);
157-
InstantiatedMethod parsedGenericMethod = (InstantiatedMethod)context.ResolveGenericMethodInstantiation(false, parsedDeclaringType, nameAndSignature, parsedArgs);
162+
for (int i = 0; i < _methodToLookup.Instantiation.Length; i++)
163+
{
164+
TypeDesc leftType = context.ResolveRuntimeTypeHandle(parsedArgsHandles[i]);
165+
TypeDesc rightType = _methodToLookup.Instantiation[i];
166+
if (leftType != rightType)
167+
return false;
168+
}
158169

159-
return parsedGenericMethod == _methodToLookup;
170+
return true;
160171
}
161172

162173
internal override bool MatchGenericMethodEntry(GenericMethodEntry entry)
163174
{
164175
TypeSystemContext context = _methodToLookup.Context;
165176

166-
DefType parsedDeclaringType = context.ResolveRuntimeTypeHandle(entry._declaringTypeHandle) as DefType;
167-
Instantiation parsedArgs = context.ResolveRuntimeTypeHandles(entry._genericMethodArgumentHandles);
168-
InstantiatedMethod parsedGenericMethod = (InstantiatedMethod)context.ResolveGenericMethodInstantiation(false, parsedDeclaringType, entry._methodNameAndSignature, parsedArgs);
177+
TypeDesc parsedDeclaringType = context.ResolveRuntimeTypeHandle(entry._declaringTypeHandle);
178+
if (_methodToLookup.OwningType != parsedDeclaringType)
179+
return false;
169180

170-
return parsedGenericMethod == _methodToLookup;
181+
if (!_methodToLookup.NameAndSignature.Equals(entry._methodNameAndSignature))
182+
return false;
183+
184+
if (entry._genericMethodArgumentHandles.Length != _methodToLookup.Instantiation.Length)
185+
return false;
186+
187+
for (int i = 0; i < _methodToLookup.Instantiation.Length; i++)
188+
{
189+
TypeDesc leftType = context.ResolveRuntimeTypeHandle(entry._genericMethodArgumentHandles[i]);
190+
TypeDesc rightType = _methodToLookup.Instantiation[i];
191+
if (leftType != rightType)
192+
return false;
193+
}
194+
195+
return true;
171196
}
172197
}
173198

src/tests/nativeaot/SmokeTests/DynamicGenerics/DynamicGenerics.main.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ public static int Main(string[] args)
152152
#if UNIVERSAL_GENERICS
153153
new CoreFXTestLibrary.Internal.TestInfo("B279085.TestB279085Repro", () => global::B279085.TestB279085Repro(), null),
154154
#endif
155+
new CoreFXTestLibrary.Internal.TestInfo("GitHub118072.RunTest", () => global::GitHub118072.RunTest(), null),
155156
new CoreFXTestLibrary.Internal.TestInfo("GenericVirtualMethods.TestCalls", () => global::GenericVirtualMethods.TestCalls(), null),
156157
new CoreFXTestLibrary.Internal.TestInfo("GenericVirtualMethods.TestLdFtnToGetStaticMethodOnGenericType", () => global::GenericVirtualMethods.TestLdFtnToGetStaticMethodOnGenericType(), null),
157158
new CoreFXTestLibrary.Internal.TestInfo("GenericVirtualMethods.TestLdFtnToInstanceGenericMethod", () => global::GenericVirtualMethods.TestLdFtnToInstanceGenericMethod(), null),
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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+
4+
using System;
5+
using System.Reflection;
6+
using System.Runtime.CompilerServices;
7+
8+
using CoreFXTestLibrary;
9+
10+
class GitHub118072
11+
{
12+
[MethodImpl(MethodImplOptions.NoInlining)]
13+
static MethodInfo GetMI1() => typeof(GitHub118072).GetMethod(nameof(CallMethod1));
14+
[MethodImpl(MethodImplOptions.NoInlining)]
15+
static MethodInfo GetMI2() => typeof(GitHub118072).GetMethod(nameof(CallMethod2));
16+
[MethodImpl(MethodImplOptions.NoInlining)]
17+
static MethodInfo GetMI3() => typeof(GitHub118072).GetMethod(nameof(CallMethod1));
18+
[MethodImpl(MethodImplOptions.NoInlining)]
19+
static MethodInfo GetMI4() => typeof(GitHub118072).GetMethod(nameof(CallMethod2));
20+
21+
[TestMethod]
22+
public static void RunTest()
23+
{
24+
Type current = typeof(object);
25+
26+
GetMI1().MakeGenericMethod(typeof(object)).Invoke(null, []);
27+
current = FillCache(current);
28+
GetMI2().MakeGenericMethod(typeof(object)).Invoke(null, []);
29+
current = FillCache(current);
30+
GetMI3().MakeGenericMethod(typeof(object)).Invoke(null, []);
31+
current = FillCache(current);
32+
GetMI4().MakeGenericMethod(typeof(object)).Invoke(null, []);
33+
34+
static Type FillCache(Type current)
35+
{
36+
for (int i = 0; i < 400; i++)
37+
{
38+
Type next = typeof(MyClass<>).MakeGenericType(current);
39+
Activator.CreateInstance(next);
40+
current = next;
41+
}
42+
43+
return current;
44+
}
45+
}
46+
47+
public static string CallMethod1<T>() => default(MyStruct).Method<T>();
48+
49+
public static string CallMethod2<T>() => default(MyStruct).Method<T>();
50+
51+
struct MyStruct
52+
{
53+
public string Method<T>() => typeof(T).Name;
54+
}
55+
56+
class MyClass<T> { }
57+
}

0 commit comments

Comments
 (0)