RFR 8176147: JVM should throw CFE for duplicate Signature attributes

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 8176147: JVM should throw CFE for duplicate Signature attributes

harold seigel
Hi,

Please review this JDK-9 fix to throw a ClassFormatError exception if
the attributes table for a class, field, or method contains more than
one Signature attribute.

Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8176147/webrev/

JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8176147

The fix was tested with the JCK lang and vm tests, the JTreg hotspot,
java/io, java/lang, java/util and other tests, the RBT tier2 -tier5
tests, the colocated and non-colocated NSK tests, and with JPRT.

Thanks, Harold

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8176147: JVM should throw CFE for duplicate Signature attributes

George Triantafillou
Hi Harold,

Looks good!

-George

On 3/7/2017 8:52 AM, harold seigel wrote:

> Hi,
>
> Please review this JDK-9 fix to throw a ClassFormatError exception if
> the attributes table for a class, field, or method contains more than
> one Signature attribute.
>
> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8176147/webrev/
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8176147
>
> The fix was tested with the JCK lang and vm tests, the JTreg hotspot,
> java/io, java/lang, java/util and other tests, the RBT tier2 -tier5
> tests, the colocated and non-colocated NSK tests, and with JPRT.
>
> Thanks, Harold
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8176147: JVM should throw CFE for duplicate Signature attributes

harold seigel
Thanks George!

Harold


On 3/7/2017 9:09 AM, George Triantafillou wrote:

> Hi Harold,
>
> Looks good!
>
> -George
>
> On 3/7/2017 8:52 AM, harold seigel wrote:
>> Hi,
>>
>> Please review this JDK-9 fix to throw a ClassFormatError exception if
>> the attributes table for a class, field, or method contains more than
>> one Signature attribute.
>>
>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8176147/webrev/
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8176147
>>
>> The fix was tested with the JCK lang and vm tests, the JTreg hotspot,
>> java/io, java/lang, java/util and other tests, the RBT tier2 -tier5
>> tests, the colocated and non-colocated NSK tests, and with JPRT.
>>
>> Thanks, Harold
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8176147: JVM should throw CFE for duplicate Signature attributes

David Holmes
In reply to this post by harold seigel
Hi Harold,

Changes look good.

To avoid duplication in the tests I've been using this pattern for the
JDK 10 work I'm doing that's checking classfile format:

      public static void main(String args[]) throws Throwable {
          String[] badClasses = new String[] {
              "class1",
              "class2",
              "class3",
          };
          String[] messages = new String[] {
              "message1",
              "message2",
              "message3",
          };

          for (int i = 0; i < badClasses.length; i++ ) {
              try {
                  Class c = Class.forName(badClasses[i]);
                  throw new Error("Missing ClassFormatError: " +
messages[i]);
              }
              catch (ClassFormatError expected) {
                  if (!expected.getMessage().contains(messages[i]))
                     throw new Error("Wrong ClassFormatError message: \"" +
                                     expected.getMessage() + "\" does
not contain \"" +
                                     messages[i] + "\"");
                  System.out.println("OK - got expected exception: " +
expected);
              }
          }
      }

Cheers,
David

On 7/03/2017 11:52 PM, harold seigel wrote:

> Hi,
>
> Please review this JDK-9 fix to throw a ClassFormatError exception if
> the attributes table for a class, field, or method contains more than
> one Signature attribute.
>
> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8176147/webrev/
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8176147
>
> The fix was tested with the JCK lang and vm tests, the JTreg hotspot,
> java/io, java/lang, java/util and other tests, the RBT tier2 -tier5
> tests, the colocated and non-colocated NSK tests, and with JPRT.
>
> Thanks, Harold
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8176147: JVM should throw CFE for duplicate Signature attributes

David Holmes
Sorry just noticed something minor ...

For the class signature attribute you could just check if
_generic_signature_index == 0 instead of adding the local
signature_exists flag.

David

On 8/03/2017 12:13 PM, David Holmes wrote:

