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

Breaking Change in .NET 9.0.0 causes incorrect behavior of TypeScript Compiler running with Jurassic #110004

Closed
kpreisser opened this issue Nov 20, 2024 · 6 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@kpreisser
Copy link

kpreisser commented Nov 20, 2024

Description

Hi!

We are using Jurassic in a .NET application to run JavaScript code. Jurassic compiles JavaScript code into .NET IL (lightweight functions) using System.Reflection.Emit.

After updating from .NET 8.0.11 to .NET 9.0.0, we found that running the TypeScript Compiler v4.5.5 with Jurassic causes incorrect errors to be reported by the TS compiler (see section Actual Behavior). With .NET 8.0.11, the TS compiler runs fine, just like if you run it on other JS runtimes, such as Node.js or in a browser.

It looks like there is some kind of regression breaking change (see next post) in .NET 9.0 when emitting the IL code or JIT-compiling the IL code, but unfortunately I have no idea what the issue could be (as the TypeScript compiler is a very large JavaScript file).

However, I then tested preview versions of .NET 9.0, and found that the issue appears to first occur with .NET 9.0.0-preview.4.24266.19, whereas it still works correctly with .NET 9.0.0-preview.3.24172.9.

I tested this on Windows 11 Version 24H2 (Build 26100.2314) x64, as well as on Ubuntu 24.04 x64.

Reproduction Steps

I created a console application that reproduces the issue. You can find it here: https://github.com/kpreisser/Repro-Net90-Jurassic-TSC-Error/

  • Clone the above project.
  • Run dotnet run.

Expected behavior

The app should complete without error (ExitCode=0) and should output a text like the following:

