On Mon, 29 Jan 2024 19:45:41 GMT, Paul Sandoz <[email protected]> wrote:
> The implementation of method `VectorSpecies::fromMemorySegment`, in
> `AbstractSpecies::fromMemorySegment`, neglects to perform bounds checks on
> the offset argument when the method is compiled by C2 (bounds checks are
> performed when interpreted and by C1).
>
> This is an oversight and explicit bounds checks are required, as is already
> case for the other load and store memory access methods (including storing to
> memory memory segments).
>
> The workaround is to call the static method `{T}Vector::fromMemorySegment`.
>
> The fix is for the implementation(s) of `VectorSpecies::fromMemorySegment` to
> do the same and call `{T}Vector::fromMemorySegment`, following the same
> pattern for implementations of `VectorSpecies::fromArray`.
>
> The tests have been conservatively updated to call the species access method
> where possible in the knowledge that it calls the vector access method (the
> tests were intended to test out of bounds access when compiled by C2).
>
> Thinking ahead its tempting to remove the species access methods, simplifying
> functionality that is duplicated.
test/jdk/jdk/incubator/vector/templates/X-LoadStoreTest.java.template line 271:
> 269: @DontInline
> 270: static $abstractvectortype$ fromArray($type$[] a, int i) {
> 271: return ($abstractvectortype$) SPECIES.fromArray(a, i);
These new tests focus on the species method - but should we also test the
statics factories in the record class, just in case a regression is introduced
and the two implementation start diverging again?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17621#discussion_r1472143425
