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

[Release/8.0] Fix FP state restore on macOS exception forwarding #109579

Merged

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Nov 6, 2024

Backport of #105003, #109458 and part of #99255

Customer Impact

[x] Customer reported
[ ] Found internally

Original issue: #109423
Attempt to load Java VM into .NET process on macOS x64 always crashes due to a problem in hardware exception forwarding from coreclr's exception handling port to Java's exception handling port (or any other registered one).

Regression

[x] Yes
[ ] No

Regression in .NET 8.0.8 due to a change to fix AVX512 support on macOS for debugging.

Testing

Testing using a repro provided by the customer and coreclr tests

Risk

Low, fixes a buffer overflow, a stack overflow and obviously incorrect floating point context restoration in case of hardware exception in external non-.NET code.

@janvorli janvorli added Servicing-consider Issue for next servicing release review area-ExceptionHandling-coreclr labels Nov 6, 2024
@janvorli janvorli added this to the 8.0.x milestone Nov 6, 2024
@janvorli janvorli requested a review from VSadov November 6, 2024 11:35
@janvorli janvorli self-assigned this Nov 6, 2024
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. we will take for consideration in 8.0.x

@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 7, 2024
@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.12 Nov 7, 2024
@jonpryor
Copy link
Member

jonpryor commented Nov 26, 2024

It looks like this fix is also needed on .NET 9 + Intel + macOS. From (build log):

  # jonp: LoadJvmLibrary(/Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.25-9/x64/Contents/Home/lib/jli/libjli.dylib)=140707647285392
  # jonp: JNI_CreateJavaVM=4419599558; JNI_GetCreatedJavaVMs=4419599627
  # jonp: executing JNI_CreateJavaVM=1076dbcc6
  /var/folders/p1/44pzfl0j2m301zzfb0fv0p380000gn/T/MSBuildTemprunner/tmp87eb9e12ad0742e0ac307f233a414bd3.exec.cmd: line 2:  8473 Floating point exception: 8   "/Users/runner/hostedtoolcache/dotnet/dotnet" "/Users/runner/work/1/s/bin/Release-net9.0//jnimarshalmethod-gen.dll" "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/Hello-NativeAOTFromJNI.dll" -v -v --keeptemp -L "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/"
##[error]samples/Hello-NativeAOTFromJNI/Hello-NativeAOTFromJNI.targets(44,5): Error MSB3073: The command ""/Users/runner/hostedtoolcache/dotnet/dotnet" "/Users/runner/work/1/s/bin/Release-net9.0//jnimarshalmethod-gen.dll" "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/Hello-NativeAOTFromJNI.dll" -v -v --keeptemp -L "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/" " exited with code 136.

Note the Floating point exception: 8 in the output.

@janvorli
Copy link
Member Author

It looks like this fix is also needed on .NET 9 + Intel + macOS.

Right, I've created a backporting PR to 9 for that today.

@janvorli janvorli merged commit 7db9d7b into dotnet:release/8.0-staging Dec 3, 2024
112 of 117 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants