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

[David Holmes]

On Mon, 25 Nov 2024 17:06:57 GMT, Richard Reingruber <rrich 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
>
> Hi @MBaesken 
> 
> I don't think this fix will help.
> The uninitialized field is read in [JVM_VirtualThreadPinnedEvent](https://github.com/openjdk/jdk/blob/15ae8d02eeb9c80f5453b88d38081debf956cb65/src/hotspot/share/prims/jvm.cpp#L3805).
> With your change `freeze_ok` will be read and the assertion in the next line will fail.
> 
> The real problem seems to be that `JavaThread::_last_freeze_fail_result` is not always set when the virtual thread is pinned because the freeze attempt failed.
> 
> Currently the setting is only ever done in [`freeze_epilog`](https://github.com/openjdk/jdk/blob/15ae8d02eeb9c80f5453b88d38081debf956cb65/src/hotspot/share/runtime/continuationFreezeThaw.cpp#L1670) but `freeze_epilog` is not always reached from `freeze_internal`.
> 
> From looking quickly I think there are (at least) 2 locations where setting `JavaThread::_last_freeze_fail_result` is missed:
> 
> * [`res` is set](https://github.com/openjdk/jdk/blob/15ae8d02eeb9c80f5453b88d38081debf956cb65/src/hotspot/share/runtime/continuationFreezeThaw.cpp#L1724) and returned a few lines below without setting `_last_freeze_fail_result`
> * [`preempt_epilog`](https://github.com/openjdk/jdk/blob/15ae8d02eeb9c80f5453b88d38081debf956cb65/src/hotspot/share/runtime/continuationFreezeThaw.cpp#L1682) also fails to set `_last_freeze_fail_result`
> 
> My suggestion would be to a add setting `_last_freeze_fail_result` in these cases too. Could look like this https://github.com/openjdk/jdk/commit/723c1c12eb90e6ee9019e5189c9fad705d82a420.

EDIT: for some reason @reinrich 's comment was not showing up when I made mine.

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

PR Comment: https://git.openjdk.org/jdk/pull/22361#issuecomment-2499081951