RFR(XS) 8191585 VM anonymous classes created during CDS dump time cause crash

[harold seigel]

Hi Ioi,

The new changes look good.

Thanks, Harold


On 3/6/2018 3:15 PM, Ioi Lam wrote:
> Hi Harold,
>
> Thanks for catching this. I've moved the check into do_cld() as you 
> suggested. I also added comments in 
> cld::is_{platform,system}_class_loader_data(), similar to the existing 
> comments in cld::is_boot_class_loader_data().
>
> http://cr.openjdk.java.net/~iklam/jdk11/8191585-vm-anon-classes-cause-dumping-crash.v02/ 
>
>
> Thanks
>
> - Ioi
>
>
> On 3/6/18 11:45 AM, harold seigel wrote:
>> Hi Ioi,
>>
>> This change will cause 
>> ClassLoaderData::is_builtin_class_loader_data() to return false for 
>> all anonymous classes except for the boot loader.
>>
>> Also, other callers of is_builtin_class_loader_data(), such as 
>> Klass::class_loader_and_module_name(), probably want that function to 
>> return true even for anonymous CLDs.
>>
>> Instead of changing is_platform_class_loader_data() and 
>> is_system_class_loader_data(), could you add a !is_anonymous() check 
>> to do_cld() ?
>>
>> Or, add new is_*_class_loader_data() methods that only return true if 
>> the CLD is not anonymous?
>>
>> Thanks, Harold
>>
>>
>> On 3/6/2018 2:20 PM, Ioi Lam wrote:
>>> Hi,
>>>
>>> Please review this small VM change and the corresponding test case.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8191585
>>> http://cr.openjdk.java.net/~iklam/jdk11/8191585-vm-anon-classes-cause-dumping-crash.v01/ 
>>>
>>>
>>> The crash happens in the following code:
>>>
>>> class CombineDictionariesClosure : public CLDClosure {
>>>     void do_cld(ClassLoaderData* cld) {
>>>       if (cld->is_system_class_loader_data() || 
>>> cld->is_platform_class_loader_data()) {
>>>         for (int i = 0; i < cld->dictionary()->table_size(); ++i) {
>>>                                  ^^^^^^^^^^^ == NULL
>>>
>>> The bug is in cld->is_system_class_loader_data() -- the CLD of anon 
>>> classes have a NULL
>>> cld::dictionary(), but its cld::class_loader() returns the loader 
>>> object of the holder
>>> class.
>>>
>>> The fix is to reject if cld::is_anonymous() is true.
>>>
>>> Thanks
>>> - Ioi
>>
>