Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect aliasing in non-scalar indexed assignments #15209

Closed
mbauman opened this issue Feb 23, 2016 · 2 comments · Fixed by #25890
Closed

Detect aliasing in non-scalar indexed assignments #15209

mbauman opened this issue Feb 23, 2016 · 2 comments · Fixed by #25890
Labels
domain:arrays [a, r, r, a, y, s] kind:speculative Whether the change will be implemented is speculative

Comments

@mbauman
Copy link
Sponsor Member

mbauman commented Feb 23, 2016

Right now we only check for aliasing in the most simple of array assignments (and only for exact object equality: A===I). But with views becoming more prevalent, we'll need to add more aliasing checks to prevent corruption and unexpected behavior.

It'll be tricky to do in a type-stable manner since copy(A)::typeof(A) isn't true for views.

julia> A = [2 1]
       A[A] = -4
       A
0×2 Array{Int64,2}

julia> B = [1, 2]
       B[[2, 1]] = view(B, :)
       B
2-element Array{Int64,1}:
 1
 1

julia> C = [2 1]
       C[C] = -2^30

signal (11): Segmentation fault: 11
@vtjnash
Copy link
Sponsor Member

vtjnash commented Mar 8, 2016

maybe we should have a function that is defined to return the base array pointer, for checking for aliasing? or some more explicit / less conservative overlap computation such as a base + length conservative estimate?

@vtjnash vtjnash added the kind:speculative Whether the change will be implemented is speculative label Mar 8, 2016
@mbauman
Copy link
Sponsor Member Author

mbauman commented Mar 8, 2016

It's a little tricky with non-strided arrays, but I think a coarse data_id (a la object_id) could work:

data_id(A::StridedArray) = A === parent(A) ? UInt(pointer(A)) : data_id(parent(A))
data_id(A::AbstractArray) = A === parent(A) ? object_id(A) : data_id(parent(A))

For reference, Numpy does much more exact checking, and has some very nice documentation and algorithms for detecting strided array overlaps: https://github.com/numpy/numpy/blob/65aa24aa9e3c5da0a6ec1c2fc3d335d506a511df/numpy/core/src/private/mem_overlap.c. Our case is significantly easier, though, since we can punt on arrays that were constructed with pointer_to_array with interior pointers — if they don't track their parent arrays they're in dangerous GC territory.

@mbauman mbauman added the domain:arrays [a, r, r, a, y, s] label Aug 30, 2017
mbauman added a commit that referenced this issue Feb 27, 2018
…ignment (#25890)

This commit puts together the beginnings of an "interface" for detecting and avoiding aliasing between arrays before performing some mutating operation on one of them.

`A′ = unalias(dest, A)` is the main entry point; it checks to see if `dest` and `A` might alias one another, and then either returns `A` or a copy of `A`. When it returns a copy of `A`, it absolutely must preserve the same type. As such, this function calls an internal `unaliascopy` function, which defaults to `copy` but ensures it returns the same type.

`mightalias(A, B)` is a quick and conservative check to see if two arrays alias eachother.  Instead of requiring every array to know about every other array type, it simply asks both arrays for their `dataids` and then looks for an empty intersection.

Finally, `dataids(A)` returns a tuple of `UInt`s that describe something about the region of memory the array occupies. It defaults to simply `(objectid(A),)`. A custom implementation for an array with multiple fields of arrays would look something like `(dataids(A.a)..., dataids(A.b)..., ...)`.

Fixes #21693, fixes #15209, and a pre-req to get tests passing for #24368 (since we had spot checks for `Array` aliasing, but moving to broadcast means we need a slightly more generic solution).  These functions remain un-exported for now since I'd like us to use them a bit more before we officially support them for all custom arrays.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays [a, r, r, a, y, s] kind:speculative Whether the change will be implemented is speculative
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants