Skip to content

Commit 45b38b3

Browse files
mbaumanKristofferC
authored andcommitted
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 (cherry picked from commit 97bf148)
1 parent 90acf2a commit 45b38b3

File tree

2 files changed

+56
-16
lines changed

2 files changed

+56
-16
lines changed

base/multidimensional.jl

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,25 +1044,34 @@ end
10441044

10451045
### from abstractarray.jl
10461046

1047-
# In the common case where we have two views into the same parent, aliasing checks
1048-
# are _much_ easier and more important to get right
1049-
function mightalias(A::SubArray{T,<:Any,P}, B::SubArray{T,<:Any,P}) where {T,P}
1050-
if !_parentsmatch(A.parent, B.parent)
1051-
# We cannot do any better than the usual dataids check
1052-
return !_isdisjoint(dataids(A), dataids(B))
1053-
end
1054-
# Now we know that A.parent === B.parent. This means that the indices of A
1055-
# and B are the same length and indexing into the same dimensions. We can
1056-
# just walk through them and check for overlaps: O(ndims(A)). We must finally
1057-
# ensure that the indices don't alias with either parent
1058-
return _indicesmightoverlap(A.indices, B.indices) ||
1059-
!_isdisjoint(dataids(A.parent), _splatmap(dataids, B.indices)) ||
1060-
!_isdisjoint(dataids(B.parent), _splatmap(dataids, A.indices))
1047+
function mightalias(A::SubArray, B::SubArray)
1048+
# There are three ways that SubArrays might _problematically_ alias one another:
1049+
# 1. The parents are the same we can conservatively check if the indices might overlap OR
1050+
# 2. The parents alias eachother in a more complicated manner (and we can't trace indices) OR
1051+
# 3. One's parent is used in the other's indices
1052+
# Note that it's ok for just the indices to alias each other as those should not be mutated,
1053+
# so we can always do better than the default !_isdisjoint(dataids(A), dataids(B))
1054+
if isbits(A.parent) || isbits(B.parent)
1055+
return false # Quick out for immutables
1056+
elseif _parentsmatch(A.parent, B.parent)
1057+
# Each SubArray unaliases its own parent from its own indices upon construction, so if
1058+
# the two parents are the same, then by construction one cannot alias the other's indices
1059+
# and therefore this is the only test we need to perform:
1060+
return _indicesmightoverlap(A.indices, B.indices)
1061+
else
1062+
A_parent_ids = dataids(A.parent)
1063+
B_parent_ids = dataids(B.parent)
1064+
return !_isdisjoint(A_parent_ids, B_parent_ids) ||
1065+
!_isdisjoint(A_parent_ids, _splatmap(dataids, B.indices)) ||
1066+
!_isdisjoint(B_parent_ids, _splatmap(dataids, A.indices))
1067+
end
10611068
end
1069+
# Test if two arrays are backed by exactly the same memory in exactly the same order
10621070
_parentsmatch(A::AbstractArray, B::AbstractArray) = A === B
1063-
# Two reshape(::Array)s of the same size aren't `===` because they have different headers
1064-
_parentsmatch(A::Array, B::Array) = pointer(A) == pointer(B) && size(A) == size(B)
1071+
_parentsmatch(A::DenseArray, B::DenseArray) = elsize(A) == elsize(B) && pointer(A) == pointer(B) && size(A) == size(B)
1072+
_parentsmatch(A::StridedArray, B::StridedArray) = elsize(A) == elsize(B) && pointer(A) == pointer(B) && strides(A) == strides(B)
10651073

1074+
# Given two SubArrays with the same parent, check if the indices might overlap (returning true if unsure)
10661075
_indicesmightoverlap(A::Tuple{}, B::Tuple{}) = true
10671076
_indicesmightoverlap(A::Tuple{}, B::Tuple) = error("malformed subarray")
10681077
_indicesmightoverlap(A::Tuple, B::Tuple{}) = error("malformed subarray")

test/subarray.jl

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,3 +1075,34 @@ end
10751075
@test !isassigned(v, 3, 3) # out-of-bounds
10761076
end
10771077
end
1078+
1079+
@testset "aliasing checks with shared indices" begin
1080+
indices = [1,3]
1081+
a = rand(3)
1082+
av = @view a[indices]
1083+
b = rand(3)
1084+
bv = @view b[indices]
1085+
@test !Base.mightalias(av, bv)
1086+
@test Base.mightalias(a, av)
1087+
@test Base.mightalias(b, bv)
1088+
@test Base.mightalias(indices, av)
1089+
@test Base.mightalias(indices, bv)
1090+
@test Base.mightalias(view(indices, :), av)
1091+
@test Base.mightalias(view(indices, :), bv)
1092+
end
1093+
1094+
@testset "aliasing checks with disjoint arrays" begin
1095+
A = rand(3,4,5)
1096+
@test Base.mightalias(view(A, :, :, 1), view(A, :, :, 1))
1097+
@test !Base.mightalias(view(A, :, :, 1), view(A, :, :, 2))
1098+
1099+
B = reinterpret(UInt64, A)
1100+
@test Base.mightalias(view(B, :, :, 1), view(A, :, :, 1))
1101+
@test !Base.mightalias(view(B, :, :, 1), view(A, :, :, 2))
1102+
1103+
C = reinterpret(UInt32, A)
1104+
@test Base.mightalias(view(C, :, :, 1), view(A, :, :, 1))
1105+
@test Base.mightalias(view(C, :, :, 1), view(A, :, :, 2)) # This is overly conservative
1106+
@test Base.mightalias(@view(C[begin:2:end, :, 1]), view(A, :, :, 1))
1107+
@test Base.mightalias(@view(C[begin:2:end, :, 1]), view(A, :, :, 2)) # This is overly conservative
1108+
end

0 commit comments

Comments
 (0)