> Hi Harold,
>
> Changes look good.
>
> To avoid duplication in the tests I've been using this pattern for the
> JDK 10 work I'm doing that's checking classfile format:
>
>      public static void main(String args[]) throws Throwable {
>          String[] badClasses = new String[] {
>              "class1",
>              "class2",
>              "class3",
>          };
>          String[] messages = new String[] {
>              "message1",
>              "message2",
>              "message3",
>          };
>
>          for (int i = 0; i < badClasses.length; i++ ) {
>              try {
>                  Class c = Class.forName(badClasses[i]);
>                  throw new Error("Missing ClassFormatError: " +
> messages[i]);
>              }
>              catch (ClassFormatError expected) {
>                  if (!expected.getMessage().contains(messages[i]))
>                     throw new Error("Wrong ClassFormatError message: \"" +
>                                     expected.getMessage() + "\" does not
> contain \"" +
>                                     messages[i] + "\"");
>                  System.out.println("OK - got expected exception: " +
> expected);
>              }
>          }
>      }
>
> Cheers,
> David
>
> On 7/03/2017 11:52 PM, harold seigel wrote:
>> Hi,
>>
>> Please review this JDK-9 fix to throw a ClassFormatError exception if
>> the attributes table for a class, field, or method contains more than
>> one Signature attribute.
>>
>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8176147/webrev/
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8176147
>>
>> The fix was tested with the JCK lang and vm tests, the JTreg hotspot,
>> java/io, java/lang, java/util and other tests, the RBT tier2 -tier5
>> tests, the colocated and non-colocated NSK tests, and with JPRT.
>>
>> Thanks, Harold
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8176147: JVM should throw CFE for duplicate Signature attributes

harold seigel
Hi David,

Thanks for the review!

I'll make the _generic_signature_index and test changes before pushing.

I added the signature_exists flag instead of checking if
_generic_signature_index == 0 because the setting of
_generic_signature_index is, in a sense, a side effect of calling
parse_classfile_signature_attribute().  But, its implementation is
unlikely to change.

Thanks, Harold

On 3/7/2017 9:23 PM, David Holmes wrote:

> Sorry just noticed something minor ...
>
> For the class signature attribute you could just check if
> _generic_signature_index == 0 instead of adding the local
> signature_exists flag.
>
> David
>
> On 8/03/2017 12:13 PM, David Holmes wrote:
>> Hi Harold,
>>
>> Changes look good.
>>
>> To avoid duplication in the tests I've been using this pattern for the
>> JDK 10 work I'm doing that's checking classfile format:
>>
>>      public static void main(String args[]) throws Throwable {
>>          String[] badClasses = new String[] {
>>              "class1",
>>              "class2",
>>              "class3",
>>          };
>>          String[] messages = new String[] {
>>              "message1",
>>              "message2",
>>              "message3",
>>          };
>>
>>          for (int i = 0; i < badClasses.length; i++ ) {
>>              try {
>>                  Class c = Class.forName(badClasses[i]);
>>                  throw new Error("Missing ClassFormatError: " +
>> messages[i]);
>>              }
>>              catch (ClassFormatError expected) {
>>                  if (!expected.getMessage().contains(messages[i]))
>>                     throw new Error("Wrong ClassFormatError message:
>> \"" +
>>                                     expected.getMessage() + "\" does not
>> contain \"" +
>>                                     messages[i] + "\"");
>>                  System.out.println("OK - got expected exception: " +
>> expected);
>>              }
>>          }
>>      }
>>
>> Cheers,
>> David
>>
>> On 7/03/2017 11:52 PM, harold seigel wrote:
>>> Hi,
>>>
>>> Please review this JDK-9 fix to throw a ClassFormatError exception if
>>> the attributes table for a class, field, or method contains more than
>>> one Signature attribute.
>>>
>>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8176147/webrev/
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8176147
>>>
>>> The fix was tested with the JCK lang and vm tests, the JTreg hotspot,
>>> java/io, java/lang, java/util and other tests, the RBT tier2 -tier5
>>> tests, the colocated and non-colocated NSK tests, and with JPRT.
>>>
>>> Thanks, Harold
>>>