RFR: 8316417: ObjectMonitorIterator does not return the most recent monitor and is incorrect if no monitors exists [v6]

[Axel Boldt-Christmas]

On Thu, 21 Sep 2023 06:11:42 GMT, Axel Boldt-Christmas <aboldtch at 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