RFR: 8302798: Refactor -XX:+UseOSErrorReporting for noreturn crash reporting [v3]

[Julian Waters]

On Wed, 1 Mar 2023 10:25:06 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change to the implementation of the Windows-specific option
>> UseOSErrorReporting, toward allowing crash reporting functions to be declared
>> noreturn.  VMError::report_and_die no longer conditionally returns if the
>> Windows-only option UseOSErrorReporting is true.
>> 
>> The Windows-only sections of report_and_die now call RaiseFailFastException
>> (https://learn.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-raisefailfastexception),
>> which immediately invokes WER (Windows Error Reporting) if it is enabled,
>> without executing structured exception handler.  If WER is not enabled, it
>> just immediately terminates the program.  Thus, we no longer return to walk up
>> thestructured exception handler chain to pop out at the top as unhandled in
>> order to invoke WER.
>> 
>> This permits declaring report_and_die as [[noreturn]], once some functions
>> from the os class are also so declared.  Also adding that attribute as
>> appropriate to other functions in the os class.  This of course assumes
>> the use of [[noreturn]] in HotSpot code is approved (JDK-8302124).
>> 
>> There is a pre-existing bug that I'll be reporting separately.  If
>> UseOSErrorReporting and CreateCoredumpOnCrash are both true, we create an
>> empty .mdmp file.  We shouldn't create that file when UseOSErrorReporting.
>> 
>> Testing:
>> mach5 tier1-3
>> 
>> Manual testing with the following, to verify desired behavior.
>> 
>> -XX:ErrorHandlerTest=N
>> 1: assertion failure
>> 2: guarantee failure
>> 14: SIGSEGV
>> 15: divide by zero
>> path/to/bin/java \
>>   -XX:+UnlockDiagnosticVMOptions \
>>   -XX:+ErrorLogSecondaryErrorDetails \
>>   -XX:+UseOSErrorReporting \
>>   -XX:ErrorHandlerTest=1 \
>>   TestDebug.java
>> 
>> --- TestDebug.java ---
>> import java.lang.String;
>> public class TestDebug {
>>     static private volatile String dummy;
>>     public static void main(String[] args) throws Exception {
>>         while (true) {
>>             dummy = new String("foo bar");
>>         }
>>     }
>> }
>> --- end TestDebug.java ---
>> 
>> The state of WER can be examined and modified using Power Shell commands
>> {Get,Enable,Disable}-WindowsErrorReporting.
>> 
>> The state of reporting WER captured errors can be examined and modified using
>> Control Panel > Security and Maintenance > Maintenance : Report Problems [on,off]
>> 
>> With Report Problems off, reports are placed in
>> c:\ProgramData\Microsoft\Windows\WER\ReportArchive
>> 
>> I verified that executing the above test with WER enabled adds an entry in
>> that directory, but not when it's disabled.  Also nothing is added there when
>> the test is run with -XX:-UseOSErrorReporting.
>
> Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'master' into failfast
>  - remove failfast cuttoff of secondary errors
>  - failfast

When compiling a Windows HotSpot with gcc (see [8288293](https://bugs.openjdk.org/browse/JDK-8288293?filter=-3)) newer gcc versions have a diagnostic that can catch when noreturn code returns in certain areas, and gcc catches several with this change:

src/hotspot/os/windows/os_windows.cpp: In static member function 'static void os::abort(bool, void*, const void*)':
src/hotspot/os/windows/os_windows.cpp:1249:1: error: 'noreturn' function does return [-Werror]
 1249 | }
      | ^
src/hotspot/os/windows/os_windows.cpp: In static member function 'static void os::die()':
src/hotspot/os/windows/os_windows.cpp:1254:1: error: 'noreturn' function does return [-Werror]
 1254 | }
      | ^
src/hotspot/os/windows/os_windows.cpp: In static member function 'static void os::exit(int)':
src/hotspot/os/windows/os_windows.cpp:4792:1: error: 'noreturn' function does return [-Werror]
 4792 | }
      | ^
src/hotspot/os/windows/os_windows.cpp: In static member function 'static void os::_exit(int)':
src/hotspot/os/windows/os_windows.cpp:4796:1: error: 'noreturn' function does return [-Werror]
 4796 | }


I don't know if this is something we should be worried about with our current setup and the regular build that we have for Windows, so I'm leaving this warning here for someone else to read through

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

PR Comment: https://git.openjdk.org/jdk/pull/12759#issuecomment-1477797624