On Tue, 6 Dec 2022 03:33:25 GMT, Joe Darcy <da...@openjdk.org> wrote:
>> I would propose to say:
>>
>> Mask bits that do not match an {@code AccessFlag} for the location and
>> class file format version are ignored.
>>
>> The case arises when the mask argument contains mask bits that are not
>> consistent with the locations defined for any AccessFlag.
>> The current method throws IllegalArgumentException, while the added method
>> returns a set of AccessFlags appropriate to the location and cffv and
>> ignores the undefined mask bits.
>>
>> If the source of the error/inconsistency is an application developer calling
>> `maskToAccessFlags()` then IAE makes sense.
>> However, it would puzzling if a call to Class.accessFlags() threw
>> IllegalArgumentException; in that case the mask bits are those of a loaded
>> class, presumed to be conforming to the spec, but none the less having
>> unexpected mask bits.
>> Such an recurrence is likely very rare but might be the result of a class
>> file created by some means other than the javac compiler. The declaration of
>> each AccessFlag implements the location and cffv information and
>> corresponding mask bits according the JVM spec. If the VM loads the class,
>> then the question is whether the inconsistency should be reported and if so
>> how.
>
> From an end-user perspective, "the system" should screen out or otherwise
> take care of undefined/inappropriate modifier bits and access flags in loaded
> class files.
>
> In the JDK implementation, there is a separation between the HotSpot JVM and
> the class libraries, in this case core reflection. One could argue -- and as
> a maintainer of the core reflection libraries I would argue -- that HotSpot
> should screen out undefined/inappropriate access flag bits before presenting
> them to the user. If that is not done, with the non-public method to return
> the class file format version of the class file, the Java libraries side of
> core reflection has enough information to do such screening on its own.
> (IIRC, from some off-list discussions @RogerRiggs ran into cases where a
> hand-crafted class file could have the ACC_SYNTHETIC flag set on a class for
> a version of the class file format where ACC_SYNTHETIC wasn't defined. This
> flag was passed from HotSpot to the core libraries, running afoul of the
> stricter checking on the existing maskToAccessFlags method.)
>
> There are class file / byte code processing APIs where showing all the access
> flag bits, even when not defined for a class file versions, is the right
> answer. However, I don't think core reflection is one of those APIs. In the
> spirit of JVMS "4.1. The ClassFile Structure":
>
> "All bits of the access_flags item not assigned in [Table
> 4.1-B](https://docs.oracle.com/javase/specs/jvms/se19/html/jvms-4.html#jvms-4.1-200-E.1)
> are reserved for future use. They should be set to zero in generated class
> files and should be ignored by Java Virtual Machine implementations."
>
> I think it would be reasonable to mask out undefined bits either in HotSpot
> or the core reflection libraries.
Should the masking out unassigned bits that is done in this method be extended
to the existing `AccessFlag.maskToAccessFlags(mask, location)`; Instead of
throwing `IllegalArgumentException`?
The two methods should be consistent in this regard and build their return
values on the meta-data in each AccessFlag.
-------------
PR: https://git.openjdk.org/jdk/pull/11399
