On Fri, 13 Nov 2026 11:08:18 GMT, Serguei Spitsyn <[email protected]> wrote:

>> JVMTI can mistake a flattened value for its `ClassLoader` holder.
>> The function `VM_HeapWalkOperation::visit()` checks the holder object’s type 
>> instead of the value being visited. During reference walking, a flattened 
>> field inside a `ClassLoader` subclass therefore enters the class-loader 
>> path, triggering an assertion in debug builds and potentially reporting 
>> incorrect references in release builds. The check should use the visited 
>> object’s represented class.
>> 
>> The problem is in `src/hotspot/share/prims/jvmtiTagMap.cpp:3405`:
>> 
>> 
>>     } else if (java_lang_ClassLoader::is_instance(o.obj())) {
>>       return iterate_over_class_loader(o);
>>     }
>> 
>> together with:
>> 
>> 
>>   oop _obj;     // for flattened value object this is holder object
>> 
>> 
>> The fix is in this simple diff:
>> 
>> 
>> @@ -3402,7 +3402,7 @@ bool VM_HeapWalkOperation::visit(const 
>> JvmtiHeapwalkObject& o) {
>>          // a java.lang.Class
>>          return iterate_over_class(o);
>>        }
>> -    } else if (java_lang_ClassLoader::is_instance(o.obj())) {
>> +    } else if (java_lang_ClassLoader::is_subclass(klass)) {
>>        return iterate_over_class_loader(o);
>>      } else {
>>        // we report stack references only when initial object is not 
>> specified
>> 
>> 
>> Additionally, this update hardened the uses of the `obj()` by adding an 
>> assert and also replacing it with the `holder()` in relevant places:
>> 
>> 
>> +++ b/src/hotspot/share/prims/jvmtiTagMapTable.hpp
>> @@ -58,7 +58,11 @@ class JvmtiHeapwalkObject {
>>    inline bool is_value() const { return _value_klass != nullptr; }
>>    inline bool is_flat() const { return _offset != 0; }
>> 
>> -  inline oop obj() const { return _obj; }
>> +  inline oop holder() const { return _obj; }
>> +  inline oop obj() const {
>> +    assert(!is_flat(), "flattened value does not have oop");
>> +    return _obj;
>> +  }
>> 
>> 
>> Also, the following test was updated to add a necessary test coverage:
>>   - `serviceability/jvmti/valhalla/Heapwalking/ValueHeapwalkingTest.java`
>> 
>> Testing:
>>  - tested with the update test 
>> `serviceability/jvmti/valhalla/Heapwalking/ValueHeapwalkingTest.java`
>>  - ran mach5 tiers 1-6 (all are GREEN)
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Serguei Spitsyn has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   minor tweak in an assert message

Marked as reviewed by stefank (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/33843#pullrequestreview-5892168409

Reply via email to