RFR: 8344935: [ubsan]: javaThread.hpp:1241:52: runtime error: load of value 9831830, which is not a valid value for type 'freeze_result' [v3]

[Richard Reingruber]

On Tue, 26 Nov 2024 13:44:19 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> Seems we miss initialization of  _last_freeze_fail_result in the  JavaThread constructor, this should be added.
>> Causes otherwise ubsan issues in the test  java/lang/Thread/virtual/MonitorEnterExit.java#Xcomp-TieredStopAtLevel1-LM_LEGACY 
>> 
>> /priv/jenkins/client-home/workspace/openjdk-jdk-weekly-linux_x86_64-opt/jdk/src/hotspot/share/runtime/javaThread.hpp:1241:52: runtime error: load of value 9831830, which is not a valid value for type 'freeze_result'
>>     #0 0x7f5edef378eb in JavaThread::last_freeze_fail_result() src/hotspot/share/runtime/javaThread.hpp:1241
>>     #1 0x7f5edef378eb in JVM_VirtualThreadPinnedEvent src/hotspot/share/prims/jvm.cpp:3805
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove added state, add assert

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 260:

> 258:   static freeze_result freeze(JavaThread* thread, intptr_t* const sp) {
> 259:     freeze_result res = freeze_internal<SelfT, false>(thread, sp);
> 260:     JFR_ONLY(assert((res == freeze_ok) || res == thread->last_freeze_fail_result(), "freeze failure not set"));

Sorry, I forgot to add parenthesis to the 2nd comparison...

Suggestion:

    JFR_ONLY(assert((res == freeze_ok) || (res == thread->last_freeze_fail_result()), "freeze failure not set"));

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 266:

> 264:   static freeze_result freeze_preempt(JavaThread* thread, intptr_t* const sp) {
> 265:     freeze_result res = freeze_internal<SelfT, true>(thread, sp);
> 266:     JFR_ONLY(assert((res == freeze_ok) || res == thread->last_freeze_fail_result(), "freeze failure not set"));

Same here...

Suggestion:

    JFR_ONLY(assert((res == freeze_ok) || (res == thread->last_freeze_fail_result()), "freeze failure not set"));

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22361#discussion_r1860092472
PR Review Comment: https://git.openjdk.org/jdk/pull/22361#discussion_r1860096341