Skip to content
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

Jump to bottom

ILLink analyzer analysis holes around dynamic objects #94057

Closed
sbomer opened this issue Oct 26, 2023 · 2 comments · Fixed by #94236
Closed

ILLink analyzer analysis holes around dynamic objects #94057

sbomer opened this issue Oct 26, 2023 · 2 comments · Fixed by #94236
Assignees
@sbomer
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Milestone
9.0.0

Comments

@sbomer
Copy link
Member

sbomer commented Oct 26, 2023

The analyzer has basic handling of IDynamicInvocationOperation, but not IDynamicMemberInvocation and related operation types. This support should be added and moved from the attribute-based analyzer into the dataflow analyzer.

For example, this produces no analyzer warnings:

static void Test(dynamic d) {
    d.Member = 1;
}

but ILLink produces a warning:

Trim analysis warning IL2026: Program.<<Main>$>g__Test|0_0(Object): Using member 'Microsoft.CSharp.RuntimeBinder.Binder.SetMember(CSharpBinderFlags, String, Type, IEnumerable<CSharpArgumentInfo>)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Using dynamic types might cause types or members to be removed by trimmer.

Discovered in #94016.
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 26, 2023
@ghost
Copy link

ghost commented Oct 26, 2023

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

Issue Details

The analyzer has basic handling of IDynamicInvocationOperation, but not IDynamicMemberInvocation and related operation types. This support should be added and moved from the attribute-based analyzer into the dataflow analyzer.

For example, this produces no analyzer warnings:

static void Test(dynamic d) {
    d.Member = 1;
}

but ILLink produces a warning:

Trim analysis warning IL2026: Program.<<Main>$>g__Test|0_0(Object): Using member 'Microsoft.CSharp.RuntimeBinder.Binder.SetMember(CSharpBinderFlags, String, Type, IEnumerable<CSharpArgumentInfo>)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Using dynamic types might cause types or members to be removed by trimmer.

Discovered in #94016.

Author: sbomer
Assignees: -
Labels:

area-Microsoft.CSharp
Milestone: -

@agocke agocke added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed area-Microsoft.CSharp labels Oct 26, 2023
@ghost
Copy link

ghost commented Oct 26, 2023

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

Issue Details

The analyzer has basic handling of IDynamicInvocationOperation, but not IDynamicMemberInvocation and related operation types. This support should be added and moved from the attribute-based analyzer into the dataflow analyzer.

For example, this produces no analyzer warnings:

static void Test(dynamic d) {
    d.Member = 1;
}

but ILLink produces a warning:

Trim analysis warning IL2026: Program.<<Main>$>g__Test|0_0(Object): Using member 'Microsoft.CSharp.RuntimeBinder.Binder.SetMember(CSharpBinderFlags, String, Type, IEnumerable<CSharpArgumentInfo>)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Using dynamic types might cause types or members to be removed by trimmer.

Discovered in #94016.

Author: sbomer
Assignees: -
Labels:

untriaged, area-Tools-ILLink
Milestone: -

@agocke agocke added this to the 9.0.0 milestone Oct 26, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 26, 2023
@sbomer sbomer mentioned this issue Oct 26, 2023
Merged
sbomer added a commit that referenced this issue Oct 26, 2023
@sbomer
Fix analyzer assert and add dynamic object tests (#94058)
56b3ee4 
Fixes the assert from
#94016. #94057
tracks adding full support for this in the analyzer.

We had some existing tests that only covered
IDynamicInvocationOperation. Those have been moved to a shared
testcase (shared by ILLink/NativeAot), and some additional
testcases have been added that include coverage for
IDynamicMemberReference, which was causing the assert.

This includes the same fix for IDynamicIndexerAccessOperation.
@sbomer sbomer mentioned this issue Oct 31, 2023
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Oct 31, 2023
@sbomer sbomer self-assigned this Oct 31, 2023
sbomer added a commit that referenced this issue Nov 6, 2023
@sbomer
[ILLink analyzer] Move dynamic object logic into dataflow analysis (#…
5184067 
…94236)

Moves analysis of dynamic object invocations and related
operations from the attribute analyzer into the dataflow
analyzer. This lowers dynamic object operations into calls to
methods on `Microsoft.CSharp.RuntimeBinder.Binder`, which have
`RequiresUnreferencedCode` annotations, so that the analyzer
produces the same warnings as ILLink.

Adds handling of previously unimplemented operations on dynamic
objects.  Fixes #94057
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 6, 2023
liveans pushed a commit to liveans/dotnet-runtime that referenced this issue Nov 9, 2023
@sbomer @liveans
Fix analyzer assert and add dynamic object tests (dotnet#94058)
a4d66c5 
Fixes the assert from
dotnet#94016. dotnet#94057
tracks adding full support for this in the analyzer.

We had some existing tests that only covered
IDynamicInvocationOperation. Those have been moved to a shared
testcase (shared by ILLink/NativeAot), and some additional
testcases have been added that include coverage for
IDynamicMemberReference, which was causing the assert.

This includes the same fix for IDynamicIndexerAccessOperation.
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@sbomer sbomer

Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
AppModel
Archived in project
Milestone
9.0.0
Development

Successfully merging a pull request may close this issue.

[ILLink analyzer] Move dynamic object logic into dataflow analysis sbomer/runtime
2 participants
@agocke @sbomer