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.

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

ILLink analyzer produces dataflow warnings even for invalid annotations #101211

Closed
Tracked by #101149
sbomer opened this issue Apr 17, 2024 · 3 comments · Fixed by #106672
Closed
Tracked by #101149

ILLink analyzer produces dataflow warnings even for invalid annotations #101211

sbomer opened this issue Apr 17, 2024 · 3 comments · Fixed by #106672
Assignees
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@sbomer
Copy link
Member

sbomer commented Apr 17, 2024

DynamicallyAccessedMembers annotations are only allowed on parameters of certain types, such as Type and string. While the analyzer correctly reports a warning about annotations on unsupported types, it still respects those annotations:

using System.Diagnostics.CodeAnalysis;

class Program {
    static void Main() {
        RequireAll(GetFoo()); // Unexpected IL2072 because GetFoo return value doesn't satisfy All
    }

    static void RequireAll([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Foo f) {}  // IL2098 for bad annotation, expected

    static Foo GetFoo() => throw null;
}

class Foo {}
Program.cs(8,17): warning IL2098: Parameter 'f' of method 'Program.RequireAll(Foo)' has 'DynamicallyAccessedMembersAttribute', but that attribute can only be applied to parameters of type 'System.Type' or 'System.String'. [/home/svbomer/src/test-apps/bad-flow/bad-flow.csproj]
Program.cs(5,9): warning IL2072: 'f' argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in call to 'Program.RequireAll(Foo)'. The return value of method 'Program.GetFoo()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. 

IL2098 is expected, but IL2072 is not. The invalid annotations should not produce further warnings for assignments to that location.

Copy link
Contributor

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

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 17, 2024
@sbomer sbomer added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Apr 17, 2024
@sbomer sbomer removed the area-Meta label Apr 17, 2024
Copy link
Contributor

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

@sbomer
Copy link
Member Author

sbomer commented Apr 17, 2024

Looks like ILLink/ILC can have a similar issue in some cases (found by tests added in #101214):

static UnsupportedType GetUnsupportedTypeInstance () => null;

[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
static UnsupportedType GetWithPublicMethods () {
	return GetUnsupportedTypeInstance (); // Unexpected IL2073 from ILLink/ILC
}

sbomer added a commit that referenced this issue Apr 18, 2024
Adds some test coverage for
#101211, including a case
where ILLink/NativeAot have a similar issue.
@sbomer sbomer added this to the Future milestone Apr 25, 2024
@sbomer sbomer removed the untriaged New issue has not been triaged by the area owner label Apr 25, 2024
matouskozak pushed a commit to matouskozak/runtime that referenced this issue Apr 30, 2024
Adds some test coverage for
dotnet#101211, including a case
where ILLink/NativeAot have a similar issue.
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this issue May 30, 2024
Adds some test coverage for
dotnet#101211, including a case
where ILLink/NativeAot have a similar issue.
@sbomer sbomer modified the milestones: Future, 10.0.0 Jul 26, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 19, 2024
@sbomer sbomer self-assigned this Aug 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 23, 2024
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 in-pr There is an active PR which will close this issue when it is merged
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant