Request for review (M / Verifier Error messages)

[Keith McGuigan]

Thanks Serguei!

I'll make the requested changes.

--
- Keith

On 8/1/2012 10:46 PM, serguei.spitsyn at oracle.com wrote:
> Keith,
>
> Impressive changes!
> They look good in general.
> A few minor comments below.
>
> || *src/share/vm/classfile/stackMapFrame.cpp
>
> *It seems a tab instead of space after the second comma on the line 221:
>
>   216         _offset, (StackMapFrame*)this, (StackMapFrame*)target);
>   221         _offset, (StackMapFrame*)this,   (StackMapFrame*)target);
>
> A question:
>    Would it make sense to simplify (just a little bit) from:
>      244   if (!match_flags &&
>     245       (!is_exception_handler || !has_flag_match_exception(target))) {
>
>    to this:
>      244   if (!(match_flags || (is_exception_handler && has_flag_match_exception(target)))) {
>
> *src/share/vm/classfile/verifier.hpp*
>
> A typo:
> 97 SM_STACK, // Comes from stackmap exporession stack
>
> Consistency:
>    Every comment right below the line 144 starts with a capital letter:
> 144 INVALID_BYTECODE, // there was a problem with the bytecode
>
>
> And probably, you remember about copyright comments. :)
>
>
> Thanks,
> Serguei
>
>
> On 8/1/12 10:36 AM, 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
>
>