From 97bf1484bce0e15fe8c400bd338a68c9424508f8 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Wed, 5 Jun 2024 04:22:47 -0400 Subject: [PATCH] more precise aliasing checks for SubArray (#54624) This avoids returning false positives where only the indices are shared. As the indices are not mutated by array assignments (and are explicitly warned against mutation in general), we can ignore the case where _only_ the indices are aliasing. Fix #54621 --- base/multidimensional.jl | 41 ++++++++++++++++++++++++---------------- test/subarray.jl | 31 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/base/multidimensional.jl b/base/multidimensional.jl index f25f35069c830..3bd106455d274 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -1047,25 +1047,34 @@ end ### from abstractarray.jl -# In the common case where we have two views into the same parent, aliasing checks -# are _much_ easier and more important to get right -function mightalias(A::SubArray{T,<:Any,P}, B::SubArray{T,<:Any,P}) where {T,P} - if !_parentsmatch(A.parent, B.parent) - # We cannot do any better than the usual dataids check - return !_isdisjoint(dataids(A), dataids(B)) - end - # Now we know that A.parent === B.parent. This means that the indices of A - # and B are the same length and indexing into the same dimensions. We can - # just walk through them and check for overlaps: O(ndims(A)). We must finally - # ensure that the indices don't alias with either parent - return _indicesmightoverlap(A.indices, B.indices) || - !_isdisjoint(dataids(A.parent), _splatmap(dataids, B.indices)) || - !_isdisjoint(dataids(B.parent), _splatmap(dataids, A.indices)) +function mightalias(A::SubArray, B::SubArray) + # There are three ways that SubArrays might _problematically_ alias one another: + # 1. The parents are the same we can conservatively check if the indices might overlap OR + # 2. The parents alias eachother in a more complicated manner (and we can't trace indices) OR + # 3. One's parent is used in the other's indices + # Note that it's ok for just the indices to alias each other as those should not be mutated, + # so we can always do better than the default !_isdisjoint(dataids(A), dataids(B)) + if isbits(A.parent) || isbits(B.parent) + return false # Quick out for immutables + elseif _parentsmatch(A.parent, B.parent) + # Each SubArray unaliases its own parent from its own indices upon construction, so if + # the two parents are the same, then by construction one cannot alias the other's indices + # and therefore this is the only test we need to perform: + return _indicesmightoverlap(A.indices, B.indices) + else + A_parent_ids = dataids(A.parent) + B_parent_ids = dataids(B.parent) + return !_isdisjoint(A_parent_ids, B_parent_ids) || + !_isdisjoint(A_parent_ids, _splatmap(dataids, B.indices)) || + !_isdisjoint(B_parent_ids, _splatmap(dataids, A.indices)) + end end +# Test if two arrays are backed by exactly the same memory in exactly the same order _parentsmatch(A::AbstractArray, B::AbstractArray) = A === B -# Two reshape(::Array)s of the same size aren't `===` because they have different headers -_parentsmatch(A::Array, B::Array) = pointer(A) == pointer(B) && size(A) == size(B) +_parentsmatch(A::DenseArray, B::DenseArray) = elsize(A) == elsize(B) && pointer(A) == pointer(B) && size(A) == size(B) +_parentsmatch(A::StridedArray, B::StridedArray) = elsize(A) == elsize(B) && pointer(A) == pointer(B) && strides(A) == strides(B) +# Given two SubArrays with the same parent, check if the indices might overlap (returning true if unsure) _indicesmightoverlap(A::Tuple{}, B::Tuple{}) = true _indicesmightoverlap(A::Tuple{}, B::Tuple) = error("malformed subarray") _indicesmightoverlap(A::Tuple, B::Tuple{}) = error("malformed subarray") diff --git a/test/subarray.jl b/test/subarray.jl index 3ef7b6f66a9ec..818365730f079 100644 --- a/test/subarray.jl +++ b/test/subarray.jl @@ -1077,6 +1077,37 @@ end end end +@testset "aliasing checks with shared indices" begin + indices = [1,3] + a = rand(3) + av = @view a[indices] + b = rand(3) + bv = @view b[indices] + @test !Base.mightalias(av, bv) + @test Base.mightalias(a, av) + @test Base.mightalias(b, bv) + @test Base.mightalias(indices, av) + @test Base.mightalias(indices, bv) + @test Base.mightalias(view(indices, :), av) + @test Base.mightalias(view(indices, :), bv) +end + +@testset "aliasing checks with disjoint arrays" begin + A = rand(3,4,5) + @test Base.mightalias(view(A, :, :, 1), view(A, :, :, 1)) + @test !Base.mightalias(view(A, :, :, 1), view(A, :, :, 2)) + + B = reinterpret(UInt64, A) + @test Base.mightalias(view(B, :, :, 1), view(A, :, :, 1)) + @test !Base.mightalias(view(B, :, :, 1), view(A, :, :, 2)) + + C = reinterpret(UInt32, A) + @test Base.mightalias(view(C, :, :, 1), view(A, :, :, 1)) + @test Base.mightalias(view(C, :, :, 1), view(A, :, :, 2)) # This is overly conservative + @test Base.mightalias(@view(C[begin:2:end, :, 1]), view(A, :, :, 1)) + @test Base.mightalias(@view(C[begin:2:end, :, 1]), view(A, :, :, 2)) # This is overly conservative +end + @testset "aliasing check with reshaped subarrays" begin C = rand(2,1) V1 = @view C[1, :]