On Mon, 16 Mar 2026 14:17:26 GMT, Paul Hübner <[email protected]> wrote:
>> Chen Liang has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Comment update
>> - Remove meaingless bouncer in Reflection
>
> src/hotspot/share/oops/instanceKlass.cpp line 3786:
>
>> 3784: if (!Arguments::is_valhalla_enabled()) {
>> 3785: // Remember to strip ACC_SUPER bit without Valhalla
>> 3786: access &= (~JVM_ACC_SUPER);
>
> Are you stripping `ACC_SUPER` (the name of the value back then) or
> `ACC_IDENTITY` (the new name we used when bringing back the value)?
> * If old, then maybe this is better done in `classFileParser` where we
> already have some logic to deal with `ACC_SUPER` in old classfiles.
> * If new, could this be `JVM_ACC_IDENTITY`?
When valhalla is enabled, there is only `ACC_IDENTITY`; when valhalla is not
enabled, there is only `ACC_SUPER`. So no `ACC_IDENTITY` is accidentally
stripped for core reflection. (I believe the rest of hotspot still relies on
this bit to be present to act as if the code receives an identity class, so I
am patching it only here instead of on the klass access flags).
This stripping is a restoration of mainline behavior:
https://github.com/openjdk/valhalla/blob/7695b1f9c25921c25af3a37c70c3830b2c9cd41d/src/hotspot/share/oops/instanceKlass.cpp#L3402-L3403
> src/hotspot/share/prims/jvmtiEnv.cpp line 2744:
>
>> 2742: result |= JVM_ACC_SUPER;
>> 2743: }
>> 2744: *modifiers_ptr = result;
>
> I'm not sure I understand the high-level here, why does JVMTI need to provide
> `ACC_SUPER` every time? From the JVMS:
>> In Java SE 8, the ACC_SUPER semantics became mandatory, regardless of the
>> setting of ACC_SUPER or the class file version number, and the flag no
>> longer had any effect. Now the flag has been repurposed as ACC_IDENTITY.
>
> Does this not mean that with historically-compiled class files, `ACC_SUPER,`
> could be omitted and not be present, and then `GetClassModifiers` would
> return a modifier that didn't exist in the original classfile?
You are right, `GetClassModifiers` would include redundant `ACC_SUPER` not
present in the original classfile. However, this is for mainline parity:
https://github.com/openjdk/valhalla/blob/7695b1f9c25921c25af3a37c70c3830b2c9cd41d/src/hotspot/share/prims/jvmtiEnv.cpp#L2740-L2743
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2209#discussion_r2941456566
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2209#discussion_r2941464439
