Skip to content

Conversation

@sbomer
Copy link
Member

@sbomer sbomer commented Oct 24, 2024

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.

Fixes #109157

@ghost ghost added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Oct 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Oct 24, 2024
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

// Avoid repeat scans of the same type
if (_cachedTypeToCompilerGeneratedMembers.ContainsKey (type))
if (!_cachedTypeToCompilerGeneratedMembers.TryAdd (type, null))
return type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify (might be worth a comment): If we ever get into the recursive call situation, this effectively ignores anything in the compiler generate type (as if it's empty) -> for example suppressions would not work.
I think it's perfectly OK as a behavior, but might be worth a comment to make it clear it was an intentional behavioral choice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a fix that avoids the recursive call, so this is no longer the case.

@dotnet-policy-service
Copy link
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@mrvoorhe
Copy link
Contributor

mrvoorhe commented Dec 2, 2024

@sbomer This fixed work. Will you be able to land this?

@github-actions github-actions bot locked and limited conversation to collaborators Jan 2, 2025
@sbomer sbomer reopened this Jan 6, 2025
Delay processing of warnings from building compiler-generated type info
@sbomer sbomer marked this pull request as ready for review January 6, 2025 21:39
@sbomer sbomer requested review from a team and vitek-karas January 8, 2025 19:44
Copy link
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, only nit is maybe add a comment above AddWarning for why we need to delay logging the warnings.

@sbomer sbomer merged commit 426d58f into dotnet:main Jan 17, 2025
103 of 113 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stack overflow from ILLink when LogWarning called

4 participants