RFR: 8331098: [Aarch64] Fix crash in Arrays.equals() intrinsic with -CCP [v6]
Axel Boldt-Christmas
aboldtch at openjdk.org
Wed May 8 13:21:54 UTC 2024
On Wed, 8 May 2024 08:18:30 GMT, Roman Kennke <rkennke at openjdk.org> wrote:
>> Also right now the `length_is_8aligned` very much now just look like a `!oopDesc::has_klass_gap()` check.
>>
>> The validity of choosing to start from the klass when `!length_is_8aligned` is that we must then be using `UseCompressedClassPointers /* && !UseCompactObjectHeaders for Lilliput */`. Until Lilliput starting from the Klass is valid for both `+UseCompressedClassPointers` and `-UseCompressedClassPointers` as the klass, length and base will always be tightly packed (no padding) for byte and char type arrays.
>>
>> In all modes what we effectively do is `int start_offset = align_down(length_offset, BytesPerWord )`. Not sure if the intent gets clearer then. Something along the lines of:
>> Suggestion:
>>
>> // When the length offset is not aligned to 8 bytes,
>> // then we align it down, this is valid as the new
>> // offset will always be the klass which is the same
>> // for type arrays.
>> int start_offset = align_down(length_offset, BytesPerWord);
>> int extra_length = base_offset - start_offset;
>> assert(start_offset == length_offset || start_offset == klass_offset,
>> "start offset must be 8-byte-aligned or be the klass offset");
>> assert(base_offset != start_offset, "must include the length field");
>
> Right, not assuming layout of Klass* seems saner.
> BTW, including the Klass* is not only safe because it's only char[] or byte[], it's also already guaranteed to be the same by the calling APIs. And even if it were not (arraycopy-style) - if the Klass* could be different, then array-equality would have to check this, also, anyway.
Maybe we are talking about different things, I am also unsure which method is actually intrinsified here. If it actually is `java.util.Arrays.equals(...)` then it does have the property that it does not really care about the klass. (But the method overloading ensures that you only compare the same type array object with another of the object of the same type).
class Main {
public static void main(String args[]) {
final int size = 10;
Integer iArray[] = new Integer[size];
Object oArray[] = new Object[size];
System.out.println(iArray.equals(oArray)); // Prints "false"
System.out.println(oArray.equals(iArray)); // Prints "false"
System.out.println(java.util.Arrays.equals(iArray, oArray)); // Prints "true"
System.out.println(iArray.getClass() == oArray.getClass()); // Prints "false"
}
};
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18948#discussion_r1594020876
More information about the hotspot-dev
mailing list