RFR: 8359820: Improve handshake/safepoint timeout diagnostic messages [v2]

[David Holmes]

On Thu, 17 Jul 2025 14:08:24 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> Hi, please consider the following changes:
>> 
>> The problem in the issue description is not a problem by itself, the behavior is not unexpected, but it is somewhat difficult to find out what caused SIGILL to be fired.
>> 
>> We propagate this information from `handshake::handle_timeout()` to `VMError::report()` with a help of a global variable.  The same mechanism is used to address a similar issue in the safepoint timeout handler. 
>> 
>> Tested in tiers 1-3.
>
> Anton Artemov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge remote-tracking branch 'origin/master' into JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde
>  - 8359820: Fixed test
>  - 8359820: Removed extra line
>  - Merge branch 'JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde' of https://github.com/toxaart/jdk into JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde
>  - Merge remote-tracking branch 'origin/master' into JDK-8359820-SIGILL-with-low-handshake-timeout-on-intel-sde
>  - 8359820:  Improved safepoint and handshake timeout report
>  - 8359820: Fixed newline
>  - 8359820: Explicitly report SIGILL fired by handshake timeout handler in VMError::report()

Changes requested by dholmes (Reviewer).

src/hotspot/share/runtime/handshake.cpp line 49:

> 47: #include "utilities/systemMemoryBarrier.hpp"
> 48: 
> 49: intptr_t HandshakeTimedOutThread = p2i(nullptr);

Variable names don't start with a capital, or generally use camel-case.

src/hotspot/share/utilities/vmError.cpp line 825:

> 823:         } else if (SafepointTimedOutThread != p2i(nullptr)) {
> 824:           st->print(" (sent by safepoint timeout handler, timed out thread " PTR_FORMAT ")", SafepointTimedOutThread);
> 825:         } else {

I was thinking here that we would only report the timeouts if the current thread was the one we had stashed away. That way we don't hijack a completely different occurrence of SIGILL.

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

PR Review: https://git.openjdk.org/jdk/pull/26309#pullrequestreview-3032262529
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2215173455
PR Review Comment: https://git.openjdk.org/jdk/pull/26309#discussion_r2215180005