dotnet / runtime

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

NativeAOT constrained call bug #98581

Open mrvoorhe opened 6 months ago

mrvoorhe commented 6 months ago

Description

I was experimenting with running our IL2CPP test suite to compare against nativeaot and ran into a bug in our constrained call tests. I believe our constrained call tests were ported from the .NET Core constrained call tests so this was a bit surprising to hit.

Reproduction Steps

ConsoleApp44.zip

1) Unzip the attached file 2) Run dotnet publish 3) Run .\ConsoleApp44\bin\Release\net8.0\win-x64\publish\ConsoleApp44.exe

Expected behavior

You can run dotnet run ConsoleApp44.csproj to see the expected behavior.

C:\UnitySrc\dev\TestGround\ConsoleApp44\ConsoleApp44> dotnet run                                           
ConstrainedCall1Test1 = 6
ConstrainedCall1Test2 = 9

Actual behavior

 C:\UnitySrc\dev\TestGround\ConsoleApp44> .\ConsoleApp44\bin\Release\net8.0\win-x64\publish\ConsoleApp44.exe
ConstrainedCall1Test1 = 4
ConstrainedCall1Test2 = 6

Regression?

Not a regression, both the net7 and net8 nativeaot builds had the problem.

.NET Core works correctly.

Known Workarounds

No response

Configuration

8.0.101 Windows x64

Other information

No response

ghost commented 6 months ago

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

Issue Details
### Description I was experimenting with running our IL2CPP test suite to compare against nativeaot and ran into a bug in our constrained call tests. I believe our constrained call tests were ported from the .NET Core constrained call tests so this was a bit surprising to hit. ### Reproduction Steps [ConsoleApp44.zip](https://github.com/dotnet/runtime/files/14316245/ConsoleApp44.zip) 1) Unzip the attached file 2) Run dotnet publish 3) Run `.\ConsoleApp44\bin\Release\net8.0\win-x64\publish\ConsoleApp44.exe` ### Expected behavior You can run `dotnet run ConsoleApp44.csproj` to see the expected behavior. ``` C:\UnitySrc\dev\TestGround\ConsoleApp44\ConsoleApp44> dotnet run ConstrainedCall1Test1 = 6 ConstrainedCall1Test2 = 9 ``` ### Actual behavior ``` C:\UnitySrc\dev\TestGround\ConsoleApp44> .\ConsoleApp44\bin\Release\net8.0\win-x64\publish\ConsoleApp44.exe ConstrainedCall1Test1 = 4 ConstrainedCall1Test2 = 6 ``` ### Regression? Not a regression, both the net7 and net8 nativeaot builds had the problem. .NET Core works correctly. ### Known Workarounds _No response_ ### Configuration 8.0.101 Windows x64 ### Other information _No response_
Author: mrvoorhe
Assignees: -
Labels: `untriaged`, `area-NativeAOT-coreclr`
Milestone: -
MichalStrehovsky commented 6 months ago

Likely dup of https://github.com/dotnet/runtimelab/issues/1431 (see blocked tests in issues.targets).

am11 commented 6 months ago

Interestingly, with PublishReadyToRun, it produces expected results (despite the common impl of TryResolveConstraintMethodApprox between NativeAOT and R2R).

MichalStrehovsky commented 6 months ago

Interestingly, with PublishReadyToRun, it produces expected results (despite the common impl of TryResolveConstraintMethodApprox between NativeAOT and R2R).

crossgen2 doesn't compute the target of the call in this case, the VM does it (target is compile-time ambiguous).

Even in cases where ReadyToRun needs to compute the target of a virtual call, it will usually refuse to do it because as part of productizing this code, crossgen2 devs preferred to just leave optimizations on the table instead of fixing the type system, see e.g. https://github.com/dotnet/runtime/pull/146 and a couple others. As a result of that PR, crossgen2 (and ILC, since shared codebase) doesn't devirtualize any covariant returns, for example.