8364187: Make getClassAccessFlagsRaw non-native #26517

coleenp · 2025-07-28T20:14:15Z

This change removes the intrinsic for getClassAccessFlagsRaw for reflection and initializes an rawAccessFlags field in java.lang.Class instead, that Java code can non-natively access.
Tested with tier1-4.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8364187: Make getClassAccessFlagsRaw non-native (Enhancement - P4)

Reviewers

Tobias Hartmann (@TobiHartmann - Reviewer)
Roger Riggs (@RogerRiggs - Reviewer)
Chen Liang (@liach - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26517/head:pull/26517
$ git checkout pull/26517

Update a local copy of the PR:
$ git checkout pull/26517
$ git pull https://git.openjdk.org/jdk.git pull/26517/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26517

View PR using the GUI difftool:
$ git pr show -t 26517

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26517.diff

Using Webrev

Link to Webrev Comment

… it for arrays so we really want the "raw" classfile access flags.

bridgekeeper · 2025-07-28T20:14:57Z

👋 Welcome back coleenp! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-07-28T20:15:15Z

@coleenp This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8364187: Make getClassAccessFlagsRaw non-native

Reviewed-by: thartmann, rriggs, liach

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 80 new commits pushed to the master branch:

724e8c0: 8364484: misc tests fail with Received fatal alert: handshake_failure
e0e8206: 8364166: Parallel: Remove the use of soft_ref_policy in Full GC
443afdc: 8364176: Serial: Group all class unloading logic at the end of marking phase
... and 77 more: https://git.openjdk.org/jdk/compare/4e53a9d9dfe7a1ac7c3d7402e5ca3a3d3fcbb709...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-07-28T20:15:56Z

@coleenp The following labels will be automatically applied to this pull request:

core-libs
graal
hotspot
security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-07-28T20:19:31Z

Webrevs

src/java.base/share/classes/java/lang/Class.java

…test to reflect what the JVM does.

src/java.base/share/classes/java/lang/Class.java

src/java.base/share/classes/jdk/internal/reflect/Reflection.java

RogerRiggs · 2025-07-29T14:55:04Z

src/java.base/share/classes/java/lang/Class.java

@@ -245,6 +245,7 @@ private Class(ClassLoader loader, Class<?> arrayComponentType, char mods, Protec
        modifiers = mods;
        protectionDomain = pd;
        primitive = isPrim;
+        classFileAccessFlags = flags;


Its not that hard to add a field, why not have done this for identity?

Fields are initialized by injection in javaClasses instead of through this constructor, so it's easy for us to accidentally forget to inject a field.

For IDENTITY, I didn't have to inject that one because the Java code knew when to set it, not the JVM code reading the data out of the classfile. And the logic belongs in the Java code, not the JVM. This one comes from the classfile, there isn't another way to get the information to the java.lang.Class.

The VM and Java use the same logic for the value of isIdentity().
It is computed from as many a 5 fields/flags. At present, it has to be computed on each call to Class.isIdentity().
It would be reasonable to compute the value once in the constructor, but the code in the constructor is not run.

Roger, can you file another RFE for this for repo-valhalla and I can try to figure out how to best to do this? Or I can. I didn't think the valhalla isIdentityClass() code was expensive to call.

Created JDK-8364447

src/java.base/share/classes/jdk/internal/reflect/Reflection.java

Co-authored-by: ExE Boss <[email protected]>

src/hotspot/share/opto/memnode.cpp

TobiHartmann

Compiler changes look good to me.

RogerRiggs

Looks good, thanks

liach

These changes look reasonable.

coleenp · 2025-08-01T15:20:47Z

Thank you for reviewing Tobias, Roger and Chen.
/integrate

openjdk · 2025-08-01T15:21:49Z

Going to push as commit ee3665b.
Since your change was applied there have been 93 commits pushed to the master branch:

2ba8a06: 8348868: AArch64: Add backend support for SelectFromTwoVector
8ac4a88: 8362237: IllegalArgumentException in the launcher when exception without stack trace is thrown
6c58047: 8364296: Set IntelJccErratumMitigation flag ergonomically
... and 90 more: https://git.openjdk.org/jdk/compare/4e53a9d9dfe7a1ac7c3d7402e5ca3a3d3fcbb709...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-08-01T15:22:10Z

@coleenp Pushed as commit ee3665b.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

dougxc · 2025-08-04T15:00:52Z

src/hotspot/share/prims/jvm.cpp

@@ -1741,19 +1741,6 @@ JVM_ENTRY(jobjectArray, JVM_GetClassDeclaredConstructors(JNIEnv *env, jclass ofC
 }
 JVM_END

-JVM_ENTRY(jint, JVM_GetClassAccessFlags(JNIEnv *env, jclass cls))


What about the declaration in jvm.h?

jdk/src/hotspot/share/include/jvm.h

Line 610 in 6c52b73

JVM_GetClassAccessFlags(JNIEnv *env, jclass cls);

Created issue https://bugs.openjdk.org/browse/JDK-8364750

Sorry I missed the declaration. Will fix it. Thanks for filing the bug Roger.

coleenp added 3 commits July 28, 2025 12:08

8364187: Make getClassAccessFlagsRaw non-native

1d17cbd

Add a comment

5313fa1

The only caller for getRawClassAccessFlags in Class.java doesn't call…

b4daf73

… it for arrays so we really want the "raw" classfile access flags.

openjdk bot added the rfr Pull request is ready for review label Jul 28, 2025

openjdk bot added graal [email protected] security [email protected] hotspot [email protected] core-libs [email protected] labels Jul 28, 2025

liach reviewed Jul 28, 2025

View reviewed changes

src/java.base/share/classes/java/lang/Class.java Outdated Show resolved Hide resolved

Rename getRawClassAccessFlags to getClassFileAccessFlags and fix the …

163a8e5

…test to reflect what the JVM does.

RogerRiggs reviewed Jul 29, 2025

View reviewed changes

Fix comments and remove Holder.

a497447

ExE-Boss reviewed Jul 29, 2025

View reviewed changes

src/java.base/share/classes/jdk/internal/reflect/Reflection.java Outdated Show resolved Hide resolved

src/java.base/share/classes/jdk/internal/reflect/Reflection.java Outdated Show resolved Hide resolved

Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java

45b92c5

Co-authored-by: ExE Boss <[email protected]>

TobiHartmann reviewed Jul 30, 2025

View reviewed changes

src/hotspot/share/opto/memnode.cpp Show resolved Hide resolved

Restore c2 optimization.

fb6c15e

TobiHartmann approved these changes Jul 31, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 31, 2025

RogerRiggs approved these changes Jul 31, 2025

View reviewed changes

liach approved these changes Jul 31, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Aug 1, 2025

openjdk bot closed this Aug 1, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Aug 1, 2025

coleenp deleted the reflect-af branch August 1, 2025 15:29

graalvmbot mentioned this pull request Aug 3, 2025

[JDK-8364586] Adapt JDK-8364187: Make getClassAccessFlagsRaw non-native oracle/graal#11874

Merged

dougxc reviewed Aug 4, 2025

View reviewed changes

graalvmbot mentioned this pull request Aug 12, 2025

[GR-68596] Update labsjdk to 26+10-jvmci-b01 oracle/graal#11943

Open

8364187: Make getClassAccessFlagsRaw non-native #26517

8364187: Make getClassAccessFlagsRaw non-native #26517

Uh oh!

Conversation

coleenp commented Jul 28, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jul 28, 2025

Uh oh!

openjdk bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jul 28, 2025

Uh oh!

mlbridge bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coleenp Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

RogerRiggs left a comment

Choose a reason for hiding this comment

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

coleenp commented Aug 1, 2025

Uh oh!

openjdk bot commented Aug 1, 2025

Uh oh!

openjdk bot commented Aug 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coleenp commented Jul 28, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jul 28, 2025 •

edited

Loading

mlbridge bot commented Jul 28, 2025 •

edited

Loading

coleenp Jul 31, 2025 •

edited

Loading