RFR: 8297271: AccessFlags should be specific to class file version [v2]

[Roger Riggs]

On Tue, 6 Dec 2022 03:33:25 GMT, Joe Darcy <darcy at 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