RFR: 8346193: CrashGCForDumpingJavaThread do not trigger expected crash build with clang17 [v5]

[Kim Barrett]

On Thu, 2 Jan 2025 02:43:01 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>> Function `frame::oops_do_internal` in src/hotspot/share/runtime/frame.cpp assign value to a nullptr `char *t` and intended to cause jvm crash. But after the assignment the nullptr do not use anymore, so clang17 consider the `char *t` initialization and assignment is "dead code". This PR use function `guarantee` instead of 'write a byte to nullptr' to trigger the expected jvm crash, risk is low.
>> 
>> Here is the example explain the "dead code" elimination.
>> 
>> 1. Without volatile modifier, clang will delete the "dead code" and cause no more Segmentation fault error by -O1.
>> 
>> 
>>> cat demo.c 
>> int main() { char *t = 0; *t = 'c'; return 0; }
>>> clang -O0 demo.c && ./a.out ; echo $?
>> Segmentation fault (core dumped)
>> 139
>>> clang -O1 demo.c && ./a.out ; echo $?
>> 0
>> 
>> 
>> 2. With volatile modifier, clang do not delete the "dead code" again and and the expected Segmentation fault occur by -O1.
>> 
>>> cat demo.c 
>> int main() { volatile char *t = 0; *t = 'c'; return 0; }
>>> clang -O0 demo.c && ./a.out ; echo $?
>> Segmentation fault (core dumped)
>> 139
>>> clang -O1 demo.c && ./a.out ; echo $?
>> Segmentation fault (core dumped)
>> 139
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Delete curly braces

Changes requested by kbarrett (Reviewer).

src/hotspot/share/runtime/frame.cpp line 1166:

> 1164:   // simulate GC crash here to dump java thread in error report
> 1165:   if (CrashGCForDumpingJavaThread)
> 1166:     guarantee(!CrashGCForDumpingJavaThread, "");

Besides being contrary to the HotSpot Style Guide, that isn't at all what I
meant.  We don't need to test CrashGCForDumpingJavaThread before testing it
again int the guarantee.

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

PR Review: https://git.openjdk.org/jdk/pull/22757#pullrequestreview-2526981587
PR Review Comment: https://git.openjdk.org/jdk/pull/22757#discussion_r1900508321