C:\Users\Admin\Desktop\Repro-Net90-Jurassic-TSC-Error>dotnet run
Restore complete (1,2s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  Repro-Net90-Jurassic-TSC-Error succeeded (4,6s) → bin\Debug\net9.0\Repro-Net90-Jurassic-TSC-Error.dll

Build succeeded in 6,4s
.NET Version: .NET 9.0.0-preview.3.24172.9
Starting up TypeScript Compiler...
TypeScript Compiler startup completed after 00:00:07.6654284, compiling script...
Compilation succeeded after 00:00:14.6245886.

The above output is from running with .NET 9.0.0-preview.3.24172.9, which still works correctly.

Actual behavior

The app throws an InvalidOperationException due to the TypeScript Compiler reporting several wrong errors about TS interfaces being extended incorrectly:

C:\Users\Admin\Desktop\Repro-Net90-Jurassic-TSC-Error>dotnet run
.NET Version: .NET 9.0.0-preview.4.24266.19
Starting up TypeScript Compiler...
TypeScript Compiler startup completed after 00:00:07.5914807, compiling script...
Unhandled exception. System.InvalidOperationException: Compilation of TypeScript code failed after 00:00:14.9237169:
environment-lib.d.ts: Line 329: Interface 'CallableFunction' incorrectly extends interface 'Function'.
environment-lib.d.ts: Line 359: Interface 'NewableFunction' incorrectly extends interface 'Function'.
environment-lib.d.ts: Line 606: Interface 'TemplateStringsArray' incorrectly extends interface 'readonly string[]'.
environment-lib.d.ts: Line 928: Interface 'RegExpMatchArray' incorrectly extends interface 'string[]'.
environment-lib.d.ts: Line 933: Interface 'RegExpExecArray' incorrectly extends interface 'string[]'.
environment-lib.d.ts: Line 1034: Interface 'EvalError' incorrectly extends interface 'Error'.
environment-lib.d.ts: Line 1037: Interface 'EvalErrorConstructor' incorrectly extends interface 'ErrorConstructor'.
environment-lib.d.ts: Line 1045: Interface 'RangeError' incorrectly extends interface 'Error'.
environment-lib.d.ts: Line 1048: Interface 'RangeErrorConstructor' incorrectly extends interface 'ErrorConstructor'.
environment-lib.d.ts: Line 1056: Interface 'ReferenceError' incorrectly extends interface 'Error'.
environment-lib.d.ts: Line 1059: Interface 'ReferenceErrorConstructor' incorrectly extends interface 'ErrorConstructor'.
environment-lib.d.ts: Line 1067: Interface 'SyntaxError' incorrectly extends interface 'Error'.
environment-lib.d.ts: Line 1070: Interface 'SyntaxErrorConstructor' incorrectly extends interface 'ErrorConstructor'.
environment-lib.d.ts: Line 1078: Interface 'TypeError' incorrectly extends interface 'Error'.
environment-lib.d.ts: Line 1081: Interface 'TypeErrorConstructor' incorrectly extends interface 'ErrorConstructor'.
environment-lib.d.ts: Line 1089: Interface 'URIError' incorrectly extends interface 'Error'.
environment-lib.d.ts: Line 1092: Interface 'URIErrorConstructor' incorrectly extends interface 'ErrorConstructor'.
   at Program.<Main>$(String[] args) in C:\Users\Admin\Desktop\Repro-Net90-Jurassic-TSC-Error\Program.cs:line 90
   at Program.<Main>(String[] args)

The above output is from running with .NET 9.0.0-preview.4.24266.19, which is the first preview of .NET 9.0 that shows the error.

Regression?

Yes, this worked correctly in .NET 8.0.11, and worked still correctly in .NET 9.0.0-preview.3.24172.9.

Known Workarounds

I don't know of any workarounds; compiler settings that can be set in the project file, such as CETCompat, TieredCompilation, TieredCompilationQuickJit didn't seem to change the behavior. Also, the behavior is the same when running in Debug (without optimizations) and Release (with optimizations) configurations.

Configuration

  • I can reproduce the issue with the following .NET runtime versions:
    • 9.0.0-preview.4.24266.19 (x64)
    • 9.0.0 (x64)
  • OSes:
    • Windows 11 Version 24H2 (Build 26100.2314) x64
    • Ubuntu 24.04 x64

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 20, 2024
@huoyaoyuan huoyaoyuan added area-System.Reflection.Emit and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 20, 2024
Copy link
Contributor

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

@kpreisser
Copy link
Author

kpreisser commented Nov 21, 2024

When running the Unit Tests of Jurassic, they succeed when running with .NET 8.0.11, but when changing the target framework to net9.0, a number of tests are failing, e.g. BitwiseAnd. I think that the test failures might show the underlying cause of why running the TypeScript compiler in my repro app is producing the incorrect errors.

The BitwiseAnd tests fails due to the changed JIT behavior of floating-point-to-integer conversion, which is noted in the Breaking Changes in .NET 9.0 document:

generator.Emit(OpCodes.Ldc_I4, 7);
generator.Emit(OpCodes.Conv_R8);
generator.Emit(OpCodes.Neg);
generator.Emit(OpCodes.Conv_U4);

or, in C#:

double d2 = -7;
uint u2 = unchecked((uint)d2);

In .NET 8.0, this results in 0xFFFFFFF9, whereas in .NET 9.0, this results in 0. So it looks like this may be caused by an intentional breaking change in the JIT about fp-to-int conversion, and may need to be fixed in Jurassic.

@steveharter
Copy link
Member

steveharter commented Nov 21, 2024

The BitwiseAnd tests fails due to the changed JIT behavior of floating-point-to-integer conversion, which is noted in the Breaking Changes in .NET 9.0 document:

Yes the regression is from adding saturating behavior to the hardware intrinsics that convert floating point values to integer values. I verified the regression locally by building before and after for the commit for that PR.

So it looks like this may be caused by an intentional breaking change in the JIT about fp-to-int conversion, and may need to be fixed in Jurassic.

Yes I suggest filing an issue there.

PTAL @tannergooding @khushal1996

@steveharter steveharter closed this as not planned Won't fix, can't repro, duplicate, stale Nov 21, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Nov 21, 2024
@steveharter steveharter added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-System.Reflection.Emit labels Nov 21, 2024
@kpreisser kpreisser changed the title Regression in .NET 9.0.0 causes incorrect behavior of TypeScript Compiler running with Jurassic (System.Reflection.Emit) Breaking Change in .NET 9.0.0 causes incorrect behavior of TypeScript Compiler running with Jurassic Nov 21, 2024
@kpreisser
Copy link
Author

Thank you! I filed paulbartrum/jurassic#228 to fix this in Jurassic.

@khushal1996
Copy link
Contributor

@steveharter This does look like an effect of floating point to Integer conversion change made in .NET 9. The effect mentioned here is exactly what was changed.

#97529 is the change made in .NET.

double d2 = -7;
uint u2 = unchecked((uint)d2);

The above PR will change the value of u2 from 0xFFFFFFF9 to 0.

Let me know if something needs to be looked at from .NET POV.

@tannergooding
Copy link
Member

Let me know if something needs to be looked at from .NET POV.

There shouldn't be. The code in Jurassic that was depending on this behavior was effectively "already broken".

That is, it would already fail on other hardware where the conversion behavior differed such as Arm64, WASM, or even an x64 machine with AVX512 support (where native unsigned conversion instructions existed).

We didn't even strictly guarantee the behavior on downlevel x64 hardware, and it could fail due to other considerations such as constant folding or other optimizations that occurred at runtime or compile time. I believe the behavior between x86 and x64 even historically differed at one point, until we moved towards normalizing things more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

5 participants