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

[David Holmes]

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