StringBuilding optimization bug since Java 7 update 4

Vladimir Kozlov vladimir.kozlov at oracle.com
Mon Jun 25 09:16:51 PDT 2012


I have a fix which I tested over weekend, I will send review request today.

Vladimir

On 6/25/12 5:33 AM, Krystal Mok wrote:
> 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 <mailto: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)
>
>


More information about the hotspot-compiler-dev mailing list