Skip to content

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

Merged
janvorli merged 1 commit intodotnet:release/8.0-stagingfrom
janvorli:fix-macos-external-code-hw-eh
Dec 3, 2024
Merged

[Release/8.0] Fix FP state restore on macOS exception forwarding#109579
janvorli merged 1 commit intodotnet:release/8.0-stagingfrom
janvorli:fix-macos-external-code-hw-eh

Conversation

@janvorli
Copy link
Copy Markdown
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 only use for closed issues 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
Copy Markdown
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
Copy Markdown
Contributor

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
Copy Markdown
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
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-ExceptionHandling-coreclr only use for closed issues Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants