Skip to content

Fix stack overflow in compiler-generated state #109207

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 17, 2025
Merged

Fix stack overflow in compiler-generated state #109207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc55777
Fix stack overflow in CompilerGeneratedState
sbomer Oct 24, 2024
7489de5
Trigger stack overflow in ILC
sbomer Oct 24, 2024
6ab3283
Fix issue same way for ILC and ILLink
sbomer Jan 6, 2025
59f8df7
PR feedback
sbomer Jan 8, 2025
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
66 changes: 56 additions & 10 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/CompilerGeneratedState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,47 @@ private readonly record struct TypeArgumentInfo(

private readonly TypeCacheHashtable _typeCacheHashtable;

private readonly Logger _logger;

public CompilerGeneratedState(ILProvider ilProvider, Logger logger)
{
_typeCacheHashtable = new TypeCacheHashtable(ilProvider, logger);
_typeCacheHashtable = new TypeCacheHashtable(ilProvider);
_logger = logger;
}

private sealed class TypeCacheHashtable : LockFreeReaderHashtable<MetadataType, TypeCache>
{
private ILProvider _ilProvider;
private Logger? _logger;

public TypeCacheHashtable(ILProvider ilProvider, Logger logger) => (_ilProvider, _logger) = (ilProvider, logger);
public TypeCacheHashtable(ILProvider ilProvider) => _ilProvider = ilProvider;

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

protected override TypeCache CreateValueFromKey(MetadataType key)
=> new TypeCache(key, _logger, _ilProvider);
=> new TypeCache(key, _ilProvider);

public TypeCache GetOrCreateValue(MetadataType key, out bool created)
{
TypeCache existingValue;
created = false;
if (TryGetValue(key, out existingValue))
return existingValue;

var newValue = CreateValueFromKey(key);
if (TryAdd(newValue))
{
created = true;
return newValue;
}

if (!TryGetValue(key, out existingValue))
throw new InvalidOperationException();

return existingValue;
}
}

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

// Stores a list of warnings to be emitted at the end of the cache construction
private List<(MessageOrigin, DiagnosticId, string[])>? _warnings;

internal void LogWarnings(Logger? logger)
{
if (_warnings == null || logger == null)
return;

foreach (var (origin, id, messageArgs) in _warnings)
logger.LogWarning(origin, id, messageArgs);
}

/// <summary>
/// Walks the type and its descendents to find Roslyn-compiler generated
/// code and gather information to map it back to original user code. If
/// a compiler-generated type is passed in directly, this method will walk
/// up and find the nearest containing user type. Returns the nearest user type,
/// or null if none was found.
/// </summary>
internal TypeCache(MetadataType type, Logger? logger, ILProvider ilProvider)
internal TypeCache(MetadataType type, ILProvider ilProvider)
{
Debug.Assert(type == type.GetTypeDefinition());
Debug.Assert(!CompilerGeneratedNames.IsStateMachineOrDisplayClass(type.Name));
Expand All @@ -82,6 +116,15 @@ internal TypeCache(MetadataType type, Logger? logger, ILProvider ilProvider)
var userDefinedMethods = new HashSet<MethodDesc>();
var generatedTypeToTypeArgs = new Dictionary<MetadataType, TypeArgumentInfo>();

// We delay actually logging the warnings until the compiler-generated type info is
// populated for this type, because the type info is needed to determine whether a warning
// is suppressed.
void AddWarning(MessageOrigin origin, DiagnosticId id, params string[] messageArgs)
{
_warnings ??= new List<(MessageOrigin, DiagnosticId, string[])>();
_warnings.Add((origin, id, messageArgs));
}

void ProcessMethod(MethodDesc method)
{
Debug.Assert(method == method.GetTypicalMethodDefinition());
Expand Down Expand Up @@ -139,7 +182,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
if (!generatedTypeToTypeArgs.TryAdd(generatedType, new TypeArgumentInfo(method, null)))
{
var alreadyAssociatedMethod = generatedTypeToTypeArgs[generatedType].CreatingMethod;
logger?.LogWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
AddWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
}
continue;
}
Expand Down Expand Up @@ -207,7 +250,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
if (!_compilerGeneratedTypeToUserCodeMethod.TryAdd(stateMachineType, method))
{
var alreadyAssociatedMethod = _compilerGeneratedTypeToUserCodeMethod[stateMachineType];
logger?.LogWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), stateMachineType.GetDisplayName());
AddWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), stateMachineType.GetDisplayName());
}
// Already warned above if multiple methods map to the same type
// Fill in null for argument providers now, the real providers will be filled in later
Expand Down Expand Up @@ -263,7 +306,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
if (!_compilerGeneratedMethodToUserCodeMethod.TryAdd(nestedFunction, userDefinedMethod))
{
var alreadyAssociatedMethod = _compilerGeneratedMethodToUserCodeMethod[nestedFunction];
logger?.LogWarning(new MessageOrigin(userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), nestedFunction.GetDisplayName());
AddWarning(new MessageOrigin(userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), nestedFunction.GetDisplayName());
}
break;
case MetadataType stateMachineType:
Expand Down Expand Up @@ -295,7 +338,7 @@ referencedMethod.OwningType is MetadataType generatedType &&
{
var method = info.CreatingMethod;
var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod;
logger?.LogWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
AddWarning(new MessageOrigin(method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName(), alreadyAssociatedMethod.GetDisplayName(), generatedType.GetDisplayName());
}
}
}
Expand Down Expand Up @@ -575,7 +618,10 @@ public static bool TryGetStateMachineType(MethodDesc method, [NotNullWhen(true)]
if (userType is null)
return null;

return _typeCacheHashtable.GetOrCreateValue(userType);
var typeCache = _typeCacheHashtable.GetOrCreateValue(userType, out bool created);
if (created)
typeCache.LogWarnings(_logger);
return typeCache;
}

