RFR 8056900: Enhance NoClassDefFound exception messaging

[serguei.spitsyn at oracle.com]

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