From ca72b59cb8fb8e242e4ecb37d386111bbbe87192 Mon Sep 17 00:00:00 2001 From: Jishnu Bhattacharya Date: Fri, 1 Jul 2022 01:03:03 +0530 Subject: [PATCH] Fix integer overflow in `reverse!` (#45871) (cherry picked from commit 3c049196070150bdb1135149ea6f52b61ba4f0c6) --- base/array.jl | 28 +++++++++++++++++----------- base/sort.jl | 7 +------ test/offsetarray.jl | 17 +++++++++++++++++ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/base/array.jl b/base/array.jl index b8ad7e137f25e..baef032f96b3f 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1833,6 +1833,11 @@ function reverseind(a::AbstractVector, i::Integer) first(li) + last(li) - i end +# This implementation of `midpoint` is performance-optimized but safe +# only if `lo <= hi`. +midpoint(lo::T, hi::T) where T<:Integer = lo + ((hi - lo) >>> 0x01) +midpoint(lo::Integer, hi::Integer) = midpoint(promote(lo, hi)...) + """ reverse!(v [, start=1 [, stop=length(v) ]]) -> v @@ -1861,17 +1866,18 @@ julia> A """ function reverse!(v::AbstractVector, start::Integer, stop::Integer=lastindex(v)) s, n = Int(start), Int(stop) - liv = LinearIndices(v) - if n <= s # empty case; ok - elseif !(first(liv) ≤ s ≤ last(liv)) - throw(BoundsError(v, s)) - elseif !(first(liv) ≤ n ≤ last(liv)) - throw(BoundsError(v, n)) - end - r = n - @inbounds for i in s:div(s+n-1, 2) - v[i], v[r] = v[r], v[i] - r -= 1 + if n > s # non-empty and non-trivial + liv = LinearIndices(v) + if !(first(liv) ≤ s ≤ last(liv)) + throw(BoundsError(v, s)) + elseif !(first(liv) ≤ n ≤ last(liv)) + throw(BoundsError(v, n)) + end + r = n + @inbounds for i in s:midpoint(s, n-1) + v[i], v[r] = v[r], v[i] + r -= 1 + end end return v end diff --git a/base/sort.jl b/base/sort.jl index d26e9a4b09332..53ebefd1840ab 100644 --- a/base/sort.jl +++ b/base/sort.jl @@ -11,7 +11,7 @@ using .Base: copymutable, LinearIndices, length, (:), AbstractMatrix, AbstractUnitRange, isless, identity, eltype, >, <, <=, >=, |, +, -, *, !, extrema, sub_with_overflow, add_with_overflow, oneunit, div, getindex, setindex!, length, resize!, fill, Missing, require_one_based_indexing, keytype, - UnitRange, max, min + UnitRange, max, min, midpoint using .Base: >>>, !== @@ -166,11 +166,6 @@ same thing as `partialsort!` but leaving `v` unmodified. partialsort(v::AbstractVector, k::Union{Integer,OrdinalRange}; kws...) = partialsort!(copymutable(v), k; kws...) -# This implementation of `midpoint` is performance-optimized but safe -# only if `lo <= hi`. -midpoint(lo::T, hi::T) where T<:Integer = lo + ((hi - lo) >>> 0x01) -midpoint(lo::Integer, hi::Integer) = midpoint(promote(lo, hi)...) - # reference on sorted binary search: # http://www.tbray.org/ongoing/When/200x/2003/03/22/Binary diff --git a/test/offsetarray.jl b/test/offsetarray.jl index 7621e14013627..b6aab384f2cd4 100644 --- a/test/offsetarray.jl +++ b/test/offsetarray.jl @@ -415,6 +415,23 @@ rv = reverse(v) cv = copy(v) @test reverse!(cv) == rv +@testset "reverse! (issue #45870)" begin + @testset for n in [4,5] + offset = typemax(Int)-n + vo = OffsetArray([1:n;], offset) + vo2 = OffsetArray([1:n;], offset) + @test reverse!(vo) == OffsetArray(n:-1:1, offset) + @test reverse!(vo) == vo2 + @test_throws BoundsError reverse!(vo, firstindex(vo)-1, firstindex(vo)) + @test reverse!(vo, firstindex(vo), firstindex(vo)-1) == vo2 + @test reverse!(vo, firstindex(vo), firstindex(vo)) == vo2 + @test reverse!(vo, lastindex(vo), lastindex(vo)) == vo2 + @test reverse!(vo, lastindex(vo), lastindex(vo)+1) == vo2 # overflow in stop + @test reverse!(vo, firstindex(vo)+1) == OffsetArray([1;n:-1:2], offset) + @test reverse!(vo2, firstindex(vo)+1, lastindex(vo)-1) == OffsetArray([1;n-1:-1:2;n], offset) + end +end + A = OffsetArray(rand(4,4), (-3,5)) @test lastindex(A) == 16 @test lastindex(A, 1) == 1