dotnet / runtime

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

Handling byref-like types in LINQ expression interpreter #96160

Open leandromoh opened 10 months ago

leandromoh commented 10 months ago

Description

When upgraded an existing application that uses C# expression tree and AOT to use dotnet 8 some unit tests failed. After some investigation and few tests with expression tree in apart console application I could reproduce the same error using the following code:

image

seems the problems only happens if the parameter of the function is a ReadOnlySpan<char> AND if building using dotnet 8. If I change any of these things (replace span by string or building using dotnet 7) everything work as expected.

Reproduction Steps

In a .NET 8 app with Native AOT enabled (<PublishAot>true</PublishAot>) run the following code:


using System;
using System.Linq.Expressions;

public class Program
{
    delegate T Parse<T>(ReadOnlySpan<char> text);

    private static Parse<T> ReturnDefault<T>()
    {
        var line = Expression.Parameter(typeof(ReadOnlySpan<char>), "span");

        var result = Expression.Lambda<Parse<T>>(Expression.Default(typeof(T)), line);

        return result.Compile();
    }

    public static void Main()
    {
        var func = ReturnDefault<(int age, bool isAlive, DateTime date)>();

        var yy = func("foo bar");

        Console.WriteLine(yy);
        return;
    }
}

Expected behavior

output (0, False, 01/01/0001 00:00:00)

Actual behavior

System.InvalidProgramException: 'Common Language Runtime detected an invalid program.'
   at Thunk1ret_ValueTuple`3_ReadOnlySpan`1(Func`2, ReadOnlySpan`1)
   at ConsoleApp2.Program.Main() in C:\Users\leandro\Desktop\workspace\ConsoleApp1\ConsoleApp2\Program.cs:line 26

Regression?

it works in net 7.0 + AOT

Known Workarounds

No response

Configuration

No response

Other information

No response

ghost commented 10 months ago

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

Issue Details
### Description When upgraded an existing application that uses C# expression tree and AOT to use dotnet 8 some unit tests failed. After some investigation and few tests with expression tree in apart console application and got the same error using the following code: ![image](https://github.com/dotnet/runtime/assets/11452028/85453b95-8c17-4f9f-832f-a54e763e100a) seems the problems only happens if the parameter of the function is a `ReadOnlySpan` AND if building using dotnet 8. If I change any of these things (replace span by string or building using dotnet 7) everything work as expected. ### Reproduction Steps In a .NET 8 app with Native AOT enabled (`true`) run the following code: ```c# using System; using System.Linq.Expressions; public class Program { delegate T Parse(ReadOnlySpan text); private static Parse ReturnDefault() { var line = Expression.Parameter(typeof(ReadOnlySpan), "span"); var result = Expression.Lambda>(Expression.Default(typeof(T)), line); return result.Compile(); } public static void Main() { var func = ReturnDefault<(int age, bool isAlive, DateTime date)>(); var yy = func("foo bar"); Console.WriteLine(yy); return; } } ``` ### Expected behavior output `(0, False, 01/01/0001 00:00:00)` ### Actual behavior ``` System.InvalidProgramException: 'Common Language Runtime detected an invalid program.' at Thunk1ret_ValueTuple`3_ReadOnlySpan`1(Func`2, ReadOnlySpan`1) at ConsoleApp2.Program.Main() in C:\Users\leandro\Desktop\workspace\ConsoleApp1\ConsoleApp2\Program.cs:line 26 ``` ### Regression? it works in net 7.0 + AOT ### Known Workarounds _No response_ ### Configuration _No response_ ### Other information _No response_
Author: leandromoh
Assignees: -
Labels: `area-System.Linq.Expressions`, `untriaged`
Milestone: -
hez2010 commented 10 months ago

This can be reproduced without NativeAOT if you change result.Compile() to result.Compile(true). The exception was thrown due to boxed ByRef-like values, so seems like a ExpressionTree interpreter bug.

MichalStrehovsky commented 10 months ago

This repro doesn't work in .NET 7 either:

Unhandled Exception: System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at System.Delegate.CreateObjectArrayDelegate(Type, Func`2) + 0x12d
   at Internal.Runtime.Augments.DynamicDelegateAugments.CreateObjectArrayDelegate(Type delegateType, Func`2 invoker) + 0x26
   at exprs!<BaseAddress>+0x36b190
   at System.Dynamic.Utils.DelegateHelpers.CreateObjectArrayDelegate(Type, Func`2) + 0x54
   at System.Linq.Expressions.Interpreter.LightLambda.MakeDelegate(Type) + 0xf9
   at System.Linq.Expressions.Interpreter.LightDelegateCreator.CreateDelegate(IStrongBox[]) + 0x71
   at System.Linq.Expressions.Interpreter.LightDelegateCreator.CreateDelegate() + 0x20
   at System.Linq.Expressions.Expression`1.Compile() + 0x70
   at Program.ReturnDefault[T]() + 0xe4
   at Program.Main() + 0x37

I agree with @hez2010 - this looks like an expression interpreter bug. But it's likely the fix for this will be just to generate a better exception message (instead of trying to box a span and hope the runtime will not allow it).

LINQ expressions on Native AOT run on top of an interpreter because it's fundamentally not possible to generate new code at runtime. If you're using expressions for perf, on native AOT you'd be better off using reflection. The expression interpreter uses reflection, but with extra steps.

leandromoh commented 10 months ago

@MichalStrehovsky strange it doesnt work on your computer... I and @EvanMulawski could reproduce the code with success using dotnet 7 (calling .Compile())

my csproj:

<PropertyGroup>
   <OutputType>Exe</OutputType>
   <TargetFramework>net7.0</TargetFramework>
   <ImplicitUsings>enable</ImplicitUsings>
   <PublishAot>true</PublishAot>
   <Nullable>enable</Nullable>
</PropertyGroup>
PS C:\Users\leandro> dotnet --info
.NET SDK:
 Version:           8.0.100
 Commit:            57efcf1350
 Workload version:  8.0.100-manifests.6a1e483a

Ambiente de runtime:
 OS Name:     Windows
 OS Version:  10.0.19045
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.100\

Cargas de trabalho do .NET instaladas:
 Workload version: 8.0.100-manifests.6a1e483a
Não há cargas de trabalho instaladas para exibir.

Host:
  Version:      8.0.0
  Architecture: x64
  Commit:       5535e31a71

.NET SDKs installed:
  6.0.125 [C:\Program Files\dotnet\sdk]
  8.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found
leandromoh commented 10 months ago

@MichalStrehovsky @hez2010 I cant figure out why/where the span is being boxed. and, if it really is, why the code works when not using AOT since the runtime is the same for both?

hez2010 commented 10 months ago

@MichalStrehovsky @hez2010 I cant figure out why/where the span is being boxed. and, if it really is, why the code works when not using AOT since the runtime is the same for both?

ExpressionTree without NAOT is using il emit to generate code for jitting at runtime dynamically thus the boxing can be avoided, but with NAOT it doesn't support runtime code generation so instead an interpreter is used and which is boxing the type.

You may see the code path here: https://source.dot.net/#System.Linq.Expressions/System/Linq/Expressions/LambdaExpression.cs,137 where CanCompileToIL will be false under NativeAOT.

I'm not sure about if the boxing here can be avoided in the interpreter.

leandromoh commented 10 months ago

I'm not sure about if the boxing here can be avoided in the interpreter.

@hez2010 since the code works (calling result.Compile()) with dotnet 7 + AOT I think it is possible. It is a regression bug indeed.

MichalStrehovsky commented 10 months ago

@MichalStrehovsky strange it doesnt work on your computer... I and @EvanMulawski could reproduce the code with success using dotnet 7 (calling .Compile())

I don't think you're actually running the program with native AOT. The exception stack trace posted above is a JIT-based stack trace (you'll not see the Thunk1ret part after publishing as native AOT):

System.InvalidProgramException: 'Common Language Runtime detected an invalid program.'
   at Thunk1ret_ValueTuple`3_ReadOnlySpan`1(Func`2, ReadOnlySpan`1)
   at ConsoleApp2.Program.Main() in C:\Users\leandro\Desktop\workspace\ConsoleApp1\ConsoleApp2\Program.cs:line 26

What is happening is:

The issue has always existed. You'll definitely see this repro with .NET 7 PublishAot once you actually publish the app with dotnet publish. You're not actually publishing with native AOT and that's why you don't see it in 7.

leandromoh commented 10 months ago

@MichalStrehovsky thank you for your detailed explanation! I was not aware of some things you said.

If I understand correctly, the problem is: when my expression tree is going to be interpreted in AOT, the interpreter, which uses reflection, can not do it for ref structs because reflection has works with object as input/output, that is, it would need to box the ref struct, what is not allowed. Is my understanding correct?

If you're using expressions for perf, on native AOT you'd be better off using reflection. The expression interpreter uses reflection, but with extra steps.

If my understanding is correctly, I can not use reflection directly neither, since root of the problem is the boxing done by reflection. What I think could solve the problem, is introduce new specific methods/APIs for reflection to handler ref struct or even unmanaged types.

jkotas commented 10 months ago

it would need to box the ref struct, what is not allowed. Is my understanding correct?

Correct.

What I think could solve the problem, is introduce new specific methods/APIs for reflection to handler ref struct

This reflection feature request is tracked by https://github.com/dotnet/runtime/issues/10057. You can comment about your scenarios there. cc @steveharter

The ref struct support for the expression tree interpreter would be a new major feature that conflicts with System.Linq.Expression status as effectively archived library.

MichalPetryka commented 10 months ago

The ref struct support for the expression tree interpreter would be a new major feature

Wouldn't it be considered as a bug since it works with the ILEmit implementation but not the interpreter one?

jkotas commented 10 months ago

Yes, this specific issue is a bug that still requires major changes to fix.

Support for new language/runtimes features like byref-like structs is generally non-existent in the expression interpreter (e.g. see https://github.com/dotnet/runtime/issues/27499).

MichalStrehovsky commented 10 months ago

Yes, this specific issue is a bug that still requires major changes to fix.

I'd at least leave the bug open so that the area owners can consider adding a better failure mode ("But it's likely the fix for this will be just to generate a better exception message"). It is not the first time a new feature was added to the C# language without looking at how it behaves in this library (see e.g. #86629). Looks like we have a pattern here that the library owners (who also happen to be C# compiler devs) should look at.