private static TypeDesc? GetFirstConstructorArgumentAsType(CustomAttributeValue<TypeDesc> attribute)
Expand Down
26 changes: 22 additions & 4 deletions src/tools/illink/src/linker/Linker.Dataflow/CompilerGeneratedState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,17 @@ public static bool TryGetStateMachineType (MethodDefinition method, [NotNullWhen
var userDefinedMethods = new HashSet<MethodDefinition> ();
var generatedTypeToTypeArgs = new Dictionary<TypeDefinition, TypeArgumentInfo> ();

List<(MessageOrigin, DiagnosticId, string[])>? _warnings = null;

// We delay actually logging the warnings until the compiler-generated type info is
// populated for this type, because the type info is needed to determine whether a warning
// is suppressed.
void AddWarning (MessageOrigin origin, DiagnosticId id, params string[] messageArgs)
{
_warnings ??= new List<(MessageOrigin, DiagnosticId, string[])> ();
_warnings.Add ((origin, id, messageArgs));
}

void ProcessMethod (MethodDefinition method)
{
bool isStateMachineMember = CompilerGeneratedNames.IsStateMachineType (method.DeclaringType.Name);
Expand Down Expand Up @@ -163,7 +174,7 @@ referencedMethod.DeclaringType is var generatedType &&
// fill in null for now, attribute providers will be filled in later
if (!generatedTypeToTypeArgs.TryAdd (generatedType, new TypeArgumentInfo (method, null))) {
var alreadyAssociatedMethod = generatedTypeToTypeArgs[generatedType].CreatingMethod;
_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
AddWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
}
continue;
}
Expand Down Expand Up @@ -214,7 +225,7 @@ referencedMethod.DeclaringType is var generatedType &&

if (!_compilerGeneratedTypeToUserCodeMethod.TryAdd (stateMachineType, method)) {
var alreadyAssociatedMethod = _compilerGeneratedTypeToUserCodeMethod[stateMachineType];
_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), stateMachineType.GetDisplayName ());
AddWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), stateMachineType.GetDisplayName ());
}
// Already warned above if multiple methods map to the same type
// Fill in null for argument providers now, the real providers will be filled in later
Expand Down Expand Up @@ -265,7 +276,7 @@ referencedMethod.DeclaringType is var generatedType &&
// Nested functions get suppressions from the user method only.
if (!_compilerGeneratedMethodToUserCodeMethod.TryAdd (nestedFunction, userDefinedMethod)) {
var alreadyAssociatedMethod = _compilerGeneratedMethodToUserCodeMethod[nestedFunction];
_context.LogWarning (new MessageOrigin (userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), nestedFunction.GetDisplayName ());
AddWarning (new MessageOrigin (userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), nestedFunction.GetDisplayName ());
}
break;
case TypeDefinition stateMachineType:
Expand All @@ -292,12 +303,19 @@ referencedMethod.DeclaringType is var generatedType &&
if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, info)) {
var method = info.CreatingMethod;
var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod;
_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
AddWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
}
}
}

_cachedTypeToCompilerGeneratedMembers.Add (type, compilerGeneratedCallees);

if (_warnings != null) {
foreach (var (origin, id, messageArgs) in _warnings) {
_context.LogWarning (origin, id, messageArgs);
}
}

return type;

/// <summary>
Expand Down
137 changes: 137 additions & 0 deletions .../Mono.Linker.Tests.Cases/Warnings/Dependencies/MultipleMethodsUseSameAsyncStateMachine.il
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
.assembly extern System.Runtime
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
}

