Request for review (M / Verifier Error messages)

Keith McGuigan keith.mcguigan at oracle.com
Thu Aug 2 06:01:02 PDT 2012


Hi Karen,

Thanks for the review!  Comments inline:

On 8/1/2012 4:29 PM, Karen Kinnear wrote:
> Keith,
>
> First - thank you for doing this and for attaching the detailed output information,
> which I think will be very useful.
>
> Looks good and I like the way you structured this.
>
> Minor questions/comments:
> 1. stackMapFrame.hpp lines 230-231: is that comment still accurate?

No, that comment is out-of-date now (mostly).  If anything it belongs in 
set_mark(), where we actually "bogus-out" the slots.  I'll move it there.

> and stackMapFrame.cpp lines 225-226 - are these still true?

Yes, this one is still accurate.

> 2. FLAGS_MISMATCH/bad_flags:
> Would there be any value in printing out what the flags are? Or is that
> obvious?

Whenever this type of error occurs the current stack state and the 
stackmap frame will both be printed, so it will be obvious what the 
mismatch is (especially considering there's only one type of flag at the 
moment).  It looks like my tests are missing this case -- I'll try to 
add it.

> 3. StackMapTable.hpp/cpp
> would it be hard to rename print to print_on since you now pass it a stream?
> Also end of print, you call tty->print_cr, don't you want to use str not tty?

I'll definitely fix the tty => str.  I think I did at one point try to 
make this some of these print() calls be print_on(), but that conflicted 
with a "print_on() const" in one of the allocation superclasses -- so in 
order to name it that I'd have to make the function (and everything it 
calls) const-safe.  I'll try again and see if I can get it to work 
without major surgery.

Thanks!
--
- Keith

>
> thanks,
> Karen
>
> On Aug 1, 2012, at 1:36 PM, Keith McGuigan wrote:
>
>>
>> Anyone?
>>
>> On 7/26/2012 6:02 PM, Keith McGuigan wrote:
>>> Hello,
>>>
>>> Would appreciate any review of the following code:
>>> http://cr.openjdk.java.net/~kamg/7116786/
>>>
>>> This code adds additional information to VerifyError messages to make it
>>> easier to diagnose bytecode problems in the field.  It may be a full (or
>>> partial) solution for JEP 136 (http://openjdk.java.net/jeps/136).
>>>
>>> The basic idea is to create an ErrorContext object when a verification
>>> error occurs and populate it with context information so that things
>>> like the current frame, stackmaps, and bytecodes can be included in the
>>> VerifyError's message.  In addition a diagnostic flag,
>>> -XX:+VerifyVerbose is added which prints out the details of the
>>> verification as it occurs.
>>>
>>> The included test case has a 'testcases.jar' file that contains
>>> hand-modified classfiles that trigger VerifyErrors in each possible
>>> location in the verifier.  Some cases appeared unreachable so those
>>> cases are missing.  I'm attaching the output of the test so you may see
>>> the resulting error message format.
>>>
>>> --
>>> - Keith
>


More information about the hotspot-runtime-dev mailing list