RFR: JDK-8318626: GetClassFields does not filter out ConstantPool.constantPoolOop field

[Serguei Spitsyn]

On Tue, 24 Oct 2023 00:46:54 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

> FilteredFieldStream is intended to filter out some fields which does not represent valid java objects.
> Currently the only filtered field is "constantPoolOop" from jdk.internal.reflect.ConstantPool class.
> The change fixes FilteredFieldStream implementation to handle cases when filtered fields is the last field of the class ("constantPoolOop" is the only field of jdk.internal.reflect.ConstantPool)
> 
> Testing:
>   - new test added that compares results of GetClassFields JVMTI function (it uses FilteredFieldStream) with Class.getDeclaredFields();
>   - test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassFields tests

The fix itself looks good. I've posted a couple of nits on the new test.
Thanks,
Serguei

test/hotspot/jtreg/serviceability/FilteredFields/FilteredFieldsTest.java line 25:

> 23: 
> 24: /*
> 25:  * @test

@bug and @summary is needed as well.
The test folder has to be at least `test/hotspot/jtreg/serviceability/jvmti`.
It would be nice to check with Leonid on this.

test/hotspot/jtreg/serviceability/FilteredFields/libFilteredFieldsTest.cpp line 63:

> 61:     jfieldID *fields = nullptr;
> 62: 
> 63:     jvmtiError err = jvmti->GetClassFields(clazz, &fcount, &fields);

Nit: The function `check_jvmti_status()` or `check_jvmti_error()` from jvmti_common.h` can be used here.
The function `fatal()` in other cases like at line 54 can be used.

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

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16328#pullrequestreview-1702664188
PR Review Comment: https://git.openjdk.org/jdk/pull/16328#discussion_r1375135273
PR Review Comment: https://git.openjdk.org/jdk/pull/16328#discussion_r1375137583