On Thu, 21 Sep 2023 06:11:42 GMT, Axel Boldt-Christmas <abold...@openjdk.org>
wrote:
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectSynchronizer.java
>> line 86:
>>
>>> 84:
>>> 85: ObjectMonitorIterator() {
>>> 86: mon = new ObjectMonitor(inUseList);
>>
>> How did this ever work? `inUseList` is an address that points to a
>> `MonitorList`, not an `ObjectMonitor`. Your new code has it right, but it
>> seems that this old code would have failed either during construction, or
>> during the first `mon.nextOM()` call.
>
> `inUseList` will end up with the same value as `inUseListHead`. The reason
> the old code worked is because `getAddressField` does not type check and
> `reinterpret_cast<addres>(&ObjectSynchronizer::_in_use_list) ==
> reinterpret_cast<addres>(&ObjectSynchronizer::_in_use_list._head)`
>
> Effectively I changed this to load it correctly (regardless of what
> `offset_of(MonitorList, _head)` ends up being) and name the variables more
> appropriately.
>
> C++ interpretation of what the java change does:
> ```C++
> // Old code
> // Type type = db.lookupType("ObjectSynchronizer");
> // inUseList = type.getAddressField("_in_use_list").getValue();
> address inUseList =
> *(reinterpret_cast<address*>(&ObjectSynchronizer::_in_use_list));
>
> // New code
> // Type objectSynchronizerType = db.lookupType("ObjectSynchronizer");
> // Type monitorListType = db.lookupType("MonitorList");
> // Address monitorListAddr =
> objectSynchronizerType.getField("_in_use_list").getStaticFieldAddress();
> // inUseListHead =
> monitorListType.getAddressField("_head").getAddress(monitorListAddr);
> address monitorListAddr =
> reinterpret_cast<address>(&ObjectSynchronizer::_in_use_list);
> address inUseList = *(reinterpret_cast<address*>(monitorListAddr +
> offset_of(MonitorList, _head)));
Just to clarify what might cause confusion (at least it is what confused me at
first when I read this code) is that `getAddress()`/
`getAddressField(...).getValue()` does not return the address of the field. It
returns the value of the field (loaded and) interpreted as an address.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15782#discussion_r1332528142
