On Wed, 20 Sep 2023 22:37:59 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Axel Boldt-Christmas has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed unnecessary comment
>
> 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)));
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15782#discussion_r1332516949
