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

PublishAot affects non-AOT behavior #81803

Closed
AndriySvyryd opened this issue Feb 8, 2023 · 16 comments · Fixed by #88539
Closed

PublishAot affects non-AOT behavior #81803

AndriySvyryd opened this issue Feb 8, 2023 · 16 comments · Fixed by #88539

Comments

@AndriySvyryd
Copy link
Member

Description

Adding <PublishAot>true</PublishAot> breaks dynamic code even when running the JIT version

Reproduction Steps

Expression<Func<int, int, int, int>> e = (_, _, _) => 1;

e.Compile();
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
  </PropertyGroup>

</Project>

dotnet run

Expected behavior

No exceptions

Actual behavior

Unhandled exception. System.PlatformNotSupportedException: Dynamic code generation is not supported on this platform.
   at System.Reflection.Emit.AssemblyBuilder.ThrowDynamicCodeNotSupported()
   at System.Reflection.Emit.DynamicMethod.Init(String name, MethodAttributes attributes, CallingConventions callingConvention, Type returnType, Type[] signature, Type owner, Module m, Boolean skipVisibility, Boolean transparentMethod)
   at System.Reflection.Emit.DynamicMethod..ctor(String name, Type returnType, Type[] parameterTypes)
   at System.Dynamic.Utils.DelegateHelpers.CreateObjectArrayDelegateRefEmit(Type delegateType, Func`2 handler)

Regression?

No response

Known Workarounds

No response

Configuration

SDK: 8.0.100-preview.1.23107.12

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 8, 2023
@ghost
Copy link

ghost commented Feb 8, 2023

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

Issue Details

Description

Adding <PublishAot>true</PublishAot> breaks dynamic code even when running the JIT version

Reproduction Steps

Expression<Func<int, int, int, int>> e = (_, _, _) => 1;

e.Compile();
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
  </PropertyGroup>

</Project>

dotnet run

Expected behavior

No exceptions

Actual behavior

Unhandled exception. System.PlatformNotSupportedException: Dynamic code generation is not supported on this platform.
   at System.Reflection.Emit.AssemblyBuilder.ThrowDynamicCodeNotSupported()
   at System.Reflection.Emit.DynamicMethod.Init(String name, MethodAttributes attributes, CallingConventions callingConvention, Type returnType, Type[] signature, Type owner, Module m, Boolean skipVisibility, Boolean transparentMethod)
   at System.Reflection.Emit.DynamicMethod..ctor(String name, Type returnType, Type[] parameterTypes)
   at System.Dynamic.Utils.DelegateHelpers.CreateObjectArrayDelegateRefEmit(Type delegateType, Func`2 handler)

Regression?

No response

Known Workarounds

No response

Configuration

SDK: 8.0.100-preview.1.23107.12

Other information

No response

Author: AndriySvyryd
Assignees: -
Labels:

area-System.Reflection.Emit

Milestone: -

@MichalStrehovsky
Copy link
Member

This is intentional so that people don't get unpleasant surprise after they debug their app, they're ready to publish, and it doesn't work.

What's the scenario where a working Ref.Emit would be needed during inner dev loop, but got broken on publish?

Cc @eerhardt

@ghost
Copy link

ghost commented Feb 8, 2023

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

Issue Details

Description

Adding <PublishAot>true</PublishAot> breaks dynamic code even when running the JIT version

Reproduction Steps

Expression<Func<int, int, int, int>> e = (_, _, _) => 1;

e.Compile();
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
  </PropertyGroup>

</Project>

dotnet run

Expected behavior

No exceptions

Actual behavior

