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

Handling byref-like types in LINQ expression interpreter #96160

Open
leandromoh opened this issue Dec 19, 2023 · 13 comments
Open

Handling byref-like types in LINQ expression interpreter #96160

leandromoh opened this issue Dec 19, 2023 · 13 comments

Comments

@leandromoh
Copy link

leandromoh commented Dec 19, 2023

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 ghost added the untriaged New issue has not been triaged by the area owner label Dec 19, 2023
@ghost
Copy link

ghost commented Dec 19, 2023

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

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

Author: leandromoh
Assignees: -
Labels:

area-System.Linq.Expressions, untriaged

Milestone: -

@hez2010
Copy link
Contributor

hez2010 commented Dec 19, 2023

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
Copy link
Member

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
Copy link
Author

leandromoh commented Dec 19, 2023

@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
Copy link
Author

@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
Copy link
Contributor

hez2010 commented Dec 19, 2023

@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
Copy link
Author

leandromoh commented Dec 19, 2023

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
Copy link
Member

@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:

  • You're doing an F5 launch in visual studio. This runs on top of a JIT. The program will only be compiled with native AOT once you publish the app (with dotnet publish or in VS UI) - PublishAot is a publishing setting, not a debug-time setting.
  • In .NET 7, adding PublishAot to the project file didn't do much to the debug time experience. It only affected publishing. The app runs on a JIT and LINQ expressions will be reflection.emitted at runtime.
  • In .NET 8, we made updates to make F5 debugging as close as possible to the native AOT experience (even though it's still running on top of a JIT). One of the things it does is that it also switches LINQ expressions to be interpreter based, not reflection.emit based. So you can now see the bug/limitation while F5 debugging the app in VS as well.

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.

@MichalStrehovsky MichalStrehovsky changed the title Native aot regression in .net 8.0 (Expression tree + ReadOnlySpan) Handling byref-like types in LINQ expression interpreter Dec 21, 2023
@leandromoh
Copy link
Author

leandromoh commented Dec 29, 2023

@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
Copy link
Member

jkotas commented Dec 30, 2023

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 #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.

@jkotas jkotas closed this as completed Dec 30, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Dec 30, 2023
@MichalPetryka
Copy link
Contributor

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
Copy link
Member

jkotas commented Dec 30, 2023

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 #27499).

@MichalStrehovsky
Copy link
Member

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.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 2, 2024
@cston cston added this to the Future milestone Aug 14, 2024
@cston cston removed the untriaged New issue has not been triaged by the area owner label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants