RFR: 8331098: [Aarch64] Fix crash in Arrays.equals() intrinsic with -CCP [v2]

[Roman Kennke]

On Fri, 26 Apr 2024 13:01:59 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Yes, exactly. Perhaps makes sense to rename the variable to 'base_is_8aligned' or something similar?
>
>> Yes, exactly. Perhaps makes sense to rename the variable to 'base_is_8aligned' or something similar?
> 
> With a comment. "Either the base or the length is at an 8-byte-aligned address." This boolean tells you which, and because "reasons" we're trying to maintain alignment. I implore you to measure the difference from alignment, and if the read alignment makes little or no difference, don't do this micro-optimization.

This is not about an optimization, but about a correctness issue. The loop(s) have been written under the assumption that they can read full words, which is true if we start at a word boundary. However, if we don't, then we can attempt an (unaligned) read beyond the array, and if that memory is outside of the heap and unmapped, then we would crash. Note that this currently only happens when running with -UseCompressedClassPointers which almost nobody does. We encountered it with Lilliput, which changes array layout in a similar way.

Another way to think of the situation is this: The length and the array elements are a contiguous chunk of memory, starting at the array-length. We want to compare all of it, and fail as soon as we encounter a mismatch. And since the array-length may be at an unaligned location, we compare that 'head' explicitly, and then enter the main-loop at an aligned memory location (the first array element), and can do an optimized loop that reads full words. When running with -CCP (or Lilliput), the first array element will be unaligned, though, but when that happens, we know that the array-length is aligned, and we don't have to compare the unaligned head (length) ahead of the main-loop, but can instead enter the main-loop at the aligned address of the array-length field.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18948#discussion_r1581027474