On Mon, 29 Jan 2024 19:45:41 GMT, Paul Sandoz <psan...@openjdk.org> 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