RFR: 8310228: Improve error reporting for uncaught native exceptions on Windows

[David Holmes]

On Fri, 16 Jun 2023 16:00:03 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Prevents a stack overflow (or other fatal error) from occurring when handling an unknown exception. See JBS issue for full problem outline.
> 
> WRT implementation:
> - I've refactored `Handle_FLT_Exception` to check the exception code on 64 bit Windows as well. It now returns a boolean indicating whether the exception was recognized and handled. 
> - For x86, I've created a new Uncaugh_Exception_Handler function which implements the uncaught exception handler. This new function replaces the previous use of `Handle_FLT_Exception` as uncaught exception handler. x86 also supports exception chaining through a previous exception handler stored in `prev_uef_handler`. But, on 64 bit windows this field is never set, so we don't need to try to execute the fallback handler on 64 bit. Hence, it seemed clearer to factor out that bit to a separate function. Both the x86 and 64 bit impls call the `handle_FLT_exception` function.
> 
> I've tested this patch locally on Windows x64, I will need some help testing this on Windows x86 (it seems some of the test libraries are failing to build?)
> 
> Testing: tier 1-4

Not an area I'm familiar with so still mulling over it - it sounds reasonable. In the meantime a few comments below.

Thanks.

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

> 286: #endif
> 287: 
> 288: LONG WINAPI Uncaugh_Exception_Handler(struct _EXCEPTION_POINTERS* exceptionInfo);

Typo: Uncaugh

test/hotspot/jtreg/runtime/ErrorHandling/UncaughtNativeExceptionTest.java line 55:

> 53:                     FunctionDescriptor.ofVoid());
> 54: 
> 55:             mh.invokeExact(); // throws native exception

The simpler way would be to just declare a normal native method and invoke it. :)

test/hotspot/jtreg/runtime/ErrorHandling/UncaughtNativeExceptionTest.java line 64:

> 62:     public void testNativeExceptionReporting() throws Exception {
> 63:         ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
> 64:                 "--enable-preview",

What preview feature are you using??

test/hotspot/jtreg/runtime/ErrorHandling/libNativeException.c line 25:

> 23: 
> 24: #ifdef _WIN64
> 25: // Windows only

The way we control which platforms test native code gets built for is through the build system: ` make/test/JtregNativeHotspot.gmk`

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14523#pullrequestreview-1492211161
PR Review Comment: https://git.openjdk.org/jdk/pull/14523#discussion_r1237900588
PR Review Comment: https://git.openjdk.org/jdk/pull/14523#discussion_r1237905050
PR Review Comment: https://git.openjdk.org/jdk/pull/14523#discussion_r1237905267
PR Review Comment: https://git.openjdk.org/jdk/pull/14523#discussion_r1237903867