Skip to content

Commit 426d58f

Browse files
sbomersbomer
authored
Fix stack overflow in compiler-generated state (#109207)
When building the compiler-generated type info for a type, there are some cases that produce warnings due to unexpected IL. This can recurse back into the compiler-generated type info logic when checking for warning suppressions, leading to stack overflow. This fixes the issue by delaying the logging of warnings until after the compiler-generated type info has been fully built for a given type.
1 parent 6afd223 commit 426d58f

File tree

4 files changed

+242
-14
lines changed

4 files changed

+242
-14
lines changed

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/CompilerGeneratedState.cs

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,47 @@ private readonly record struct TypeArgumentInfo(
2929

3030
private readonly TypeCacheHashtable _typeCacheHashtable;
3131

32+
private readonly Logger _logger;
33+
3234
public CompilerGeneratedState(ILProvider ilProvider, Logger logger)
3335
{
34-
_typeCacheHashtable = new TypeCacheHashtable(ilProvider, logger);
36+
_typeCacheHashtable = new TypeCacheHashtable(ilProvider);
37+
_logger = logger;
3538
}
3639

3740
private sealed class TypeCacheHashtable : LockFreeReaderHashtable<MetadataType, TypeCache>
3841
{
3942
private ILProvider _ilProvider;
40-
private Logger? _logger;
4143

42-
public TypeCacheHashtable(ILProvider ilProvider, Logger logger) => (_ilProvider, _logger) = (ilProvider, logger);
44+
public TypeCacheHashtable(ILProvider ilProvider) => _ilProvider = ilProvider;
4345

4446
protected override bool CompareKeyToValue(MetadataType key, TypeCache value) => key == value.Type;
4547
protected override bool CompareValueToValue(TypeCache value1, TypeCache value2) => value1.Type == value2.Type;
4648
protected override int GetKeyHashCode(MetadataType key) => key.GetHashCode();
4749
protected override int GetValueHashCode(TypeCache value) => value.Type.GetHashCode();
4850

4951
protected override TypeCache CreateValueFromKey(MetadataType key)
50-
=> new TypeCache(key, _logger, _ilProvider);
52+
=> new TypeCache(key, _ilProvider);
53+
54+
public TypeCache GetOrCreateValue(MetadataType key, out bool created)
55+
{
56+
TypeCache existingValue;
57+
created = false;
58+
if (TryGetValue(key, out existingValue))
59+
return existingValue;
60+
61+
var newValue = CreateValueFromKey(key);
62+
if (TryAdd(newValue))
63+
{
64+
created = true;
65+
return newValue;
66+
}
67+
68+
if (!TryGetValue(key, out existingValue))
69+
throw new InvalidOperationException();
70+
71+
return existingValue;
72+
}
5173
}
5274

5375
private sealed class TypeCache
@@ -64,14 +86,26 @@ private sealed class TypeCache
6486
// or null if the type has no methods with compiler-generated members.
6587
private Dictionary<MethodDesc, List<TypeSystemEntity>>? _compilerGeneratedMembers;
6688

89+
// Stores a list of warnings to be emitted at the end of the cache construction
90+
private List<(MessageOrigin, DiagnosticId, string[])>? _warnings;
91+
92+
internal void LogWarnings(Logger? logger)
93+
{
94+
if (_warnings == null || logger == null)
95+
return;
96+
97+
foreach (var (origin, id, messageArgs) in _warnings)
98+
logger.LogWarning(origin, id, messageArgs);
99+
}
100+
67101
/// <summary>
68102
/// Walks the type and its descendents to find Roslyn-compiler generated
69103
/// code and gather information to map it back to original user code. If
70104
/// a compiler-generated type is passed in directly, this method will walk
71105
/// up and find the nearest containing user type. Returns the nearest user type,
72106
/// or null if none was found.
73107
/// </summary>
74-
internal TypeCache(MetadataType type, Logger? logger, ILProvider ilProvider)
108+
internal TypeCache(MetadataType type, ILProvider ilProvider)
75109
{
76110
Debug.Assert(type == type.GetTypeDefinition());
77111
Debug.Assert(!CompilerGeneratedNames.IsStateMachineOrDisplayClass(type.Name));
@@ -82,6 +116,15 @@ internal TypeCache(MetadataType type, Logger? logger, ILProvider ilProvider)
82116
var userDefinedMethods = new HashSet<MethodDesc>();
83117
var generatedTypeToTypeArgs = new Dictionary<MetadataType, TypeArgumentInfo>();
84118

119+
// We delay actually logging the warnings until the compiler-generated type info is
120+
// populated for this type, because the type info is needed to determine whether a warning
121+
// is suppressed.
122+
void AddWarning(MessageOrigin origin, DiagnosticId id, params string[] messageArgs)
123+
{
124+
_warnings ??= new List<(MessageOrigin, DiagnosticId, string[])>();
125+
_warnings.Add((origin, id, messageArgs));
126+
}
127+
85128
void ProcessMethod(MethodDesc method)
86129
{
87130
Debug.Assert(method == method.GetTypicalMethodDefinition());
@@ -139,7 +182,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
139182
if (!generatedTypeToTypeArgs.TryAdd(generatedType, new TypeArgumentInfo(method, null)))
140183
{
141184
var alreadyAssociatedMethod = generatedTypeToTypeArgs[generatedType].CreatingMethod;
142-
logger?.LogWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
185+
AddWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
143186
}
144187
continue;
145188
}
@@ -207,7 +250,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
207250
if (!_compilerGeneratedTypeToUserCodeMethod.TryAdd(stateMachineType, method))
208251
{
209252
var alreadyAssociatedMethod = _compilerGeneratedTypeToUserCodeMethod[stateMachineType];
210-
logger?.LogWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), stateMachineType.GetDisplayName());
253+
AddWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), stateMachineType.GetDisplayName());
211254
}
212255
// Already warned above if multiple methods map to the same type
213256
// Fill in null for argument providers now, the real providers will be filled in later
@@ -263,7 +306,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
263306
if (!_compilerGeneratedMethodToUserCodeMethod.TryAdd(nestedFunction, userDefinedMethod))
264307
{
265308
var alreadyAssociatedMethod = _compilerGeneratedMethodToUserCodeMethod[nestedFunction];
266-
logger?.LogWarning(new MessageOrigin(userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), nestedFunction.GetDisplayName());
309+
AddWarning(new MessageOrigin(userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), nestedFunction.GetDisplayName());
267310
}
268311
break;
269312
case MetadataType stateMachineType:
@@ -295,7 +338,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
295338
{
296339
var method = info.CreatingMethod;
297340
var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod;
298-
logger?.LogWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
341+
AddWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
299342
}
300343
}
301344
}
@@ -575,7 +618,10 @@ public static bool TryGetStateMachineType(MethodDesc method, [NotNullWhen(true)]
575618
if (userType is null)
576619
return null;
577620

578-
return _typeCacheHashtable.GetOrCreateValue(userType);
621+
var typeCache = _typeCacheHashtable.GetOrCreateValue(userType, out bool created);
622+
if (created)
623+
typeCache.LogWarnings(_logger);
624+
return typeCache;
579625
}
580626

581627
private static TypeDesc? GetFirstConstructorArgumentAsType(CustomAttributeValue<TypeDesc> attribute)

‎src/tools/illink/src/linker/Linker.Dataflow/CompilerGeneratedState.cs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,17 @@ public static bool TryGetStateMachineType (MethodDefinition method, [NotNullWhen
129129
var userDefinedMethods = new HashSet<MethodDefinition> ();
130130
var generatedTypeToTypeArgs = new Dictionary<TypeDefinition, TypeArgumentInfo> ();
131131

132+
List<(MessageOrigin, DiagnosticId, string[])>? _warnings = null;
133+
134+
// We delay actually logging the warnings until the compiler-generated type info is
135+
// populated for this type, because the type info is needed to determine whether a warning
136+
// is suppressed.
137+
void AddWarning (MessageOrigin origin, DiagnosticId id, params string[] messageArgs)
138+
{
139+
_warnings ??= new List<(MessageOrigin, DiagnosticId, string[])> ();
140+
_warnings.Add ((origin, id, messageArgs));
141+
}
142+
132143
void ProcessMethod (MethodDefinition method)
133144
{
134145
bool isStateMachineMember = CompilerGeneratedNames.IsStateMachineType (method.DeclaringType.Name);
@@ -163,7 +174,7 @@ referencedMethod.DeclaringType is var generatedType &&
163174
// fill in null for now, attribute providers will be filled in later
164175
if (!generatedTypeToTypeArgs.TryAdd (generatedType, new TypeArgumentInfo (method, null))) {
165176
var alreadyAssociatedMethod = generatedTypeToTypeArgs[generatedType].CreatingMethod;
166-
_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
177+
AddWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
167178
}
168179
continue;
169180
}
@@ -214,7 +225,7 @@ referencedMethod.DeclaringType is var generatedType &&
214225

215226
if (!_compilerGeneratedTypeToUserCodeMethod.TryAdd (stateMachineType, method)) {
216227
var alreadyAssociatedMethod = _compilerGeneratedTypeToUserCodeMethod[stateMachineType];
217-
_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), stateMachineType.GetDisplayName ());
228+
AddWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), stateMachineType.GetDisplayName ());
218229
}
219230
// Already warned above if multiple methods map to the same type
220231
// Fill in null for argument providers now, the real providers will be filled in later
@@ -265,7 +276,7 @@ referencedMethod.DeclaringType is var generatedType &&
265276
// Nested functions get suppressions from the user method only.
266277
if (!_compilerGeneratedMethodToUserCodeMethod.TryAdd (nestedFunction, userDefinedMethod)) {
267278
var alreadyAssociatedMethod = _compilerGeneratedMethodToUserCodeMethod[nestedFunction];
268-
_context.LogWarning (new MessageOrigin (userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), nestedFunction.GetDisplayName ());
279+
AddWarning (new MessageOrigin (userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), nestedFunction.GetDisplayName ());
269280
}
270281
break;
271282
case TypeDefinition stateMachineType:
@@ -292,12 +303,19 @@ referencedMethod.DeclaringType is var generatedType &&
292303
if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, info)) {
293304
var method = info.CreatingMethod;
294305
var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod;
295-
_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
306+
AddWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
296307
}
297308
}
298309
}
299310

