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

[Shafi Ahmad]

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