[9] RFR(S): 8142303: C2 compilation fails with "bad AD file"
Vladimir Kozlov
vladimir.kozlov at oracle.com
Fri Nov 13 15:40:31 UTC 2015
New code in library_call.cpp is somewhat similar. Can you factor out it into separate function?
If generate_limit_guard() is unsigned check then you don't need to check for negative values. Right?
Thanks,
Vladimir
On 11/13/15 7:23 AM, Tobias Hartmann wrote:
> Hi,
>
> please review the following patch.
>
> https://bugs.openjdk.java.net/browse/JDK-8142303
> http://cr.openjdk.java.net/~thartmann/8142303/jdk/webrev.00/
> http://cr.openjdk.java.net/~thartmann/8142303/hotspot/webrev.00/
>
> Problem:
> CTW crashes in C2's matching phase because the source array address of a StrCompressedCopyNode is TOP. The Java code creates a new String via String(char value[], int offset, int count) with an invalid constant offset (-1). The String constructor performs a range check and hands over the char[] to the StringUTF16.compress intrinsic for compression from UTF16 to Latin1. The problem is that the explicit range check in the Java code is not inlined into the compilation of the constructor but the intrinsic is inlined. Because C2 does not know about the range check and the intrinsics does not have any checks, C2 does not cut off the dead code (i.e. the invocation of StringUTF16.compress()). As a result, the negative constant index is fed into the intrinsic, propagates to the array address computation and is then replaced by TOP because the index is out of bounds of the ConvI2LNode.
>
> Solution:
> One solution would be to simply check for a TOP input in StrCompressedCopyNode::ideal() and cut off the dead code. However, I would like to go for a more robust solution and move the range checks into the intrinsic. This is also a problem with other String intrinsics. I added checks to the following intrinsics:
> - String.indexOfI
> - String.indexOfChar
> - StringCoderUTF16.toBytes
> - StringCoderUTF16.getChars
> - StringUTF16.compress
> - StringLatin1.inflate
>
> The only remaining String intrinsic is StringUTF16.getChar/putChar which should be safe.
>
> I verified that this has no impact on performance because C2 is able to fold the range checks in the common cases (see [1]).
>
> Tested with JPRT and jck tests (jck/api/java_lang/String,jck/api/java_lang/StringBuilder,jck/api/java_lang/StringBuffer).
>
> Thanks,
> Tobias
>
>
> [1] Results of Aleksey's String microbenchmarks [2]. String size is set to 1 to measure the impact of range checks:
>
> ## BASELINE ##
> Benchmark (cmp) (seed) (size) Mode Cnt Score Error Units
> FromCharArray.test 1 12345678900 1 avgt 75 17.142 ± 0.481 ns/op
> FromCharArray.test 0 12345678900 1 avgt 75 30.012 ± 0.295 ns/op
> IndexOfChar.test 1 12345678900 1 avgt 75 14.457 ± 0.422 ns/op
> IndexOfChar.test 0 12345678900 1 avgt 75 14.546 ± 0.462 ns/op
> IndexOfString.test 1 12345678900 1 avgt 75 15.292 ± 0.163 ns/op
> IndexOfString.test 0 12345678900 1 avgt 75 14.892 ± 0.373 ns/op
> ToCharArray.test 1 12345678900 1 avgt 75 15.131 ± 0.455 ns/op
> ToCharArray.test 0 12345678900 1 avgt 25 16.352 ± 1.418 ns/op
> concat.ConcatStringsBench.test_cmp1 N/A N/A 1 avgt 75 10.610 ± 0.267 ns/op
> concat.ConcatStringsBench.test_cmp1_cmp1 N/A N/A 1 avgt 75 36.860 ± 0.873 ns/op
> concat.ConcatStringsBench.test_cmp1_cmp2 N/A N/A 1 avgt 75 30.760 ± 0.683 ns/op
> concat.ConcatStringsBench.test_cmp2 N/A N/A 1 avgt 75 11.170 ± 0.288 ns/op
> concat.ConcatStringsBench.test_cmp2_cmp1 N/A N/A 1 avgt 75 29.256 ± 0.856 ns/op
> concat.ConcatStringsBench.test_cmp2_cmp2 N/A N/A 1 avgt 75 35.245 ± 0.809 ns/op
> construct.ConstructBench.cmp1 N/A N/A 1 avgt 75 17.146 ± 0.408 ns/op
> construct.ConstructBench.cmp2_beg N/A N/A 1 avgt 75 28.973 ± 1.023 ns/op
> construct.ConstructBench.cmp2_end N/A N/A 1 avgt 25 29.785 ± 0.130 ns/op
>
> ## FIX ##
> Benchmark (cmp) (seed) (size) Mode Cnt Score Error Units
> FromCharArray.test 1 12345678900 1 avgt 75 17.042 ± 0.425 ns/op
> FromCharArray.test 0 12345678900 1 avgt 75 29.165 ± 0.673 ns/op
> IndexOfChar.test 1 12345678900 1 avgt 75 14.925 ± 0.369 ns/op
> IndexOfChar.test 0 12345678900 1 avgt 75 15.374 ± 0.094 ns/op
> IndexOfString.test 1 12345678900 1 avgt 75 14.433 ± 0.422 ns/op
> IndexOfString.test 0 12345678900 1 avgt 75 14.880 ± 0.372 ns/op
> ToCharArray.test 1 12345678900 1 avgt 75 15.667 ± 0.404 ns/op
> ToCharArray.test 0 12345678900 1 avgt 75 16.651 ± 0.493 ns/op
> concat.ConcatStringsBench.test_cmp1 N/A N/A 1 avgt 75 11.664 ± 0.267 ns/op
> concat.ConcatStringsBench.test_cmp1_cmp1 N/A N/A 1 avgt 75 37.934 ± 0.104 ns/op
> concat.ConcatStringsBench.test_cmp1_cmp2 N/A N/A 1 avgt 75 30.743 ± 0.707 ns/op
> concat.ConcatStringsBench.test_cmp2 N/A N/A 1 avgt 75 11.217 ± 0.271 ns/op
> concat.ConcatStringsBench.test_cmp2_cmp1 N/A N/A 1 avgt 75 29.294 ± 0.814 ns/op
> concat.ConcatStringsBench.test_cmp2_cmp2 N/A N/A 1 avgt 75 35.292 ± 0.815 ns/op
> construct.ConstructBench.cmp1 N/A N/A 1 avgt 75 18.620 ± 0.390 ns/op
> construct.ConstructBench.cmp2_beg N/A N/A 1 avgt 75 28.881 ± 0.703 ns/op
> construct.ConstructBench.cmp2_end N/A N/A 1 avgt 75 29.621 ± 0.202 ns/op
>
> [2] http://cr.openjdk.java.net/~shade/density/string-density-bench.jar
>
More information about the hotspot-compiler-dev
mailing list