NativeAOT and DependencyInjection is broken

[eerhardt]

Description

It looks like #81532 has broken NativeAOT + DependencyInjection.

Reproduction Steps

dotnet publish the following app:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <OutputType>exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
    <PublishAot>true</PublishAot>
    <RuntimeFrameworkVersion>8.0.0-preview.2.23107.1</RuntimeFrameworkVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="$(RuntimeFrameworkVersion)" />
  </ItemGroup>
</Project>

using Microsoft.Extensions.DependencyInjection;
using System.Diagnostics.CodeAnalysis;

var services = new ServiceCollection();
services.AddTransient(typeof(IOptionsFactory<>), typeof(OptionsFactory<>));

var sp = services.BuildServiceProvider();

var options = sp.GetRequiredService<IOptionsFactory<RouteHandlerOptions>>().Create();
Console.WriteLine(options.ToString());


public sealed class RouteHandlerOptions
{
    public RouteHandlerOptions() { }

    public bool ThrowOnBadRequest { get; set; }
}

public interface IOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions>
    where TOptions : class
{
    TOptions Create();
}

public class OptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> :
    IOptionsFactory<TOptions>
    where TOptions : class
{
    public TOptions Create()
    {
        return Activator.CreateInstance<TOptions>();
    }
}

Expected behavior

The app should print RouteHandlerOptions.

Actual behavior

The app fails:

Unhandled Exception: System.MissingMethodException: Constructor on type 'RouteHandlerOptions' not found.
   at System.Activator.CreateInstance[T]() + 0x78
   at Program.<Main>$(String[] args) + 0x9c
   at Net8Api!<BaseAddress>+0x170849

Regression?

Yes. This worked in previous builds.

Known Workarounds

No response

Configuration

No response

Other information

cc @vitek-karas

[ghost]

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

Issue Details

---

Description

It looks like #81532 has broken NativeAOT + DependencyInjection.

Reproduction Steps

dotnet publish the following app:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <OutputType>exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
    <PublishAot>true</PublishAot>
    <RuntimeFrameworkVersion>8.0.0-preview.2.23107.1</RuntimeFrameworkVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="$(RuntimeFrameworkVersion)" />
  </ItemGroup>
</Project>

using Microsoft.Extensions.DependencyInjection;
using System.Diagnostics.CodeAnalysis;

var services = new ServiceCollection();
services.AddTransient(typeof(IOptionsFactory<>), typeof(OptionsFactory<>));

var sp = services.BuildServiceProvider();

var options = sp.GetRequiredService<IOptionsFactory<RouteHandlerOptions>>().Create();
Console.WriteLine(options.ToString());


public sealed class RouteHandlerOptions
{
    public RouteHandlerOptions() { }

    public bool ThrowOnBadRequest { get; set; }
}

public interface IOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions>
    where TOptions : class
{
    TOptions Create();
}

public class OptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> :
    IOptionsFactory<TOptions>
    where TOptions : class
{
    public TOptions Create()
    {
        return Activator.CreateInstance<TOptions>();
    }
}

Expected behavior

The app should print RouteHandlerOptions.

Actual behavior

The app fails:

Unhandled Exception: System.MissingMethodException: Constructor on type 'RouteHandlerOptions' not found.
   at System.Activator.CreateInstance[T]() + 0x78
   at Program.<Main>$(String[] args) + 0x9c
   at Net8Api!<BaseAddress>+0x170849

Regression?

Yes. This worked in previous builds.

Known Workarounds

No response

Configuration

No response

Other information

cc @vitek

Author:	eerhardt
Assignees:	-
Labels:	area-Extensions-Options
Milestone:	-

[ghost]

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

Issue Details

---

Description

It looks like #81532 has broken NativeAOT + DependencyInjection.

Reproduction Steps

dotnet publish the following app:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <OutputType>exe</OutputType>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
    <PublishAot>true</PublishAot>
    <RuntimeFrameworkVersion>8.0.0-preview.2.23107.1</RuntimeFrameworkVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="$(RuntimeFrameworkVersion)" />
  </ItemGroup>
</Project>

using Microsoft.Extensions.DependencyInjection;
using System.Diagnostics.CodeAnalysis;

var services = new ServiceCollection();
services.AddTransient(typeof(IOptionsFactory<>), typeof(OptionsFactory<>));

var sp = services.BuildServiceProvider();

var options = sp.GetRequiredService<IOptionsFactory<RouteHandlerOptions>>().Create();
Console.WriteLine(options.ToString());


public sealed class RouteHandlerOptions
{
    public RouteHandlerOptions() { }

