[8u] Request for approval: Backport of 8159244: Partially initialized string object created by C2's string concat optimization may escape

[Rob McKenna]

Approved

	-Rob

On 17/06/16 11:47, Tobias Hartmann wrote:
> Hi,
> 
> please approve the following backport to 8u.
> 
> 8159244: Partially initialized string object created by C2's string concat optimization may escape
> https://bugs.openjdk.java.net/browse/JDK-8159244
> 
> The JDK 9 and 8u versions were reviewed:
> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-June/023333.html
> 
> The JDK 9 fix was pushed to hs-comp and nightly testing showed no problems:
> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/eadc4ebb7755
> 
> Thanks,
> Tobias
> 
> On 13.06.2016 11:26, Tobias Hartmann wrote:
> > Hi,
> > 
> > please review the following patch:
> > 
> > https://bugs.openjdk.java.net/browse/JDK-8159244
> > http://cr.openjdk.java.net/~thartmann/8159244/webrev.9.00/
> > http://cr.openjdk.java.net/~thartmann/8159244/webrev.8u.00/
> > 
> > C2's String concatenation optimization replaces a series of StringBuilder.append() calls by creating a single char buffer array (or byte array with Compact Strings) and emitting direct loads/stores to/from this array. The final StringBuilder.toString() call is replaced by a new String allocation which is initialized to the buffer array (see [1] -> [2], CallStaticJava is replaced).
> > Depending on the scheduling of instructions, it may happen that a reference to the newly allocated String object escapes before the String.value field is initialized (see [2], '334 StoreP' stores the String object, '514 StoreP' initializes the String.value field). In a highly concurrent setting, another thread may try to dereference String.value from such a partially initialized String object and crash.
> > 
> > The solution is to add a StoreStore barrier after the String object initialization to prevent subsequent stores to float above (we do the same for the Object.clone intrinsic). I verified correctness of the C2 graph (see [3]) and the generated assembly code (compare baseline [7] and fix [8]).
> > 
> > TestStringObjectInitialization.java reproduces this problem with JDK 7, 8 and 9 (see [4], [5], [6]) in approximately 1 out of 10 runs. I had to disable Indify String Concat, Compressed Oops and G1 to trigger the bug with JDK 9. Tested with JPRT and RBT.
> > 
> > Thanks,
> > Tobias
> > 
> > [1] https://bugs.openjdk.java.net/secure/attachment/60305/graph_baseline_before%20SC.png
> > [2] https://bugs.openjdk.java.net/secure/attachment/60306/graph_baseline_after_sc.png
> > [3] https://bugs.openjdk.java.net/secure/attachment/60304/graph_fix.png
> > [4] https://bugs.openjdk.java.net/secure/attachment/60298/JDK7_hs_err_pid17491.log
> > [5] https://bugs.openjdk.java.net/secure/attachment/60264/JDK8u_hs_err_pid23015.log
> > [6] https://bugs.openjdk.java.net/secure/attachment/60263/JDK9_hs_err_pid383.log
> > [7] https://bugs.openjdk.java.net/secure/attachment/60303/baseline.asm
> > [8] https://bugs.openjdk.java.net/secure/attachment/60302/fix.asm
> >