Request for reviews (XS): 7047069: Array can dynamically change size when assigned to an object field

Tom Rodriguez tom.rodriguez at oracle.com
Fri May 27 13:15:04 PDT 2011


That looks good.

tom

On May 27, 2011, at 12:47 PM, Vladimir Kozlov wrote:

> Yes, removed  BIG_NEG and used -1 in find_int_con()
> 
> Thanks,
> Vladimir
> 
> Tom Rodriguez wrote:
>> You don't really need BIG_NEG at all then.
>> tom
>> On May 27, 2011, at 12:34 PM, Vladimir Kozlov wrote:
>>> I agree, I changed the code to check result of find_int_con(offset, BIG_NEG). Webrev is updated.
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> Tom Rodriguez wrote:
>>>> On May 27, 2011, at 11:51 AM, Vladimir Kozlov wrote:
>>>>> http://cr.openjdk.java.net/~kvn/7047069/webrev
>>>>> 
>>>>> Fixed 7047069: Array can dynamically change size when assigned to an object field
>>>>> 
>>>>> I lost my faith in our testing :( This broken code was there for more then 3 years and nobody hit it?
>>>>> The initialization of a newly-allocated array with arraycopy is broken when src and dest offsets are not constants. The typo in the code convert not constant offsets to constant 8: 12 + (-1)*4. So we generates copy from offset 8 which is array length and overwrite it and the rest of the beginning of the array.
>>>> I don't really like the find_int_con idiom all that much and the usage here is too clever.  It's much less clear than:
>>>> if (src_offset->Opcode() == Op_ConI || dest_offset->Opcode() == Op_ConI)
>>>> return;
>>>> intptr_t src_off  = abase + ((intptr_t) src_offset->find_int_con()  << scale);
>>>> intptr_t dest_off = abase + ((intptr_t) dest_offset->find_int_con() << scale);
>>>> Anyway, your fix is good.
>>>> tom
>>>>> Added regression test.



More information about the hotspot-compiler-dev mailing list