On Fri, 24 Apr 2026 03:30:17 GMT, Serguei Spitsyn <[email protected]> wrote:

> This enhancement is to a add JVMTI debugging support for value object 
> construction.
> The fix includes:
>  - Minor `GetLocalObject` and `GetLocalInstance` spec clarifications
>  - JVMTI functions `GetLocalObject` and `GetLocalInstance` now return a 
> snapshot of the `THIS` object if it is requested during a value object 
> construction.
>  - This update adds new test with a needed test coverage: 
> `test/hotspot/jtreg/serviceability/jvmti/valhalla/GetCtorLocal`.
>  - Some corrections in the test 
> `test/hotspot/jtreg/serviceability/jvmti/valhalla/GetSetLocal` for 
> unification with the test above.
> 
> Testing:
>   - Tested locally with new test `GetCtorLocal`
>   - Tested with mach5 tiers 1-6
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Copyrights need updating.

src/hotspot/share/prims/jvmti.xml line 5893:

> 5891:               If the frame's method is a value object constructor and
> 5892:               the requested local is the "<code>this</code>" object 
> then the
> 5893:               value_ptr is set to a snapshot of the "<code>this</code>" 
> object.

Suggestion:

              If the frame's method is a value object constructor and
              the requested local is the "<code>this</code>" object, the
              value_ptr is set to a snapshot of the "<code>this</code>" object.

src/hotspot/share/prims/jvmti.xml line 5948:

> 5946:               On return, points to the variable's value.
> 5947:               If the frame's method is a value object constructor then 
> the 
> 5948:               value_ptr is set to a snapshot of the "<code>this</code>" 
> object.

Suggestion:

              If the frame's method is a value object constructor, the 
              value_ptr is set to a snapshot of the "<code>this</code>" object.

test/hotspot/jtreg/serviceability/jvmti/valhalla/GetCtorLocal/ValueGetCtorLocal.java
 line 71:

> 69:     }
> 70: 
> 71:     private static native boolean nTestCtorThis(Thread thread);

What does the "n" in "nTestCtorThis" mean?

test/hotspot/jtreg/serviceability/jvmti/valhalla/GetCtorLocal/libValueGetCtorLocal.cpp
 line 113:

> 111:     cached_this = jni->NewGlobalRef(obj_this);
> 112:   } else {
> 113:     // cached_this must be a snapshot that not mutate with the ctor 
> changes

Suggestion:

    // cached_this must be a snapshot that does not mutate when the ctor 
changes field values

test/hotspot/jtreg/serviceability/jvmti/valhalla/GetSetLocal/libValueGetSetLocal.cpp
 line 35:

> 33: 
> 34: static jvmtiEnv *jvmti = nullptr;
> 35: static jobject cached_this = nullptr;

I don't think this is used.

test/lib/jdk/test/lib/jvmti/jvmti_common.hpp line 287:

> 285:   jlocation loc;
> 286:   jvmtiError err = jvmti->GetFrameLocation(thread, depth, &method, &loc);
> 287:   check_jvmti_status(jni, err, "notifyFramePop: Failed in JVMTI 
> GetFrameLocation");

The error message needs updating.

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

PR Review: 
https://git.openjdk.org/valhalla/pull/2363#pullrequestreview-4173460883
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2363#discussion_r3140341818
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2363#discussion_r3140344453
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2363#discussion_r3140458873
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2363#discussion_r3140476766
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2363#discussion_r3140490249
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2363#discussion_r3140493323

Reply via email to