.assembly 'library'
{
.hash algorithm 0x00008004
.ver 1:0:0:0
}

.namespace Mono.Linker.Tests.Cases.Warnings.Dependencies
{
.class public auto ansi beforefieldinit MultipleMethodsUseSameAsyncStateMachine
extends [System.Runtime]System.Object
{
// Nested Types
.class nested public sequential ansi sealed beforefieldinit '<StateMachine>d'
extends [System.Runtime]System.ValueType
implements [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine
{
// Fields
.field public static int32 'field'

// Methods
.method public final hidebysig newslot virtual
instance void MoveNext () cil managed
{
// Method begins at RVA 0x207a
// Code size 9 (0x9)
.maxstack 8

IL_0000: nop
IL_0001: ldarg.0
IL_0002: ldc.i4.1
IL_0003: stfld int32 Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'::'field'
IL_0008: ret
} // end of method '<StateMachine>d'::MoveNext

.method private final hidebysig newslot virtual
instance void System.Runtime.CompilerServices.IAsyncStateMachine.SetStateMachine (
class [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine stateMachine
) cil managed
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = (
01 00 01 00 00
)
.override method instance void [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine::SetStateMachine(class [System.Runtime]System.Runtime.CompilerServices.IAsyncStateMachine)
// Method begins at RVA 0x2050
// Code size 2 (0x2)
.maxstack 8

IL_0000: nop
IL_0001: ret
} // end of method '<StateMachine>d'::System.Runtime.CompilerServices.IAsyncStateMachine.SetStateMachine

} // end of class '<StateMachine>d'


// Methods
.method public hidebysig static
void M () cil managed
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.AsyncStateMachineAttribute::.ctor(class [System.Runtime]System.Type) = (
01 00 65 4D 6F 6E 6F 2E 4C 69 6E 6B 65 72 2E 54
65 73 74 73 2E 43 61 73 65 73 2E 57 61 72 6E 69
6E 67 73 2E 44 65 70 65 6E 64 65 6E 63 69 65 73
2E 4D 75 6C 74 69 70 6C 65 4D 65 74 68 6F 64 73
55 73 65 53 61 6D 65 41 73 79 6E 63 53 74 61 74
65 4D 61 63 68 69 6E 65 2B 3C 53 74 61 74 65 4D
61 63 68 69 6E 65 3E 64 00 00
)
.maxstack 8

IL_0000: call void Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine::RUC();
IL_0005: call void Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine::'<M>g__LocalFunction'()
IL_000a: ret
} // end of method MultipleMethodsUseSameAsyncStateMachine::M

.method private hidebysig static
void RUC () cil managed
{
.custom instance void [System.Runtime]System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute::.ctor(string) = (
01 00 03 52 55 43 00 00
)
// Method begins at RVA 0x205c
// Code size 1 (0x1)
.maxstack 8

IL_0000: ret
} // end of method MultipleMethodsUseSameAsyncStateMachine::RUC

.method public hidebysig static
void '<M>g__LocalFunction' () cil managed
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.AsyncStateMachineAttribute::.ctor(class [System.Runtime]System.Type) = (
01 00 65 4D 6F 6E 6F 2E 4C 69 6E 6B 65 72 2E 54
65 73 74 73 2E 43 61 73 65 73 2E 57 61 72 6E 69
6E 67 73 2E 44 65 70 65 6E 64 65 6E 63 69 65 73
2E 4D 75 6C 74 69 70 6C 65 4D 65 74 68 6F 64 73
55 73 65 53 61 6D 65 41 73 79 6E 63 53 74 61 74
65 4D 61 63 68 69 6E 65 2B 3C 53 74 61 74 65 4D
61 63 68 69 6E 65 3E 64 00 00
)
.maxstack 2
.locals init (
[0] valuetype Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d' s
)

IL_0000: nop
IL_0001: ldloca.s 0
IL_0003: initobj Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'
IL_0009: ldloca.s 0
IL_000b: ldc.i4.0
IL_000c: stfld int32 Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'::'field'
IL_0011: ldloca.s 0
IL_0013: call instance void Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine/'<StateMachine>d'::MoveNext()
IL_0018: nop
IL_0019: ret
} // end of method MultipleMethodsUseSameAsyncStateMachine::'<M>g__LocalFunction'

.method public hidebysig specialname rtspecialname
instance void .ctor () cil managed
{
// Method begins at RVA 0x2053
// Code size 8 (0x8)
.maxstack 8

IL_0000: ldarg.0
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
IL_0006: nop
IL_0007: ret
} // end of method MultipleMethodsUseSameAsyncStateMachine::.ctor

} // end of class Mono.Linker.Tests.Cases.Warnings.Dependencies.MultipleMethodsUseSameAsyncStateMachine

}
Loading
Loading