RFR: JDK-8245432: Lookup::defineHiddenClass should throw UnsupportedClassVersionError if the given bytes are of an unsupported major or minor version

[Mandy Chung]

On 5/28/20 5:44 PM, David Holmes wrote:
>>
>> This is to validate the given version.  The runtime will check if 
>> preview feature is enabled when such class file is loaded. I will 
>> make a comment to make it clear.
>
> Okay but I thought the intent here was to pre-validate the version 
> information so that when these bytes get passed to ASM you don't have 
> to worry about the IAE that will be thrown by ASM if there is actually 
> a problem.

Yes it is.  ASM does not check if preview features are enabled or not 
neither.  When a class file depending preview features is passed to VM, 
the VM will throw an exception if preview features are not enabled.
>
> Maybe the only real solution here is for ASM to be more specific with 
> the exceptions it throws. :(
>

This was discussed.
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/066734.html

>
> Sure but we provide that kind of cross-package access all the time. We 
> also have JAVA_MAX_SUPPORTED_VERSION in the ModuleInfo class. Seems 
> messy to add yet a third place where we need to determine what the 
> current major version number is.
>

Ah, that's another place.  I think it's better to add 
VM::isSupportedModuleDescriptorVersion and remove these constants.

> That aside isn't the minor version, as set in java.class.version 
> guaranteed to be zero?
>
This is set at build time.   The minor version is zero for the current 
versioning scheme.

Mandy

> David
> -----
>
>> Mandy
>>> Cheers,
>>> David
>>>
>>>> Mandy
>>>>
>>>> On 5/27/20 10:57 AM, Mandy Chung wrote:
>>>>> I'm reconsidering this fix along with JDK-8245061 that may require 
>>>>> to do its own checking (a similar issue w.r.t. ASM validation but 
>>>>> in this case the constant pool entry of `this_class` item is not 
>>>>> validated).
>>>>>
>>>>> thanks
>>>>> Mandy
>>>>>
>>>>> On 5/27/20 10:39 AM, forax at univ-mlv.fr wrote:
>>>>>> Hi Alan,
>>>>>> We (the ASM team) recommend to our users to check the byte 6 (and 
>>>>>> perhaps 7) instead of relying on ASM throwing an exception,
>>>>>> because you may update the version of ASM but not the visitors 
>>>>>> your are using in your code.
>>>>>>
>>>>>> It's less brittle than catching the IAE thrown by ASM.
>>>>>>
>>>>>> Rémi
>>>>>>
>>>>>> ----- Mail original -----
>>>>>>> De: "Alan Bateman" <Alan.Bateman at oracle.com>
>>>>>>> À: "mandy chung" <mandy.chung at oracle.com>, "core-libs-dev" 
>>>>>>> <core-libs-dev at openjdk.java.net>, "Remi Forax"
>>>>>>> <forax at univ-mlv.fr>
>>>>>>> Envoyé: Mercredi 27 Mai 2020 18:16:33
>>>>>>> Objet: Re: RFR: JDK-8245432: Lookup::defineHiddenClass should 
>>>>>>> throw UnsupportedClassVersionError if the given bytes are
>>>>>>> of an unsupported major or minor version
>>>>>>> On 26/05/2020 22:46, Mandy Chung wrote:
>>>>>>>> Lookup::defineHiddenClass currently throws IAE by ASM if the given
>>>>>>>> bytes are of unsupported class file version.  The implementation
>>>>>>>> should catch and throw UnsupportedClassVersionError instead.
>>>>>>>>
>>>>>>>> webrev:
>>>>>>>> http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8245432/webrev.00/ 
>>>>>>>>
>>>>>>>>
>>>>>>>> This patch also includes a spec clarification of @throws IAE if 
>>>>>>>> the
>>>>>>>> the bytes has ACC_MODULE flag set to fix JDK-8245596.
>>>>>>> Rémi - has there ever been any discussion in ASM about throwing 
>>>>>>> more
>>>>>>> specific exceptions? Only asking to see if we could avoid 
>>>>>>> needing to
>>>>>>> depend on the exception message here.
>>>>>>>
>>>>>>> -Alan
>>>>>
>>>>
>>