RFR 8056900: Enhance NoClassDefFound exception messaging

[serguei.spitsyn at oracle.com]

Hi Harold,

Thank you for the update.
It looks great!

One tip is that you can pre-define the static String constants:
   "Class name exceeds maximum length of "
   "defineClass did not throw expected NoClassDefFoundError".


But I leave it up to you.
No webrev is needed if you decide to tweak it.

Thanks,
Serguei


On 5/3/17 14:06, harold seigel wrote:
> Hi,
>
> Please review this updated RFR that includes test cases for the JNI 
> modifications.
>
> RFR: http://cr.openjdk.java.net/~hseigel/bug_8056900.3/webrev/index.html
>
> Thanks, Harold
>
>
> On 5/3/2017 3:39 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Harold,
>>
>> The fix looks good to me.
>> The test does not cover all the product changes.
>> Is it possible to extend it to check the JNI FindClass as well?
>>
>> Thanks,
>> Serguei
>>
>>
>>
>> On 5/3/17 07:23, harold seigel wrote:
>>> Hi,
>>>
>>> Please review this updated webrev: 
>>> http://cr.openjdk.java.net/~hseigel/bug_8056900.2/webrev/index.html
>>>
>>> It includes the below changes suggested by David.
>>>
>>> Thanks, Harold
>>>
>>>
>>> On 5/3/2017 12:32 AM, David Holmes wrote:
>>>> Hi Harold,
>>>>
>>>> On 3/05/2017 4:29 AM, harold seigel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this small JDK-10 change to enhance 
>>>>> NoClassDefFoundError
>>>>> exception messages.
>>>>>
>>>>> Open Webrev:
>>>>> http://cr.openjdk.java.net/~hseigel/bug_8056900/webrev/index.html
>>>>
>>>> A few comments:
>>>>
>>>> In src/share/vm/oops/instanceKlass.cpp
>>>>
>>>> 517     // This is for CDS dumping phase only -- we use the 
>>>> in_error_state to indicate that
>>>> 518     // the class has failed verification.
>>>>
>>>> That comment is somewhat misleading. The is_in_error_state() 
>>>> indicates that class initialization failed, not verification. 
>>>> However CDS dumping also uses it to indicate if a supertype was 
>>>> is_in_error_state(). I think using:
>>>>
>>>> 527                        "Verification failed for class %s",
>>>>
>>>> would be confusing to an end user trying to figure out how a 
>>>> perfectly valid class could fail verification! I suggest the more 
>>>> direct:
>>>>
>>>> "Class %s, or one of its supertypes, failed class initialization"
>>>>
>>>> ---
>>>>
>>>> src/share/vm/prims/jni.cpp
>>>>
>>>> Suggestion:
>>>>
>>>> "Class name exceeds maximum length of %d: %s", ...
>>>>
>>>> in both cases.
>>>>
>>>> 386 THROW_MSG_0(vmSymbols::java_lang_NoClassDefFoundError(), "Class 
>>>> name is null");
>>>>
>>>> Suggestion: "No class name given"
>>>>
>>>> ---
>>>>
>>>> Ditto for src/share/vm/prims/jvm.cpp changes.
>>>>
>>>> ---
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>
>>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8056900
>>>>>
>>>>> The change was tested with JCK tests, the JTreg hotspot, java/io,
>>>>> java/lang, java/util and other tests, the RBT tier2 -tier5 tests, the
>>>>> co-located NSK tests, JPRT, and with the new test provided with 
>>>>> the change.
>>>>>
>>>>> Thanks, Harold
>>>>>
>>>
>>
>