[aarch64-port-dev ] 8218966: AArch64: String.compareTo() can read memory after string

[Dmitry Samersoff]

Dmitrij,

The fix looks good to me.

-Dmitry



On 21.02.2019 18:26, Dmitrij Pochepko wrote:
> Hi all,
> 
> Please review a fix for "8218966: AArch64: String.compareTo() can read
> memory after string".
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8218966
> webrev: http://cr.openjdk.java.net/~dpochepk/8218966/webrev/
> 
> Intrinsic implementation returns wrong value in rare cases for strings
> longer than 72 characters.
> 
> Changes:
> 
> - Different encodings case. Small 16-characters loop and post-loop code
> are re-organized to stop at string end. Post-loop now also uses
> compare_string_16_x_LU() to avoid code duplication.
> - Changed calculation of prefetchLoopExitCondition. It might be
> incorrect in case when SoftwarePrefetchHintDistance was set to
> non-default small value.
> - Same encoding case. Moved loop counter update out of prefetch block.
> It might miss end-of-string check when prefetch is disabled, with memory
> after string being read.
> - Added 2 tests. They are quite similar but
> TestStringCompareToSameLength compares strings of same length, and
> TestStringCompareToDifferentLength is for different lengths. Tests cover
> 8218966 case. And also they cover different parts of intrinsic, taking
> into account conditions in the implementation and possible
> SoftwarePrefetchHintDistance values.
> 
> Testing:
> 
> Existing jtreg and jck tests were not able to detect 8218966 case. But
> they pass with the fix applied. Newly added jtreg tests can detect the
> issue and potential problems in case of changes in the implementation.
> The following testing was performed:
> 
> - jck with default vm flags
> - jck with -Xcomp -XX:-TieredCompilation
> - hotspot jtreg tests (including new tests): compiler/*, runtime/*, gc/*
> with default vm flags
> - hotspot jtreg tests (including new tests): compiler/*, runtime/*, gc/*
> with -Xcomp -XX:-TieredCompilation
> - jdk jtreg tier1-3 tests with default vm flags
> - jdk jtreg tier1-3 tests with -Xcomp -XX:-TieredCompilation
> 
> No regressions were found.
> 
> I'd like to thank Pengfei Li (Pengfei.Li at arm.com) for pre-review and
> additional testing.
> 
> I'm also about to send separate additional webrev with compareTo
> intrinsic documentation and maintenance-related improvements as separate
> enhancement.
> 
> Thanks,
> Dmitrij
>