RFR: 8167333: Invalid source path info might be used when creating ClassFileStream after CFLH transforms a shared classes in some cases

[Ioi Lam]

On 10/9/16 2:10 PM, David Holmes wrote:
> On 8/10/2016 4:23 AM, Jiangli Zhou wrote:
>> Hi David,
>>
>> Thanks for taking a look.
>>
>>> On Oct 6, 2016, at 10:33 PM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> Hi Jiangli,
>>>
>>> On 7/10/2016 2:39 PM, Jiangli Zhou wrote:
>>>> Hi,
>>>>
>>>> Please review the following fix for JDK-8167333
>>>> <https://bugs.openjdk.java.net/browse/JDK-8167333>:
>>>>
>>>>  webrev: http://cr.openjdk.java.net/~jiangli/8167333/webrev.00/
>>>> <http://cr.openjdk.java.net/~jiangli/8167333/webrev.00/>
>>>>
>>>> When a shared class is transformed by a JVMTI agent during initial
>>>> loading (via CFLH), the VM creates a new ClassFileStream using the
>>>> transformed class data. The source path info from the class’
>>>> associated SharedClassPathEntry is passed as the ‘source’ argument to
>>>> ClassFileStream. However, some shared classes may not have an
>>>> associated SharedClassPathEntry and the class_path_index is -1. The
>>>> VM needs to detect such case and not passing an invalid source path 
>>>> info.
>>>
>>> It isn't obvious to me that all callers of
>>> CFS::source()/clone_source() will handle getting a NULL. Of course I
>>> can't tell which of those callers may be involved in this particular
>>> use-case.
>>
>> I took a look of all the code that calls CFS::source()/clone_source().
>> They all handle the NULL case with explicit NULL check. For our specific
>> case, the particular caller involved
>> is InstanceKlass::print_loading_log. Before the fix, it crashed when
>> trying to print the invalid cfs->source after (cfs->source() != NULL) 
>> check.
>
> Thanks for verifying. I've looked at the latest webrev with the 
> additional asserts - all looks good.
>

Looks good to me, too. Thanks

- Ioi
> David
>
>> Thanks,
>> Jiangli
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> Tested with all existing class data sharing tests.
>>>>
>>>> Thanks,
>>>> Jiangli
>>>>
>>