JDK 10 RFR JDK-8171194: Exception "Duplicate field name&signature in class file" should report the name and signature of the field

[coleen.phillimore at oracle.com]

Yes this looks very good.  Do you have a test for it?  Or is it in the 
jck tests and verified manually for each case?

Thanks,
Coleen

On 2/15/17 1:29 AM, Shafi Ahmad wrote:
> Thank you David for reviewing it.
>
> All,
>
> May I get second reviewer for this trivial change.
>
> Regards,
> Shafi
>
>
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Wednesday, February 15, 2017 11:54 AM
>> To: Shafi Ahmad <shafi.s.ahmad at oracle.com>; hotspot-
>> dev at openjdk.java.net
>> Subject: Re: JDK 10 RFR JDK-8171194: Exception "Duplicate field
>> name&signature in class file" should report the name and signature of the
>> field
>>
>> Looks good to me! Nice and neat.
>>
>> Thanks,
>> David
>>
>> On 15/02/2017 4:02 PM, Shafi Ahmad wrote:
>>> Hi David,
>>>
>>> Please find updated webrev at
>>> http://cr.openjdk.java.net/~shshahma/8171194/webrev.01/
>>> Added overloaded method classfile_parse_error.
>>>
>>> Regards,
>>> Shafi
>>>
>>>> -----Original Message-----
>>>> From: David Holmes
>>>> Sent: Wednesday, February 15, 2017 3:55 AM
>>>> To: Shafi Ahmad <shafi.s.ahmad at oracle.com>; hotspot-
>>>> dev at openjdk.java.net
>>>> Subject: Re: JDK 10 RFR JDK-8171194: Exception "Duplicate field
>>>> name&signature in class file" should report the name and signature of
>>>> the field
>>>>
>>>> Hi Shafi,
>>>>
>>>> On 14/02/2017 11:20 PM, Shafi Ahmad wrote:
>>>>> Hi David,
>>>>>
>>>>> Thanks for reviewing it.
>>>>>
>>>>> Initially I started with fixed size of local char array but later I
>>>>> changed my
>>>> mind and make it dynamic.
>>>>> Let me know if I have to make it local char array like.
>>>>>
>>>>> +         unsigned int siglength = sig->utf8_length();
>>>>> +         unsigned int namelength = name->utf8_length();
>>>>> +         unsigned int length = siglength + namelength + 64;
>>>>> +         char *buff = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char,
>>>> length);
>>>>> +         jio_snprintf(buff, length,
>>>>> +                     "Duplicate method name \"%s\" with signature
>>>>> + \"%s\" in class
>>>> file",
>>>>> +                     name->as_C_string(), sig->as_klass_external_name());
>>>>> +         classfile_parse_error("%s %s", buff, CHECK);
>>>>>         }
>>>>>
>>>>> to
>>>>>
>>>>> +         char buff[fixedsize];  // say fixedsize is 512
>>>>> +         jio_snprintf(buff, 512,
>>>>> +                     "Duplicate method name \"%s\" with signature
>>>>> + \"%s\" in class
>>>> file",
>>>>> +                     name->as_C_string(), sig->as_klass_external_name());
>>>>> +         classfile_parse_error("%s %s", buff, CHECK);
>>>>>         }
>>>> It could still be dynamic, you just need to use
>>>> NEW_RESOURCE_ARRAY_IN_THREAD_RETURN_NULL and check for a
>> NULL return,
>>>> and fallback to not including the additional info. But the underlying
>>>> Exceptions::fthrow uses a local buffer itself (1024 max), so you
>>>> could just do the same as you propose above.
>>>>
>>>> Though it is very annoying to have to allocate a buffer just to pass
>>>> it through to classfile_parse_error which passes it through to
>>>> Exceptions::fthrow which is a varargs method. So another possibility
>>>> is to just add another overload of classfile_parse_error that takes
>>>> in the two additional strings you want to print.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Regards,
>>>>> Shafi
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: David Holmes
>>>>>> Sent: Tuesday, February 14, 2017 6:34 PM
>>>>>> To: Shafi Ahmad <shafi.s.ahmad at oracle.com>; hotspot-
>>>>>> dev at openjdk.java.net
>>>>>> Subject: Re: JDK 10 RFR JDK-8171194: Exception "Duplicate field
>>>>>> name&signature in class file" should report the name and signature
>>>>>> of the field
>>>>>>
>>>>>> Hi Shafi,
>>>>>>
>>>>>> I'm concerned about the use of NEW_RESOURCE_ARRAY_IN_THREAD.
>> If
>>>> it
>>>>>> can't allocate it will abort the VM. That seems like a bad thing to
>> happen.
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 14/02/2017 7:19 PM, Shafi Ahmad wrote:
>>>>>>> Summary: java.lang.ClassFormatError: Exception "Duplicate field
>>>>>> name&signature in class file" should report the name and signature
>>>>>> of the field.
>>>>>>> It's a very small change to single file.
>>>>>>> In the current implementation name and signature of duplicate
>>>>>>> field is
>>>>>> missing in java.lang.ClassFormatError exception message.
>>>>>>> Without a field name + signature it is hard to triggering the problem.
>>>>>>>
>>>>>>> Webrev link:
>>>> http://cr.openjdk.java.net/~shshahma/8171194/webrev.00/
>>>>>>> bug link: https://bugs.openjdk.java.net/browse/JDK-8171194
>>>>>>>
>>>>>>> Testing: jprt and jtreg test.
>>>>>>>
>>>>>>> I have verified my changes with the reproduces of
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8080842 on jdk8u60-b01
>>>>>> code base as I was not able to write reproducer of current issue.
>>>>>>> With the fix I am getting below exception message.
>>>>>>>
>>>>>>> $ java CreateBadClassFile
>>>>>>> .foreach() call: Exception in thread "main" java.lang.ClassFormatError:
>>>>>> Duplicate field name "hasNext" with signature "Ljava.lang.Object;"
>>>>>> in class file WidgetCollection$1
>>>>>>> 	at java.lang.ClassLoader.defineClass1(Native Method)
>>>>>>> 	at java.lang.ClassLoader.defineClass(ClassLoader.java:760)
>>>>>>> 	at
>>>>>> java.security.SecureClassLoader.defineClass(SecureClassLoader.java:
>>>>>> 14
>>>>>> 2)
>>>>>>>                 . . .
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>