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

[David Holmes]

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
>>>>>