Request for review: JDK-8011260: fatal error: LineNumberTable attribute has wrong length in class file
harold seigel
harold.seigel at oracle.com
Thu Apr 11 11:45:15 PDT 2013
Hi Sergui,
Thanks for reviewing this change.
I set a breakpoint at /Verifier::should_verify_for()/, conditioned on
/is_anon_loader/ being TRUE, and then ran the Queens program. The
breakpoint did not trigger. So I don't think any anonymous classes are
loaded unless explicitly requested by user code.
bool Verifier::should_verify_for(oop class_loader, bool
should_verify_class, bool is_anon_loader) {
return ((class_loader == NULL && !is_anon_loader) ||
!should_verify_class) ?
BytecodeVerificationLocal : BytecodeVerificationRemote;
}
Thanks, Harold
On 4/11/2013 12:40 PM, serguei.spitsyn at oracle.com wrote:
> 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/5dbfe38d/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list