300311
_cachedTypeToCompilerGeneratedMembers.Add (type, compilerGeneratedCallees);
312+
313+
if (_warnings != null) {
314+
foreach (var (origin, id, messageArgs) in _warnings) {
315+
_context.LogWarning (origin, id, messageArgs);
316+
}
317+
}
318+
301319
return type;
302320

303321
/// <summary>

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/Dependencies/MultipleMethodsUseSameAsyncStateMachine.il

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
.assembly extern System.Runtime
2+
{
3+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
4+
}
5+
6+
.assembly 'library'
7+
{
8+
.hash algorithm 0x00008004
9+
.ver 1:0:0:0
10+
}
11+
12+
.namespace Mono.Linker.Tests.Cases.Warnings.Dependencies
13+
{
14+
.class public auto ansi beforefieldinit MultipleMethodsUseSameAsyncStateMachine
15+
extends [System.Runtime]System.Object
16+
{
17+
// Nested Types
18+
.class nested public sequential ansi sealed beforefieldinit '<StateMachine>d'
19+
extends [System.Runtime]System.ValueType
20+
implements [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine
21+
{
22+
// Fields
23+
.field public static int32 'field'
24+
25+
// Methods
26+
.method public final hidebysig newslot virtual
27+
instance void MoveNext () cil managed
28+
{
29+
// Method begins at RVA 0x207a
30+
// Code size 9 (0x9)
31+
.maxstack 8
32+
33+
IL_0000: nop
34+
IL_0001: ldarg.0
35+
IL_0002: ldc.i4.1
36+
IL_0003: stfld int32 Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'::'field'
37+
IL_0008: ret
38+
} // end of method '<StateMachine>d'::MoveNext
39+
40+
.method private final hidebysig newslot virtual
41+
instance void System.Runtime.CompilerServices.IAsyncStateMachine.SetStateMachine (
42+
class [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine stateMachine
43+
) cil managed
44+
{
45+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = (
46+
01 00 01 00 00
47+
)
48+
.override method instance void [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine::SetStateMachine(class [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine)
49+
// Method begins at RVA 0x2050
50+
// Code size 2 (0x2)
51+
.maxstack 8
52+
53+
IL_0000: nop
54+
IL_0001: ret
55+
} // end of method '<StateMachine>d'::System.Runtime.CompilerServices.IAsyncStateMachine.SetStateMachine
56+
57+
} // end of class '<StateMachine>d'
58+
59+
60+
// Methods
61+
.method public hidebysig static
62+
void M () cil managed
63+
{
64+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.AsyncStateMachineAttribute::.ctor(class [System.Runtime]System.Type) = (
65+
01 00 65 4D 6F 6E 6F 2E 4C 69 6E 6B 65 72 2E 54
66+
65 73 74 73 2E 43 61 73 65 73 2E 57 61 72 6E 69
67+
6E 67 73 2E 44 65 70 65 6E 64 65 6E 63 69 65 73
68+
2E 4D 75 6C 74 69 70 6C 65 4D 65 74 68 6F 64 73
69+
55 73 65 53 61 6D 65 41 73 79 6E 63 53 74 61 74
70+
65 4D 61 63 68 69 6E 65 2B 3C 53 74 61 74 65 4D
71+
61 63 68 69 6E 65 3E 64 00 00
72+
)
73+
.maxstack 8
74+
75+
IL_0000: call void Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine::RUC();
76+
IL_0005: call void Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine::'<M>g__LocalFunction'()
77+
IL_000a: ret
78+
} // end of method MultipleMethodsUseSameAsyncStateMachine::M
79+
80+
.method private hidebysig static
81+
void RUC () cil managed
82+
{
83+
.custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute::.ctor(string) = (
84+
01 00 03 52 55 43 00 00
85+
)
86+
// Method begins at RVA 0x205c
87+
// Code size 1 (0x1)
88+
.maxstack 8
89+
90+
IL_0000: ret
91+
} // end of method MultipleMethodsUseSameAsyncStateMachine::RUC
92+
93+
.method public hidebysig static
94+
void '<M>g__LocalFunction' () cil managed
95+
{
96+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.AsyncStateMachineAttribute::.ctor(class [System.Runtime]System.Type) = (
97+
01 00 65 4D 6F 6E 6F 2E 4C 69 6E 6B 65 72 2E 54
98+
65 73 74 73 2E 43 61 73 65 73 2E 57 61 72 6E 69
99+
6E 67 73 2E 44 65 70 65 6E 64 65 6E 63 69 65 73
100+
2E 4D 75 6C 74 69 70 6C 65 4D 65 74 68 6F 64 73
101+
55 73 65 53 61 6D 65 41 73 79 6E 63 53 74 61 74
102+
65 4D 61 63 68 69 6E 65 2B 3C 53 74 61 74 65 4D
103+
61 63 68 69 6E 65 3E 64 00 00
104+
)
105+
.maxstack 2
106+
.locals init (
107+
[0] valuetype Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d' s
108+
)
109+
110+
IL_0000: nop
111+
IL_0001: ldloca.s 0
112+
IL_0003: initobj Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'
113+
IL_0009: ldloca.s 0
114+
IL_000b: ldc.i4.0
115+
IL_000c: stfld int32 Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'::'field'
116+
IL_0011: ldloca.s 0
117+
IL_0013: call instance void Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'::MoveNext()
118+
IL_0018: nop
119+
IL_0019: ret
120+
} // end of method MultipleMethodsUseSameAsyncStateMachine::'<M>g__LocalFunction'
121+
122+
.method public hidebysig specialname rtspecialname
123+
instance void .ctor () cil managed
124+
{
125+
// Method begins at RVA 0x2053
126+
// Code size 8 (0x8)
127+
.maxstack 8
128+
129+
IL_0000: ldarg.0
130+
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
131+
IL_0006: nop
132+
IL_0007: ret
133+
} // end of method MultipleMethodsUseSameAsyncStateMachine::.ctor
134+
135+
} // end of class Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine
136+
137+
}

0 commit comments

Comments
 (0)