Skip to content

Commit 97bf148

Browse files
authored
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
1 parent 1de7623 commit 97bf148

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
@@ -1047,25 +1047,34 @@ end
10471047

10481048
### from abstractarray.jl
10491049

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

1077+
# Given two SubArrays with the same parent, check if the indices might overlap (returning true if unsure)
10691078
_indicesmightoverlap(A::Tuple{}, B::Tuple{}) = true
10701079
_indicesmightoverlap(A::Tuple{}, B::Tuple) = error("malformed subarray")
10711080
_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
@@ -1077,6 +1077,37 @@ end
10771077
end
10781078
end
10791079

1080+
@testset "aliasing checks with shared indices" begin
1081+
indices = [1,3]
1082+
a = rand(3)
1083+
av = @view a[indices]
1084+
b = rand(3)
1085+
bv = @view b[indices]
1086+
@test !Base.mightalias(av, bv)
1087+
@test Base.mightalias(a, av)
1088+
@test Base.mightalias(b, bv)
1089+
@test Base.mightalias(indices, av)
1090+
@test Base.mightalias(indices, bv)
1091+
@test Base.mightalias(view(indices, :), av)
1092+
@test Base.mightalias(view(indices, :), bv)
1093+
end
1094+
1095+
@testset "aliasing checks with disjoint arrays" begin
1096+
A = rand(3,4,5)
1097+
@test Base.mightalias(view(A, :, :, 1), view(A, :, :, 1))
1098+
@test !Base.mightalias(view(A, :, :, 1), view(A, :, :, 2))
1099+
1100+
B = reinterpret(UInt64, A)
1101+
@test Base.mightalias(view(B, :, :, 1), view(A, :, :, 1))
1102+
@test !Base.mightalias(view(B, :, :, 1), view(A, :, :, 2))
1103+
1104+
C = reinterpret(UInt32, A)
1105+
@test Base.mightalias(view(C, :, :, 1), view(A, :, :, 1))
1106+
@test Base.mightalias(view(C, :, :, 1), view(A, :, :, 2)) # This is overly conservative
1107+
@test Base.mightalias(@view(C[begin:2:end, :, 1]), view(A, :, :, 1))
1108+
@test Base.mightalias(@view(C[begin:2:end, :, 1]), view(A, :, :, 2)) # This is overly conservative
1109+
end
1110+
10801111
@testset "aliasing check with reshaped subarrays" begin
10811112
C = rand(2,1)
10821113
V1 = @view C[1, :]

0 commit comments

Comments
 (0)