From 74673e33ba767eeb9b9091d06ab89bf78ec5bc60 Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Fri, 23 May 2025 14:47:51 +0200 Subject: [PATCH 1/4] Fix slicing of `PtrArray`s --- src/ptr_array.jl | 7 ++++--- src/views.jl | 9 +++++++-- test/runtests.jl | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/ptr_array.jl b/src/ptr_array.jl index eca6308..98c41ea 100644 --- a/src/ptr_array.jl +++ b/src/ptr_array.jl @@ -1008,9 +1008,10 @@ end ) v end -@propagate_inbounds function Base.getindex(A::PtrArray, i::Vararg{Integer}) - @boundscheck checkbounds(A, i...) - unsafe_getindex(A, i...) +# Copied from Base +@inline function Base.getindex(A::PtrArray, i1::Int, i2::Int, I::Int...) + @boundscheck checkbounds(A, i1, i2, I...) # generally _to_linear_index requires bounds checking + return @inbounds A[Base._to_linear_index(A, i1, i2, I...)] end @propagate_inbounds function Base.setindex!( A::PtrArray, diff --git a/src/views.jl b/src/views.jl index c0667b2..dd4e6a1 100644 --- a/src/views.jl +++ b/src/views.jl @@ -2,18 +2,23 @@ A::AbstractPtrArray{T,N}, i::Vararg{Union{Integer,AbstractRange,Colon},N} ) where {T,N} - PtrArray(SubArray(A, Base.to_indices(A, i))) + j = Base.to_indices(A, i) + @boundscheck checkbounds(A, j...) + PtrArray(SubArray(A, j)) end @inline function Base.view( A::AbstractPtrArray{T,N}, i::AbstractUnitRange ) where {T,N} - view(vec(A), i) + V = vec(A) + @boundscheck checkbounds(V, i) + view(V, i) end @inline function Base.view( A::AbstractPtrArray{T,1}, i::AbstractUnitRange ) where {T} + @boundscheck checkbounds(A, i) sx = only(static_strides(A)) if sx === static(1) p = pointer(A) + (first(i) - first(offsets(A))) * sizeof(T) diff --git a/test/runtests.jl b/test/runtests.jl index e3cc784..a0ad70f 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -449,4 +449,38 @@ end @test A == B end end + + @testset "PtrArray slicing" begin + # Testing for the bug in https://github.com/JuliaSIMD/StrideArrays.jl/issues/88 + x = [1; 2;; 3; 4] + GC.@preserve x begin + ptrarrays = ( + StrideArraysCore.PtrArray(pointer(x), (2, 2)), + StrideArraysCore.PtrArray(pointer(x), (StaticInt(2), StaticInt(2))), + ) + for y in ptrarrays + @test y[1, 1] == x[1, 1] + @test y[2, 1] == x[2, 1] + @test y[1, 2] == x[1, 2] + @test y[2, 2] == x[2, 2] + y1 = y[1, :] + @test y1[1] == x[1, 1] + @test y1[2] == x[1, 2] + y2 = y[2, :] + @test y2[1] == x[2, 1] + @test y2[2] == x[2, 2] + y3 = y[:, 1] + @test y3[1] == x[1, 1] + @test y3[2] == x[2, 1] + @test_throws BoundsError y[1:5] + @test_throws BoundsError y[3, :] + @test_throws BoundsError y[:, 3] + @test_throws BoundsError y[2:3, :] + @test_throws BoundsError y1[3] + @test_throws BoundsError y1[1:3] + @test_throws BoundsError y2[3] + @test_throws BoundsError y2[1:3] + end + end + end end From 1b6043c73dcaa75d6d0f37fd5d8a2894190f43cb Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Mon, 30 Jun 2025 23:35:56 +0200 Subject: [PATCH 2/4] Try a different approach --- src/ptr_array.jl | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ptr_array.jl b/src/ptr_array.jl index 98c41ea..ecb24c1 100644 --- a/src/ptr_array.jl +++ b/src/ptr_array.jl @@ -994,24 +994,25 @@ end pstore!(pointer(A) + (i - oneunit(i)) * static_sizeof(T), v) v end -@inline function unsafe_getindex(A::PtrVector{T}, i::Integer) where {T} +@inline function unsafe_getindex(A::PtrVector{T}, i::Vararg{Integer}) where {T} + i_ = first(i) pload( pointer(A) + - (i - ArrayInterface.offset1(A)) * only(LayoutPointers.bytestrides(A)) + (i_ - ArrayInterface.offset1(A)) * only(LayoutPointers.bytestrides(A)) ) end -@inline function unsafe_setindex!(A::PtrVector{T}, v, i::Integer) where {T} +@inline function unsafe_setindex!(A::PtrVector{T}, v, i::Vararg{Integer}) where {T} + i_ = first(i) pstore!( pointer(A) + - (i - ArrayInterface.offset1(A)) * only(LayoutPointers.bytestrides(A)), + (i_ - ArrayInterface.offset1(A)) * only(LayoutPointers.bytestrides(A)), v ) v end -# Copied from Base -@inline function Base.getindex(A::PtrArray, i1::Int, i2::Int, I::Int...) - @boundscheck checkbounds(A, i1, i2, I...) # generally _to_linear_index requires bounds checking - return @inbounds A[Base._to_linear_index(A, i1, i2, I...)] +@propagate_inbounds function Base.getindex(A::PtrArray, i::Vararg{Integer}) + @boundscheck checkbounds(A, i...) + unsafe_getindex(A, i...) end @propagate_inbounds function Base.setindex!( A::PtrArray, From 785a71504790c6b8e1c6f2c78bd039c3815b1081 Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Mon, 30 Jun 2025 23:38:40 +0200 Subject: [PATCH 3/4] Update tests --- test/runtests.jl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index a0ad70f..de6fa5a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -459,11 +459,15 @@ end StrideArraysCore.PtrArray(pointer(x), (StaticInt(2), StaticInt(2))), ) for y in ptrarrays + # This triggered the bug, a 2D index into a Vector, which is used by display/show + y1 = y[1, :] + @test y1[2, 1] == x[1, 2] + + # Some general tests with slicing and bounds checking @test y[1, 1] == x[1, 1] @test y[2, 1] == x[2, 1] @test y[1, 2] == x[1, 2] @test y[2, 2] == x[2, 2] - y1 = y[1, :] @test y1[1] == x[1, 1] @test y1[2] == x[1, 2] y2 = y[2, :] From 913ebbfd4508d0f259c6bb929d056220702699d2 Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Mon, 14 Jul 2025 11:47:50 +0200 Subject: [PATCH 4/4] Use matrix definition syntax compatible with Julia 1.6 --- test/runtests.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index de6fa5a..5585053 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -452,7 +452,7 @@ end @testset "PtrArray slicing" begin # Testing for the bug in https://github.com/JuliaSIMD/StrideArrays.jl/issues/88 - x = [1; 2;; 3; 4] + x = [1 3; 2 4] GC.@preserve x begin ptrarrays = ( StrideArraysCore.PtrArray(pointer(x), (2, 2)),