[aarch64-port-dev ] RFR: 8215202: AArch64: jtreg test test/jdk/sun/nio/cs/FindEncoderBugs.java fails

[Dmitrij Pochepko]

Hi,

Good catch. Looks good to me (although I'm not a reviewer). Though it 
only affects exotic case, when we simultaneously:

- know, that input char array contains unmappable symbol and we rely on that
- output array contains some data starting from unmappable symbol 
potential position, but within original "string" length
- coding error action for unmappable symbol is not set to ignore or replace

Since this behavior is reproducible using public API, I think it makes 
sense to backport to 11u together with your previous patch. I'll be 
happy to help.

Dmitrij

On 11/12/2018 11:05 AM, Nick Gasson (Arm Technology China) wrote:
> Hi,
>
> Please help me review this patch to fix an issue with the ISO-8859-1
> encoder intrinsic on AArch64 (nio-dev CC-ed due to test change).
>
> Webrev: http://cr.openjdk.java.net/~njian/8215202/webrev.0/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215202
>
> The jtreg test test/jdk/sun/nio/cs/FindEncoderBugs.java fails with the
> following error on AArch64 with compilation:
>
>    Buffer overrun: ISO-8859-1 "\u8a8e"[0/1] => UNMAPPABLE[1] ""[0/19] -114
>    failures=1
>
>    Passed = 223410156, failed = 10000
>
>    STDERR:
>    java.lang.AssertionError: Some tests failed
>            at FindEncoderBugs.main(FindEncoderBugs.java:526)
>
> The problem is caused by the encoder function for the ISO-8859-1
> character set writing one extra character to the output array when it
> encounters a character it can't encode. This is implemented as an
> intrinsic on AArch64, but the intrinsic doesn't exactly match the
> original Java code in ISO_8859_1.java:
>
>    @HotSpotIntrinsicCandidate
>    private static int implEncodeISOArray(char[] sa, int sp,
>                                          byte[] da, int dp, int len) {
>        int i = 0;
>        for (; i < len; i++) {
>            char c = sa[sp++];
>            if (c > '\u00FF') // (1)
>                break;
>            da[dp++] = (byte)c; // (2)
>        }
>        return i;
>    }
>
> In the intrinsic code statements (1) and (2) are swapped.
>
> Also Extended the FindEncoderBugs test so that it hits all cases in
> the intrinsic assembly. Currently only the final NEXT_1 section is
> covered by this test.
>
> Thanks,
> Nick