RFR: 8291360: Create entry points to expose low-level class file information

[David Holmes]

On Fri, 29 Jul 2022 18:02:46 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

> Please review this change to fix JDK-8291360.  This fix adds entry points getClassFileVersion() and getClassAccessFlagsRaw() to class java.lang.Class.  The new entry points return the current class's class file version and its raw access flags.
> 
> The fix was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 1-3 on Linux x64.  Additionally, the JCK lang, vm, and api tests and new regression tests were run locally on Linux x64.
> 
> Thanks, Harold

Hi Harold,

Generally seems fine. A few nits and comments.

Thanks.

src/hotspot/share/include/jvm.h line 1163:

> 1161: 
> 1162: /*
> 1163:  * Value types support.

Value types? This is supporting the core reflection work isn't it?

src/hotspot/share/prims/jvm.cpp line 4050:

> 4048: /*
> 4049:  * Return the current class's class file version.  The low order 16 bits of the
> 4050:  * the returned jint contains the class's major version.  The high order 16 bits

typo "the the" across the lines
typo s/contains/contain/

src/hotspot/share/prims/jvm.cpp line 4051:

> 4049:  * Return the current class's class file version.  The low order 16 bits of the
> 4050:  * the returned jint contains the class's major version.  The high order 16 bits
> 4051:  * contains the class's minor version.

typo s/contains/contain/

src/hotspot/share/prims/jvm.cpp line 4064:

> 4062:   assert(c->is_instance_klass(), "must be");
> 4063:   InstanceKlass* ik = InstanceKlass::cast(c);
> 4064:   return (ik->minor_version() << 16) | ik->major_version();

I'm curious why the format is minor:major rather than major:minor ?

src/java.base/share/classes/java/lang/Class.java line 4698:

> 4696:      *
> 4697:      * If the class is an array type then the access flags of the component type is
> 4698:      * returned.  If the class is a primitive then ACC_ABSTRACT | ACC_FINAL | ACC_PUBLIC.

The `ACC_ABSTRACT` seems odd - is that way of indicating this "class" can't be instantiated? Is there some spec document that explains this choice?

test/hotspot/jtreg/runtime/ClassFile/ClassAccessFlagsRawTest.java line 60:

> 58:         int flags = (int)m.invoke((new int[3]).getClass());
> 59:         if (flags != 1041) {
> 60:             throw new RuntimeException("expected 1041, got " + flags + " for primitive array");

Hex output would be clearer here too.

test/hotspot/jtreg/runtime/ClassFile/ClassAccessFlagsRawTest.java line 66:

> 64:         flags = (int)m.invoke((new SUPERnotset[2]).getClass());
> 65:         if (flags != 1) {
> 66:             throw new RuntimeException("expected 1, got " + flags + " for object array");

Again hex output would be clearer

test/hotspot/jtreg/runtime/ClassFile/ClassFileVersionTest.java line 31:

> 29:  * @modules java.base/java.lang:open
> 30:  * @compile classFileVersions.jcod
> 31:  * @run main/othervm --enable-preview ClassFileVersionTest

What preview feature is being used here?

test/hotspot/jtreg/runtime/ClassFile/ClassFileVersionTest.java line 45:

> 43:         if (ver != expectedResult) {
> 44:             throw new RuntimeException(
> 45:                 "expected " + expectedResult + ", got " + ver + " for class " + className);

It would be clearer to show the expected and actual in minor:major format. That way if the test fails we can easily see which bit is wrong.

test/hotspot/jtreg/runtime/ClassFile/ClassFileVersionTest.java line 55:

> 53: 
> 54:         testIt("Version64", 64);
> 55:         testIt("Version59", 59);

Any particular reason to choose 59? Shouldn't there also be tests for non-zero minor versions?

test/hotspot/jtreg/runtime/ClassFile/ClassFileVersionTest.java line 62:

> 60:         int ver = (int)m.invoke((new int[3]).getClass());
> 61:         if (ver != 64) {
> 62:             throw new RuntimeException("expected 64, got " + ver + " for primitive array");

Again minor:major format.

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

Changes requested by dholmes (Reviewer).

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