RFR: 8343756: CAN_SHOW_REGISTERS_ON_ASSERT for Windows [v2]

[Markus Grönlund]

On Wed, 13 Nov 2024 17:22:24 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> src/hotspot/share/utilities/debug.hpp line 41:
>> 
>>> 39: #define CAN_SHOW_REGISTERS_ON_ASSERT
>>> 40: extern char* g_assert_poison;
>>> 41: extern const char* g_assert_poison_page_for_reporting;
>> 
>> This g_assert_poison_page_for_reporting only exists to recognize assert poison crash when the assert poison itself is disarmed, right? So, during error reporting?
>> 
>> Makes sense but proposal for simplification:
>> - have two pointers initialized to the poison at arm time. E.g. "g_assert_poison" (the real one) and "g_assert_poison_copy"
>> - on disarm, switch g_assert_poison but leave the other unchanged
>> - when checking, always check against the copy - it never changes
>> - now you can unify the several code pieces that do `if ((si->si_signo == SIGSEGV || si->si_signo == SIGBUS) && si->si_addr == g_assert_poison_copy)` to something like `bool VMError::was_assertion_poison_crash(siginfo)`
>
> void disarm_assert_poison() {
>   g_assert_poison_page_for_reporting = g_assert_poison;
>   g_assert_poison = &g_dummy;
> }
> 
> Its only initialized when the real page deactivates.
> 
> g_assert_poison_page_for_reporting is only used in print_siginfo() to skip printing the sigInfo, and it only applies on Windows, I believe.
> 
> I will try to see if there is value in attempting to put something consolidated into VMError.

I think it turned out better. Thanks for the tip, Thomas.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21994#discussion_r1841320054