RFR(S) 8244764 Improve assertion for CPP_VTABLE_PATCH_TYPES_DO

[coleen.phillimore at oracle.com]

+1 this looks good.
Coleen

On 6/24/20 2:37 PM, Lois Foltan wrote:
>
>
> On 6/24/2020 2:28 PM, Ioi Lam wrote:
>>
>>
>> On 6/24/20 9:23 AM, Lois Foltan wrote:
>>> On 6/23/2020 8:09 PM, Ioi Lam wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8244764
>>>> http://cr.openjdk.java.net/~iklam/jdk16/8244764-CPP_VTABLE_PATCH_TYPES_DO.v01/ 
>>>>
>>>>
>>>> When a new subtype of MetaData is added, CPP_VTABLE_PATCH_TYPES_DO 
>>>> must to be
>>>> updated. However, there was no assertion check for this. In the 
>>>> past, projects
>>>> like Valhalla and Loom would get weird crashes with CDS after they 
>>>> added new
>>>> subclassesof Klass.
>>>>
>>>> With this patch, you will get a fatal error like this:
>>>>
>>>> # fatal error: Cannot find C++ vtable for 0x000000080002ec40 -- you
>>>>   probably added a new subtype of Klass or MetaData without updating
>>>>   CPP_VTABLE_PATCH_TYPES_DO
>>>>
>>>>
>>>> (The code is getting messy. I will clean it up in a subtask:
>>>> JDK-8248186 Move CDS C++ vtable code to cppVtables.cpp)
>>>>
>>>> Thanks
>>>> - Ioi
>>>
>>> Looks good Ioi.  One comment:
>>>
>>> share/memory/metaspaceShared.cpp:
>>> line #977: In the switch statement, MetaspaceObj::MethodDataType 
>>> will now be accepted without complaint even though the former 
>>> comment stated "should have been removed in removed_unsharable_info"?
>>>
>>
>> Hi Lois, thanks for the review.
>>
>> Good catch. I've added the ShouldNotReachHere back:
>>
>>
>> diff -r fab75f91cdfb src/hotspot/share/memory/metaspaceShared.cpp
>> --- a/src/hotspot/share/memory/metaspaceShared.cpp    Tue Jun 23 
>> 17:00:46 2020 -0700
>> +++ b/src/hotspot/share/memory/metaspaceShared.cpp    Wed Jun 24 
>> 11:26:25 2020 -0700
>> @@ -972,6 +972,10 @@
>>    case MetaspaceObj::RecordComponentType:
>>      // These have no vtables.
>>      break;
>> +  case MetaspaceObj::MethodDataType:
>> +    // We don't archive MethodData <-- should have been removed in 
>> removed_unsharable_info
>> +    ShouldNotReachHere();
>> +    break;
>>    default:
>>      for (kind = 0; kind < _num_cloned_vtable_kinds; kind ++) {
>>
>> Thanks
>> - Ioi
>>
>
> Looks good!  Thanks for fixing!
> Lois
>
>>
>>
>>> Thanks,
>>> Lois
>>
>