Request for review: JDK-8011260: fatal error: LineNumberTable attribute has wrong length in class file

[serguei.spitsyn at oracle.com]

On 4/11/13 9:31 AM, serguei.spitsyn at oracle.com wrote:
> The fix looks good modulo the bootstrap anonymous classes.

To be clear, I've reviewed the second webrev.

Thanks,
Serguei

> How many anonymous classes are loaded by bootstrap CL?
> Is it worth to care about?
>
> Thanks,
> Serguei
>
> On 4/11/13 5:51 AM, harold seigel wrote:
>> Hi David,
>>
>> >> /But if I understand the fix correctly we are now verifying all 
>> anonymous classes regardless of which loader they are associated - is 
>> that right? /
>>
>> Yes, that is right.  We will now verify all anonymous classes 
>> regardless of loader.
>>
>> Thanks, Harold
>>
>> On 4/10/2013 10:42 PM, David Holmes wrote:
>>> On 11/04/2013 12:27 AM, harold seigel wrote:
>>>> Hi David,
>>>>
>>>> Thanks for looking at this.  To clarify, by "anonymous classes" I mean
>>>> only classes loaded by the anonymous class loader, not anonymous inner
>>>> classes.
>>>
>>> Of course :)
>>>
>>>> Please see in-lined responses.
>>>>
>>>> Thanks, Harold
>>>>
>>>> On 4/9/2013 8:03 PM, David Holmes wrote:
>>>>> Hi Harold,
>>>>>
>>>>> On 9/04/2013 11:55 PM, harold seigel wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review the following bug fix:
>>>>>>
>>>>>> Summary:   The assertion was prevented by changing the code to 
>>>>>> verify
>>>>>> classes loaded by the anonymous class loader, unless -noverify was
>>>>>
>>>>> So if I understand this right:
>>>>>
>>>>> - previously: loader == NULL -> BytecodeVerificationLocal
>>>>> - now: loader == NULL && anonymous-loader ->
>>>>> BytecodeVerificationRemote
>>>>>
>>>>> in other words we were skipping verification for anonymous classes.
>>>> Yes, unless full verification (-verify) was explicitly specified or
>>>> their host classes had their own non-null class loaders.
>>>
>>> I'm still a little confused - sorry. Normally we don't verify 
>>> boot-loader classes (unless requested). So I would have expected 
>>> that anonymous classes associated with the boot-loader would also 
>>> not get verified. But if I understand the fix correctly we are now 
>>> verifying all anonymous classes regardless of which loader they are 
>>> associated - is that right?
>>>
>>> Thanks,
>>> David
>>>
>>>>>
>>>>> I guess I don't fully understand the implications here. Given we
>>>>> normally skip verification for classes loaded by the boot-loader, 
>>>>> does
>>>>> this mean we now verify anonymous classes "linked" to the 
>>>>> boot-loader?
>>>>> Are all anonymous classes considered to be non-boot-loader classes?
>>>>>
>>>> Yes, we would now verify anonymous classes "linked" to the 
>>>> boot-loader.
>>>>
>>>> The answer to 'should anonymous calls considered to be non-boot-loader
>>>> classes" might be yes and no, depending on the context.  And this 
>>>> change
>>>> proposes that the answer be 'no' when it comes to whether or not to 
>>>> skip
>>>> verification.
>>>>
>>>>> Makes me wonder if there are other places where we check a loader for
>>>>> NULL but also need to check it is not related to anonymous classes ?
>>>>>
>>>> That's a good point.  Should I create a new bug to look into this?
>>>>>> specified.  Also, the relevant assertion messages were improved to
>>>>>> include additional information, including class name.
>>>>>
>>>>> Thanks for that. I hadn't realized the classfile_parse_error routines
>>>>> created and threw exceptions.
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_8011260/
>>>>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8011260/>
>>>>>>
>>>>>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=8011260
>>>>>>
>>>>>> The change was tested by hand to ensure that the assertion no longer
>>>>>> occurs when the test is run.  Additional testing was done using 
>>>>>> JCK Lang
>>>>>> and VM, JTREG tests, ute vm.quick.testlist and vm.mlvm.testlist 
>>>>>> tests,
>>>>>> and JPRT tests.
>>>>>>
>>>>>> Thank you!
>>>>>> Harold
>>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130411/f0551202/attachment.html