Skip to content

Commit ca72b59

Browse files
jishnubKristofferC
authored andcommitted
Fix integer overflow in reverse! (#45871)
(cherry picked from commit 3c04919)
1 parent a2868fb commit ca72b59

File tree

3 files changed

+35
-17
lines changed

3 files changed

+35
-17
lines changed

base/array.jl

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1833,6 +1833,11 @@ function reverseind(a::AbstractVector, i::Integer)
18331833
first(li) + last(li) - i
18341834
end
18351835

1836+
# This implementation of `midpoint` is performance-optimized but safe
1837+
# only if `lo <= hi`.
1838+
midpoint(lo::T, hi::T) where T<:Integer = lo + ((hi - lo) >>> 0x01)
1839+
midpoint(lo::Integer, hi::Integer) = midpoint(promote(lo, hi)...)
1840+
18361841
"""
18371842
reverse!(v [, start=1 [, stop=length(v) ]]) -> v
18381843
@@ -1861,17 +1866,18 @@ julia> A
18611866
"""
18621867
function reverse!(v::AbstractVector, start::Integer, stop::Integer=lastindex(v))
18631868
s, n = Int(start), Int(stop)
1864-
liv = LinearIndices(v)
1865-
if n <= s # empty case; ok
1866-
elseif !(first(liv) s last(liv))
1867-
throw(BoundsError(v, s))
1868-
elseif !(first(liv) n last(liv))
1869-
throw(BoundsError(v, n))
1870-
end
1871-
r = n
1872-
@inbounds for i in s:div(s+n-1, 2)
1873-
v[i], v[r] = v[r], v[i]
1874-
r -= 1
1869+
if n > s # non-empty and non-trivial
1870+
liv = LinearIndices(v)
1871+
if !(first(liv) s last(liv))
1872+
throw(BoundsError(v, s))
1873+
elseif !(first(liv) n last(liv))
1874+
throw(BoundsError(v, n))
1875+
end
1876+
r = n
1877+
@inbounds for i in s:midpoint(s, n-1)
1878+
v[i], v[r] = v[r], v[i]
1879+
r -= 1
1880+
end
18751881
end
18761882
return v
18771883
end

base/sort.jl

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ using .Base: copymutable, LinearIndices, length, (:),
1111
AbstractMatrix, AbstractUnitRange, isless, identity, eltype, >, <, <=, >=, |, +, -, *, !,
1212
extrema, sub_with_overflow, add_with_overflow, oneunit, div, getindex, setindex!,
1313
length, resize!, fill, Missing, require_one_based_indexing, keytype,
14-
UnitRange, max, min
14+
UnitRange, max, min, midpoint
1515

1616
using .Base: >>>, !==
1717

@@ -166,11 +166,6 @@ same thing as `partialsort!` but leaving `v` unmodified.
166166
partialsort(v::AbstractVector, k::Union{Integer,OrdinalRange}; kws...) =
167167
partialsort!(copymutable(v), k; kws...)
168168

169-
# This implementation of `midpoint` is performance-optimized but safe
170-
# only if `lo <= hi`.
171-
midpoint(lo::T, hi::T) where T<:Integer = lo + ((hi - lo) >>> 0x01)
172-
midpoint(lo::Integer, hi::Integer) = midpoint(promote(lo, hi)...)
173-
174169
# reference on sorted binary search:
175170
# http://www.tbray.org/ongoing/When/200x/2003/03/22/Binary
176171

test/offsetarray.jl

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,23 @@ rv = reverse(v)
415415
cv = copy(v)
416416
@test reverse!(cv) == rv
417417

418+
@testset "reverse! (issue #45870)" begin
419+
@testset for n in [4,5]
420+
offset = typemax(Int)-n
421+
vo = OffsetArray([1:n;], offset)
422+
vo2 = OffsetArray([1:n;], offset)
423+
@test reverse!(vo) == OffsetArray(n:-1:1, offset)
424+
@test reverse!(vo) == vo2
425+
@test_throws BoundsError reverse!(vo, firstindex(vo)-1, firstindex(vo))
426+
@test reverse!(vo, firstindex(vo), firstindex(vo)-1) == vo2
427+
@test reverse!(vo, firstindex(vo), firstindex(vo)) == vo2
428+
@test reverse!(vo, lastindex(vo), lastindex(vo)) == vo2
429+
@test reverse!(vo, lastindex(vo), lastindex(vo)+1) == vo2 # overflow in stop
430+
@test reverse!(vo, firstindex(vo)+1) == OffsetArray([1;n:-1:2], offset)
431+
@test reverse!(vo2, firstindex(vo)+1, lastindex(vo)-1) == OffsetArray([1;n-1:-1:2;n], offset)
432+
end
433+
end
434+
418435
A = OffsetArray(rand(4,4), (-3,5))
419436
@test lastindex(A) == 16
420437
@test lastindex(A, 1) == 1

0 commit comments

Comments
 (0)