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

[David Holmes]

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
>