Request for review (M / Verifier Error messages)

[serguei.spitsyn at oracle.com]

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


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