RFR: 8344802: Crash in StubRoutines::verify_mxcsr with -XX:+EnableX86ECoreOpts and -Xcheck:jni [v6]

[Julian Waters]

On Tue, 11 Feb 2025 21:47:31 GMT, Volodymyr Paprotski <vpaprotski at openjdk.org> wrote:

>> (Also see `8319429: Resetting MXCSR flags degrades ecore`)
>> 
>> This PR fixes two issues:
>> - the original issue is a crash caused by `__ warn` corrupting the stack on Windows only
>> - This issue also uncovered that -Xcheck:jni test cases were getting 65k lines of warning on HelloWorld (on both Linux _and_ windows):
>> 
>> OpenJDK 64-Bit Server VM warning: MXCSR changed by native JNI code, use -XX:+RestoreMXCSROnJNICall
>> 
>> 
>> First, the crash. Caused when FXRSTOR is attempting to write reserved bits into MXCSR. If those bits happen to be set, crash. (Hence the crash isn't deterministic. But frequent enough if `__ warn` is used). It is caused by the binding not reserving stack space for register parameters ()
>> ![image](https://github.com/user-attachments/assets/4ad63908-088b-4e9d-9e7d-a3509bee046a)
>> Prolog of the warn function then proceeds to store the for arg registers onto the stack, overriding the fxstore save area. (See https://learn.microsoft.com/en-us/cpp/build/x64-calling-convention?view=msvc-170#calling-convention-defaults)
>> 
>> Fix uses `frame::arg_reg_save_area_bytes` to bump the stack pointer.
>> 
>> ---
>> 
>> I also kept the fix to `verify_mxcsr` since without it, `-Xcheck:jni` is practically unusable when `-XX:+EnableX86ECoreOpts` are set (65k+ lines of warnings)
>
> Volodymyr Paprotski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comments from Sandhya

Looks good otherwise

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 783:

> 781: 
> 782: #ifdef _WIN64
> 783:   // Windows always allocates space for it's register args

Suggestion:

  // Windows always allocates space for its register args

src/hotspot/os/windows/os_windows.cpp line 2757:

> 2755: 
> 2756: #if defined(_M_AMD64)
> 2757:     extern bool handle_FLT_exception(struct _EXCEPTION_POINTERS* exceptionInfo);

This seems strange to declare inside another method like this, not a showstopper but it might be better to declare it where handle_FLT_exception used to be defined in this file

-------------

Marked as reviewed by jwaters (Committer).

PR Review: https://git.openjdk.org/jdk/pull/22673#pullrequestreview-2612367075
PR Review Comment: https://git.openjdk.org/jdk/pull/22673#discussion_r1952906388
PR Review Comment: https://git.openjdk.org/jdk/pull/22673#discussion_r1952902264