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

[Jiangli Zhou]

Thanks, David!

Jiangli

> On Oct 9, 2016, at 2:10 PM, David Holmes <david.holmes at oracle.com> 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.
> 
> David
> 
>> Thanks,
>> Jiangli
>> 
>>> 
>>> Thanks,
>>> David
>>> 
>>>> Tested with all existing class data sharing tests.
>>>> 
>>>> Thanks,
>>>> Jiangli
>>>> 
>>