[10] RFR(S): 8182036: Load from initializing arraycopy uses wrong memory state

[Vladimir Kozlov]

On 6/16/17 4:45 AM, Roland Westrelin wrote:
> 
> Thanks for reviewing this, Vladimir,
> 
>> What if you have a second arraycopy after 0x42 store which also modify
>> src[1]?
> 
> Can you post the code snippet you think could cause a problem? I'm not
> sure I'm following.
> I should have made it clearer: this covers a corner case where the
> arraycopy is responsible for the initialization of a just allocated
> array. The arraycopy is right after the allocation and nothing is
> allowed between the arraycopy and the allocation.

Got it.

Thinking in general the optimization to align copy address should be 
done in arraycopy code stubs. It increases complexity of generated code 
and adds problems to C2. And stubs know what alignment they need.

Anyway the fix seems reasonable for current code.

About last change which removed (ac->_src_type != ac->_dest_type) code. 
Do we ever hit it? Before that we exit this code if src_elem != dest_elem.

> 
>> You did not explain changes in memnode.cpp
> 
> During arraycopy node expansion, a ClearArrayNode is created to
> initialize the head section followed by the LoadI/StoreI pair for the
> first element to copy. The ClearArrayNode is transformed into a StoreL
> so we have a StoreI followed by a StoreL. The LoadI used to have the
> StoreL as memory input but I moved it to another slice which is why the
> assert now fails: the StoreL used to have 2 uses so we wouldn't go into
> the loop where the assert is.

Okay, so it is the case that arraycopy follows ClearArrayNode.
The comment should be "// initializing by arraycopy"

Vladimir

> 
>> I would like to fix it first in JDK 10 and let it be tested before we
>> backport into previous releases.
> 
> Sure. That sounds fine to me.
> 
> Roland.
>