dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.07k stars 4.69k forks source link

ILLink analyzer produces dataflow warnings even for invalid annotations #101211

Closed sbomer closed 1 month ago

sbomer commented 5 months ago

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.

dotnet-policy-service[bot] commented 5 months ago

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

dotnet-policy-service[bot] commented 5 months ago

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

sbomer commented 5 months ago

Looks like ILLink/ILC can have a similar issue in some cases (found by tests added in https://github.com/dotnet/runtime/pull/101214):

static UnsupportedType GetUnsupportedTypeInstance () => null;

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