RFR: 8304939: os::win32::exit_process_or_thread should be marked noreturn [v3]

[David Holmes]

On Tue, 24 Oct 2023 09:46:51 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> On Windows we have a non-trivial function (exit_process_or_thread) that provides the implementation of various functions like os::die, os::abort, &etc. Those os functions are marked as noreturn, so this implementation helper should also be noreturn.
>> 
>> The current change does several things around exit_process_or_thread, which is moved out of the os::win32 class since it does not need access to anything in that class other than the Ept enum, which is also moved to os_windows.cpp as well. The signature is changed to void, since exit_process_or_thread's current return value is simply the exit code parameter that it is passed, and to qualify as noreturn it should not return its current value of int. The only usage of this return value is in thread_native_entry, and this usage can easily be replaced by returning the res local that exit_process_or_thread is passed. For this os::infinite_sleep takes the place of the return statement in exit_process_or_thread, to make sure it qualifies as noreturn.
>> 
>> Although not mentioned in the title, raise_fail_fast has also been fixed. RaiseFailFastException is not itself noreturn, so os::infinite_sleep has been added there as well, to make raise_fail_fast qualify as a noreturn method
>> 
>> All the changes mentioned above fixes any Undefined Behaviour on Windows with regards to noreturn, by making all code marked noreturn (os::die() os::exit() etc) never return. This is not a problem on other platforms since their implementations of os::abort or os::die etc already call noreturn methods.
>> 
>> This fix also fixes compilation on gcc windows, which warns about noreturn methods returning prior to this changeset
>
> Julian Waters 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 'openjdk:master' into noreturn
>  - Minor Style Change in os_windows.cpp
>  - 8304939

On my to-do list ...

> moved out of the os::win32

This doesn't seem to add any value. Seemed fine to me in os::win32 versus os class.

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

PR Comment: https://git.openjdk.org/jdk/pull/16303#issuecomment-1780336868
PR Comment: https://git.openjdk.org/jdk/pull/16303#issuecomment-1780337907