StringBuilding optimization bug since Java 7 update 4

[Krystal Mok]

Hi all,

Is anybody fixing this bug already?

I looked at this issue earlier today. It looks like the problem had
something to do with coalescing stacked string concats. When I comment out
the "try to coalesce separate concats" block from PhaseStringOpts's
constructor, the bug doesn't reproduce any more.

There was a lot of noise in the original test case, that it was hard to
analyze what went wrong on the IR level.
Here's a smaller repro case derived from Riven's:
https://gist.github.com/2987732
It still reproduces on JDK7u4/HS23 and JDK8b43/HS24, both with OSR and
standard compilation.

Strangely, it doesn't reproduce when OSR is disabled (with either
-UseOnStackReplacement or -CICompileOSR):

$ java -XX:-UseOnStackReplacement Main
CompilerOracle: exclude Main.logError
Java Version: 24.0-b14-internal-jvmg
try OSR compilation
   1071    1    b        java.lang.String::length (6 bytes)
   1135    2     n       java.lang.System::arraycopy (0 bytes)   (static)
   1148    3    b        java.lang.Object::<init> (1 bytes)
   1178    4    b
 java.lang.AbstractStringBuilder::ensureCapacityInternal (16 bytes)
   1229    5    b        java.lang.String::getChars (62 bytes)
   1397    6    b        java.lang.AbstractStringBuilder::append (48 bytes)
   2013    7    b        java.lang.StringBuilder::append (8 bytes)
   2682    8    b        java.lang.Math::min (11 bytes)
   2702    9    b        java.lang.String::<init> (67 bytes)
   3140   10    b        java.util.Arrays::copyOfRange (63 bytes)
   3331   11    b        java.lang.AbstractStringBuilder::<init> (12 bytes)
   3390   12    b        java.lang.StringBuilder::toString (17 bytes)
   3993   13    b        java.lang.String::valueOf (14 bytes)
   4028   14    b        java.lang.StringBuilder::<init> (18 bytes)
try standard compilation
   5563   15    b        Main::foo (98 bytes)
$

...

Regards,
Kris

On Sat, Jun 23, 2012 at 1:06 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com
> wrote:

> Thank you for finding this issue, I filed bug report:
>
> 7179138: Incorrect result with String concatenation optimization
>
> Few questions:
>
> Can we add your tests to Hotspot VM regression tests?
>
> http://hg.openjdk.java.net/**hsx/hotspot-main/hotspot/file/**
> 1c280e5b8d31/test/compiler/<http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/file/1c280e5b8d31/test/compiler/>
>
> What should we put into the test's Copyright line?
> And what reference you want in test's @author line?
>
> Thanks,
> Vladimir
>
>
> Skip Balk wrote:
>
>> Dear HotSpot developers,
>>  This is the first time I post anything on any mailinglist, so please
>> forgive me if my message is not quite up to the standards you are used to.
>>  Last week, I encountered a bug in simple String concatenation, like:
>>  String s = "test";
>>    s = s+s;
>>    s = s+s;
>>    s = s+s;
>>  yielding the string: "nullnulltesttest" instead of "**
>> testtesttesttesttesttesttestte**st". The first Java version that seems
>> to suffer from this bug is Java 7u4, and is confirmed to occur in 7u5,
>> 7u6(rc). It has been further reproduced on 32bit, 64bit, clientvm and
>> servervm.
>>  After a few thousand (interpreted) runs of this code, it starts to give
>> these incorrect results, leading me to assume that a HotSpot optimisation
>> is the root cause of this problem, which is backed by the fact that when
>> running the java process with the -Xint argument, the bug does not occur.
>>  Unfortunately, today I discovered that with my trivial sourcecode, the
>> issue only occured with the Eclipse compiler. The Javac output seemed to be
>> 'friendly' to HotSpot.
>>  Upon further investigation, it turned out that "s=s+s" was compiled to
>> different bytecode by Javac and Eclipse:
>>    Eclipse: s = new StringBuilder(String.valueOf(**
>> s)).append(s).toString();
>>    Javac:   s = new StringBuilder().append(s).**append(s).toString();
>> When writing the version Eclipse produces in Java sourcecode, the javac
>> compiler also produced the bytecode that made HotSpot trip.
>>  Without further ado: here are the full code-dumps (both for eclipse and
>> javac)
>>    http://riven8192.blogspot.com/**2012/06/hotspot-bug-in-java-**
>> 7u4-7u5-7u6rc-with.html<http://riven8192.blogspot.com/2012/06/hotspot-bug-in-java-7u4-7u5-7u6rc-with.html>
>>  "javap -c" output, with sourcecode containing "s=s+s"
>>    Javac:   http://pastebin.com/raw.php?i=**pC3kRC6c<http://pastebin.com/raw.php?i=pC3kRC6c>
>>    Eclipse: http://pastebin.com/raw.php?i=**Pbj0fyZ8<http://pastebin.com/raw.php?i=Pbj0fyZ8>
>>  Console output:
>>    Java Version: 23.0-b21
>>    Failed at iteration: 11983
>>    Length mismatch: 16 <> 32
>>    Expected: "**testtesttesttesttesttesttestte**st"
>>    Actual:   "nullnulltesttest"
>>  Last but not least, this bug seems to be triggered by the
>> empty-for-loop, which leads me to believe this is a case of too aggresive
>> dead code removal. I hope you can
>>  With kind regards,
>>    Riven (http://www.java-gaming.org/ administrator)
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120625/5d6e0e0c/attachment.html