> 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

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/33843/files
  - new: https://git.openjdk.org/jdk/pull/33843/files/5f6755da..4735754f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=33843&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=33843&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/33843.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/33843/head:pull/33843

PR: https://git.openjdk.org/jdk/pull/33843

Reply via email to