[9] RFR (S): 8159431: C1 arraycopy intrinsic type checks missing

[Zoltán Majó]

Hi Vladimir,


On 06/28/2016 06:54 PM, Vladimir Kozlov wrote:
> Looks good. But you need to fix ot for aarch64 and ppc64 too. Please, 
> ask SAP and RH for help with code and testing change if needed.

thank you, Vladimir!

OK, I'll contact SAP and RH then.

Best regards,


Zoltan

>
> Thanks,
> Vladimir
>
> On 6/27/16 11:20 AM, Zoltán Majó wrote:
>> Hi Vladimir,
>>
>>
>> thank you for the feedback!
>>
>> On 06/25/2016 12:56 AM, Vladimir Kozlov wrote:
>>> Zoltan, check that klass is not j.l.Object is not enough.
>>> See next code in stub generators how to check for arrays (similar code
>>> exists on all platforms):
>>>
>>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/ff0c5aa32086/src/cpu/x86/vm/stubGenerator_x86_64.cpp#l2649 
>>>
>>>
>>
>> right, thank you for catching that!
>>
>> Here is the updated webrev:
>> http://cr.openjdk.java.net/~zmajo/8159431/webrev.01/
>>
>> Testing: JPRT (testset hotspot).
>>
>> Thank you!
>>
>> Best regards,
>>
>>
>> Zoltan
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 6/24/16 9:05 AM, Zoltán Majó wrote:
>>>> Hi,
>>>>
>>>>
>>>> please review the patch for 8159431.
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8159431
>>>>
>>>> Problem: The C1 intrinsic for
>>>> java.lang.System.arraycopy(Object src, int srcPos, Object dest, int
>>>> destPos, int length)
>>>>
>>>> does not verify that 'src' or 'dest' is a "proper" array. As a
>>>> result, the intrinsic reads the non-existing 'length' field from
>>>> java.lang.Object (i.e., the intrinsic reads past the end of the
>>>> java.lang.Object instance), which is incorrect.
>>>>
>>>> Also, the intrinsic only checks if 'src' is a subclass of 'dest'
>>>> before writing into 'dest' without checking the special case
>>>> 'dest'==java.lang.Object. That can result in data being written to a
>>>> random heap location instead of the required ArrayStoreException
>>>> being thrown.
>>>>
>>>> Solution: Check the type of 'src' and 'dest' at runtime if the
>>>> compiler was not able to prove at compile-time that 'src' and 'dest'
>>>> are "proper" arrays.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~zmajo/8159431/webrev.00/
>>>>
>>>> Testing: JPRT (testset hotspot).
>>>>
>>>> I'll do an RBT run once the code is close to its final shape.
>>>>
>>>> The problem was originally reported by Xiang Yuan from Linaro. Once
>>>> the code is reviewed, I plan to push the patch with Xiang and myself
>>>> as contributors.
>>>>
>>>> The open arm port and ppc are also likely to be affected. I'll file
>>>> bugs for those architectures and notify persons working on the
>>>> appropriate ports.
>>>>
>>>> Thank you!
>>>>
>>>> Best regards,
>>>>
>>>>
>>>> Zoltan
>>>>
>>