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

[SendaoYan]

On Tue, 31 Dec 2024 10:38:28 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/runtime/frame.cpp line 1166:
>> 
>>> 1164:   // simulate GC crash here to dump java thread in error report
>>> 1165:   if (CrashGCForDumpingJavaThread) {
>>> 1166:     volatile char *t = nullptr; // Use volatile to prevent compiler from optimising away the store
>> 
>> No, don't do this. We don't need more ways to force a crash. Use
>> VMError::controlled_crash instead. Note that this will require upgrading that
>> function to !PRODUCT rather than DEBUG_ONLY. (Don't forget "optimized"
>> builds.)
>> 
>> OTOH, based on the comment's description of what this is needed for, why not
>> just `guarantee(!CrashGCForDumpingJavaThread, "")` ?
>
> Or do we really need to support this in "optimized" builds?  I'm not sure who uses this.

> OTOH, based on the comment's description of what this is needed for, why not just `guarantee(!CrashGCForDumpingJavaThread, "")` ?

Previously I am not quite sure that use `ShouldNotReachHere()` instead of `*t = 'c'` could break the original test intention or not. Use `guarantee(!CrashGCForDumpingJavaThread, "")` make the original test [check](https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java#L103) success. So I think use `guarantee(!CrashGCForDumpingJavaThread, "")` will acceptable. The code has been changed. Thanks your advices.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22757#discussion_r1900045713