Unhandled exception. System.PlatformNotSupportedException: Dynamic code generation is not supported on this platform.
   at System.Reflection.Emit.AssemblyBuilder.ThrowDynamicCodeNotSupported()
   at System.Reflection.Emit.DynamicMethod.Init(String name, MethodAttributes attributes, CallingConventions callingConvention, Type returnType, Type[] signature, Type owner, Module m, Boolean skipVisibility, Boolean transparentMethod)
   at System.Reflection.Emit.DynamicMethod..ctor(String name, Type returnType, Type[] parameterTypes)
   at System.Dynamic.Utils.DelegateHelpers.CreateObjectArrayDelegateRefEmit(Type delegateType, Func`2 handler)

Regression?

No response

Known Workarounds

No response

Configuration

SDK: 8.0.100-preview.1.23107.12

Other information

No response

Author: AndriySvyryd
Assignees: -
Labels:

untriaged, area-NativeAOT-coreclr

Milestone: -

@eerhardt
Copy link
Member

eerhardt commented Feb 8, 2023

This was fixed with #80759. Linq.Expressions should still work, even though PublishAot=true. It just doesn't use Ref.Emit in that case, it falls back to the interpreter mode, like it will for the published app.

@AndriySvyryd - can you try a newer build?

@AndriySvyryd
Copy link
Member Author

This was fixed with #80759. Linq.Expressions should still work, even though PublishAot=true. It just doesn't use Ref.Emit in that case, it falls back to the interpreter mode, like it will for the published app.

This is intentional so that people don't get unpleasant surprise after they debug their app, they're ready to publish, and it doesn't work.

What's the scenario where a working Ref.Emit would be needed during inner dev loop, but got broken on publish?

I was just using this specific case to illustrate that PublishAot has a functional impact on the code. The approach EF Core is using to add support for NativeAOT is to first run a portion of the app in JIT, perform all the dynamic operations that would be normally not supported by NativeAOT, then generate source code for the app that would be compatible with NativeAOT.

Currently it's possible to do this by removing PublishAot before running the EF tool, but this would be less than ideal experience once we ship 8.0

@eerhardt
Copy link
Member

eerhardt commented Feb 8, 2023

I was just using this specific case to illustrate that PublishAot has a functional impact on the code.

This is intentional, as @MichalStrehovsky says above. We want the app running during dotnet run and F5 to behave the same way as it will once publishing the app. There are other impacts besides not being able to use Ref.Emit, for example "Startup Hooks" are disabled, so is BinaryFormatter and "Built-in COM" support.

The approach EF Core is using to add support for NativeAOT is to first run a portion of the app in JIT

My understanding is that EF Core doesn't use Ref.Emit directly, but instead uses System.Linq.Expressions. If this is the case, Linq.Expressions will opt to not use Ref.Emit, but instead use an interpreted mode. So it will still work correctly with PublishAot in the .csproj. You just need to upgrade to a newer build.

@AndriySvyryd
Copy link
Member Author

My understanding is that EF Core doesn't use Ref.Emit directly, but instead uses System.Linq.Expressions. If this is the case, Linq.Expressions will opt to not use Ref.Emit, but instead use an interpreted mode.

Correct. As long as there are no other intentional functional differences than the ones mentioned above it should be fine.

You just need to upgrade to a newer build.

I still get it with SDK 8.0.100-preview.2.23108.9 and runtime 8.0.0-preview.2.23102.7

@jkotas
Copy link
Member

jkotas commented Feb 9, 2023

The approach EF Core is using to add support for NativeAOT is to first run a portion of the app in JIT, perform all the dynamic operations that would be normally not supported by NativeAOT, then generate source code for the app that would be compatible with NativeAOT.

How do you guarantee that the test execution of the app performed all operations that the app needs to perform dynamically?

One of principles is to avoid designs that require trial and error: https://github.com/dotnet/designs/blob/main/accepted/2020/form-factors.md#technical-roadmap .

@AndriySvyryd
Copy link
Member Author

How do you guarantee that the test execution of the app performed all operations that the app needs to perform dynamically?

Of course, we can't guarantee that. We discover the user's DbContext and the statically known LINQ queries. But we also plan to provide a hook that would allow the users to explicitly call the dynamic queries and we'll generate code for the exercised combinations. Truly dynamic models and queries won't be supported. We are investigating whether we can provide an analyzer that would find the unsupported queries to avoid trial and error. Also, if the user runs their tests with <PublishAot>true</PublishAot> we'll throw for anything unsupported.

@agocke
Copy link
Member

agocke commented Mar 1, 2023

Given that this is by design, I'm going to close this issue. If there are more specific concerns, we can address them in a separate issue.

@agocke agocke closed this as completed Mar 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Mar 1, 2023
@AndriySvyryd
Copy link
Member Author

AndriySvyryd commented Mar 1, 2023

The specific issue described in the first comment still reproes in 8.0.100-preview.3.23151.5 even though @eerhardt said in #81803 (comment) that is should have been fixed

@eerhardt
Copy link
Member

eerhardt commented Mar 1, 2023

Looks like I missed a couple places in System.Linq.Expressions to respect IsDynamicCodeSupported in #80759.

In this case:

if (CanEmitObjectArrayDelegate)
{
return CreateObjectArrayDelegateRefEmit(delegateType, handler);
}
else
{
return DynamicDelegateLightup.CreateObjectArrayDelegate(delegateType, handler);
}

CanEmitObjectArrayDelegate needs to be updated to respect this switch.

According to

<!-- Configure LINQ expressions - disable Emit everywhere -->
<IlcArg Include="--feature:System.Linq.Expressions.CanCompileToIL=false" />
<IlcArg Include="--feature:System.Linq.Expressions.CanEmitObjectArrayDelegate=false" />
<IlcArg Include="--feature:System.Linq.Expressions.CanCreateArbitraryDelegates=false" />

It looks like CanCreateArbitraryDelegates should be updated as well.

Re-opening to fix these places in System.Linq.Expressions.

@eerhardt eerhardt reopened this Mar 1, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 1, 2023
@eerhardt eerhardt added area-System.Linq.Expressions and removed untriaged New issue has not been triaged by the area owner area-NativeAOT-coreclr labels Mar 1, 2023
@eerhardt eerhardt self-assigned this Mar 1, 2023
@ghost
Copy link

ghost commented Mar 1, 2023

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

Issue Details

Description

Adding <PublishAot>true</PublishAot> breaks dynamic code even when running the JIT version

Reproduction Steps

Expression<Func<int, int, int, int>> e = (_, _, _) => 1;

e.Compile();
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishAot>true</PublishAot>
  </PropertyGroup>

</Project>

dotnet run

Expected behavior

No exceptions

Actual behavior

Unhandled exception. System.PlatformNotSupportedException: Dynamic code generation is not supported on this platform.
   at System.Reflection.Emit.AssemblyBuilder.ThrowDynamicCodeNotSupported()
   at System.Reflection.Emit.DynamicMethod.Init(String name, MethodAttributes attributes, CallingConventions callingConvention, Type returnType, Type[] signature, Type owner, Module m, Boolean skipVisibility, Boolean transparentMethod)
   at System.Reflection.Emit.DynamicMethod..ctor(String name, Type returnType, Type[] parameterTypes)
   at System.Dynamic.Utils.DelegateHelpers.CreateObjectArrayDelegateRefEmit(Type delegateType, Func`2 handler)

