RFR 8174725: JVM should throw ClassDefNotFoundError if ACC_MODULE is set in access_flags

David Holmes david.holmes at oracle.com
Wed Feb 15 22:38:21 UTC 2017


Hi Harold,

On 15/02/2017 11:30 PM, harold seigel wrote:
> Hi David,
>
> I did not put the check in verify_legal_class_modifiers() because it
> only checks the modifiers if _need_verify is set.  However, we want to
> always check for ACC_MODULE (in class files >= 53) regardless of
> _need_verify's value.  So verify_legal_class_modifiers() would need a
> special case to check for ACC_MODULE.

That raises the question as to what classfile "format" checks are 
unconditional and which are only carried out when "verification" is 
enabled ??

> Also, since a classfile version check is already needed in
> parse_stream(), when fetching the access_flags from the stream, it was
> convenient to just do the check and possible throw right there.
> However, if you think it's important, I'll move the check to
> verify_legal_class_modifiers().

It seems like a good place for it (not withstanding it currently returns 
immediately) and avoid the need to duplicate the code. It is the 
duplication I disliked - but "verify_legal_class_modifiers" certainly 
sounds like the place that would check for ACC_MODULE.

> Thanks for proposing a better error message.  Instead of saying 'claims
> to be a module', how about:
>
>     "% is not a class because access_flag ACC_MODULE is set"

Okay I guess.

Thanks,
David

> Thanks, Harold
>
> On 2/14/2017 8:00 PM, David Holmes wrote:
>> Hi Harold,
>>
>> On 15/02/2017 9:53 AM, harold seigel wrote:
>>> Hi,
>>>
>>> Please review this updated webrev:
>>> http://cr.openjdk.java.net/~hseigel/bug_8174725.2/webrev/index.html
>>
>> I'm unclear why this logic is needed in two places instead of just
>> putting it inside ClassFileParser::verify_legal_class_modifiers ?
>>
>> Also with regards to the error message ... I think it is expressed
>> inappropriately for the NCDFE case. If we were throwing
>> ClassFormatError then "Illegal ACC_MODULE class modifier ..." would be
>> appropriate. But for NCDFE we need to phrase it in terms of the
>> inability to find the class in the given class representation - I
>> suggest:
>>
>> "%s claims to be a module (ACC_MODULE is set)"
>>
>> Thanks,
>> David
>> -----
>>
>>> This webrev has 'return' statements after the calls to fthrow() and a
>>> new test case for a class file in an InnerClasses attribute that has
>>> ACC_MODULE set.
>>>
>>> Thanks, Harold
>>>
>>>
>>> On 2/14/2017 10:32 AM, Alan Bateman wrote:
>>>>
>>>>
>>>> On 14/02/2017 14:52, harold seigel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this small change to throw a NoClassDefFoundError
>>>>> exception, for class file versions >= 53, if a class's access_flags
>>>>> have ACC_MODULE set.  This behavior will be required in the upcoming
>>>>> JVM-9 Spec.
>>>>>
>>>>> Open Web:
>>>>> http://cr.openjdk.java.net/~hseigel/bug_8174725/webrev/index.html
>>>>>
>>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8174725
>>>>>
>>>>>
>>>>> The fix was tested with JPRT, the hotspot, java/lang, java/util,
>>>>> java/io, JFR, and other JTReg tests, the JCK lang and VM tests, RBT
>>>>> tier2 - tier5 tests on LinuxX64, and the colocated and non-colocated
>>>>> NSK tests.
>>>> This looks okay to me although I think I would name the test case
>>>> "BadAccModule" rather than "badAccModule" as it's a bit unusual to
>>>> have a class name starting with a lower case letter.
>>>>
>>>> -Alan
>>>
>


More information about the hotspot-runtime-dev mailing list