    public bool ThrowOnBadRequest { get; set; }
}

public interface IOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions>
    where TOptions : class
{
    TOptions Create();
}

public class OptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> :
    IOptionsFactory<TOptions>
    where TOptions : class
{
    public TOptions Create()
    {
        return Activator.CreateInstance<TOptions>();
    }
}

Expected behavior

The app should print RouteHandlerOptions.

Actual behavior

The app fails:

Unhandled Exception: System.MissingMethodException: Constructor on type 'RouteHandlerOptions' not found.
   at System.Activator.CreateInstance[T]() + 0x78
   at Program.<Main>$(String[] args) + 0x9c
   at Net8Api!<BaseAddress>+0x170849

Regression?

Yes. This worked in previous builds.

Known Workarounds

No response

Configuration

No response

Other information

cc @vitek-karas

Author:	eerhardt
Assignees:	-
Labels:	untriaged, area-NativeAOT-coreclr
Milestone:	-

[vitek-karas]

The root cause of the failure is this:

GetRequiredService<IOperationsFactory<MyType>>();

Inside GetRequiredService<T>() there's a typeof(T).

The problem is that we only validate generic data flow for the GetRequiredService<> instantiation, not for any instantiations which may happen inside the type parameter. But the typeof(T) effectively provides reflection access to that type since it will do ldtoken on it.

We don't even run the body of the GetRequiredService through data flow analysis because we don't see any interesting type in it - the typeof(T) looks fine, there are no annotations on it. But combined with the wrong suppression in DI, it exposes a hole.

I think the change as it was before revert (so #81532) covers all of the top-level generic instantiations correctly. But it doesn't cover these nested ones. So one possible fix would be to extend the generic argument data flow to apply recursively on the type parameters. It will overmark a little bit in some cases, but I don't see a better way to do this unfortunately.

I also have to think what does "recursively" mean actually - do we need to go into the base types and interfaces as well? Currently I don't think so, since the outer type will be marked as constructed which will trigger generic argument data flow on base type and interfaces already?

[MichalStrehovsky]

We will see a MethodTable being constructed for IOperationsFactory<MyType> (because of the typeof). Can we make sure we keep whatever is required by the T there? We get the GetDependenciesDueToEETypePresence callback to metadata manager with that specific instantiation. First thing we currently do is that we throw away the instantiation: maybe if we first make sure whatever is required by the annotations on the T is kept, the problem will go away? We'd still do the validation on the definition only, but enforcement of kept things can be done before we uninstantiate.

[vitek-karas]

One problem I found with that is warning origin would be messed up in some cases. For example:

class TargetType
{
    [RequiresUnreferencedCode()]
    public void RucMethod();
}

interface INeedsMethods<[DAM(PublicMethods)] T>
{
}

static void GenericMethod<T> { _ = typeof(T); }

static void Test()
{
    // This should produce a warning saying that RucMethod as RUC on it
    GenericMethod<INeedsMethods<TargetType>>();
}

If we implement the behavior inside GetDependenciesDueToEETypePresence the best we could do is to blame the INeedsMethods type definition. Since all we know at that point is that there's INeedsMethods<TargetType> used somewhere.

Ideally the warning would come from the Test method since that's the place where the instantiation is "created".
But for that to happen we would have to do the recursive data flow on generics as I mentioned above.

[MichalStrehovsky]

I think this is one of those where yes, the annotations are not consistent, but it doesn't pose an actual safety problem.

It might be an issue if the app has a wrong suppression only. If this situation doesn't apply to the wrong suppression in di, it should be fine not to warn.

[vitek-karas]

Digging deeper - doing the recursive data flow is problematic for other reasons. It would require us to run data flow on all method bodies which have any generic instantiations in them anywhere. For example, calling a generic method would be enough to trigger data flow on the method. That would dramatically increase the number of methods which run data flow on, which would in turn slow down the compiler.

[vitek-karas]

If this situation doesn't apply to the wrong suppression in di, it should be fine not to warn.

But it does, right? If we don't warn on the RUC method in the above example, we won't warn on the same RUC method if it's done through DI. And yes, there's a wrong suppression in DI, but we're basically trying to workaround that in the compiler. So we're basically saying, we will make it so that the suppression in DI is NOT wrong - and that means that whatever the compiler does, it must not cause additional holes.

[vitek-karas]

The DI based example would be the one from the issue above, just add RUD onto:

    [RequiresUnreferencedCode]
    public RouteHandlerOptions() { }

With that the example above should produce a warning about access to the RUC method (in this case it would actually call the method, so it's not just a potential access). So we can't disable warnings while doing the marking of the inner generic arguments.