Regression?

No response

Known Workarounds

No response

Configuration

SDK: 8.0.100-preview.1.23107.12

Other information

No response

Author: AndriySvyryd
Assignees: -
Labels:

area-System.Linq.Expressions

Milestone: -

@eerhardt
Copy link
Member

eerhardt commented Mar 3, 2023

@MichalStrehovsky - in trying to fix this issue (main...eerhardt:runtime:Fix81803), I hit a snag:

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Works around https://github.com/dotnet/linker/issues/2392")]
private static Func<Type, Func<object?[], object?>, Delegate> CreateObjectArrayDelegateInternal()
=> Type.GetType("Internal.Runtime.Augments.DynamicDelegateAugments")!
.GetMethod("CreateObjectArrayDelegate")!
.CreateDelegate<Func<Type, Func<object?[], object?>, Delegate>>();

When running on CoreCLR, this type/method doesn't exist. Is there a way to simulate what CreateObjectArrayDelegate does when running on CoreCLR?

@MichalStrehovsky
Copy link
Member

When running on CoreCLR, this type/method doesn't exist. Is there a way to simulate what CreateObjectArrayDelegate does when running on CoreCLR?

We would need Ref.Emit on CoreCLR to emulate it, so it's better to not take this codepath and leave the existing Ref.Emit one since it's ref.emit either way. In Native AOT, this relies on the fact that the AOT compiler will generate extra thunks for each delegate type in the app to support this.

I understand it's a bit of a pickle because we don't want ref.emit to work. We might need some sort of backdoor ref-emit.

What I mean is that we would add a private API from CoreLib that we reflection-discover like the above CreateObjectArrayDelegate. It would set a threadstatic somewhere in CoreLib that would say "this thread now allows Ref.Emit" and e.g. return an IDisposable (that would set it back to ref emit not supported). Then whenever we're about to throw in CoreLib because someone attempted ref-emit, we would additionally check for this threadstatic.

@inforithmics
Copy link

inforithmics commented Mar 9, 2023

I'm not a Runtime Expert, but in Xamarin the same problem exsists.
Because on iOS only Native Code is allowed (AOT). So to support Dynamic Code the (Mono) Interpreter is used as a FallBack to handle Reflection.Emit and other dynamic Code Patterns.

Project Property for that
<UseInterpreter>true</UseInterpreter>

dotnet/android#6556

eerhardt added a commit to eerhardt/runtime that referenced this issue Jul 7, 2023
* CanEmitObjectArrayDelegate
* CanCreateArbitraryDelegates

These properties are all set to `false` when running on NativeAOT, so have them respect RuntimeFeature.IsDynamicCodeSupported.

Fix dotnet#81803
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 7, 2023
eerhardt added a commit that referenced this issue Jul 11, 2023
…8539)

* Respect IsDynamicCodeSupported in more places in Linq.Expressions

* CanEmitObjectArrayDelegate
* CanCreateArbitraryDelegates

These properties are all set to `false` when running on NativeAOT, so have them respect RuntimeFeature.IsDynamicCodeSupported.

However, CanEmitObjectArrayDelegate needs a work around because DynamicDelegateAugments.CreateObjectArrayDelegate does not exist in CoreClr's System.Private.CoreLib.

Allow System.Linq.Expressions to create an object[] delegate using Ref.Emit even though RuntimeFeature.IsDynamicCodeSupported is set to false (ex. using a feature switch). To enable this, add an internal method in CoreLib that temporarily allows the current thread to skip the RuntimeFeature check and allows DynamicMethod instances to be created. When System.Linq.Expressions needs to generate one of these delegates, it calls the internal method through Reflection and continues to use Ref.Emit to generate the delegate.

Fix #81803
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants