Skip to content

Commit 8985ef7

Browse files
jishnubjishnub
authored
Fix Int overflow bugs in offset computation (#237)
* fix Int overflow bugs in offset computation * add tests * use kwarg checkoverflow to disable the overflow check * fix overflow check for origin
1 parent 637018e commit 8985ef7

File tree

3 files changed

+67
-69
lines changed

3 files changed

+67
-69
lines changed

‎src/OffsetArrays.jl‎

Lines changed: 55 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,9 @@ julia> OffsetArray(a, OffsetArrays.Origin(0)) # set the origin to zero along eac
110110
struct OffsetArray{T,N,AA<:AbstractArray{T,N}} <: AbstractArray{T,N}
111111
parent::AA
112112
offsets::NTuple{N,Int}
113-
function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray{T,N}}
113+
@inline function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}; checkoverflow = true) where {T, N, AA<:AbstractArray{T,N}}
114114
# allocation of `map` on tuple is optimized away
115-
map(overflow_check, axes(parent), offsets)
115+
checkoverflow && map(overflow_check, axes(parent), offsets)
116116
new{T, N, AA}(parent, offsets)
117117
end
118118
end
@@ -132,53 +132,40 @@ Type alias and convenience constructor for two-dimensional [`OffsetArray`](@ref)
132132
const OffsetMatrix{T,AA<:AbstractMatrix{T}} = OffsetArray{T,2,AA}
133133

134134
# checks if the offset may be added to the range without overflowing
135-
function overflow_check(r::AbstractUnitRange{T}, offset::Integer) where {T<:Integer}
136-
Base.hastypemax(T) || return nothing
135+
function overflow_check(r::AbstractUnitRange, offset::Integer)
136+
Base.hastypemax(eltype(r)) || return nothing
137137
# This gives some performance boost https://github.com/JuliaLang/julia/issues/33273
138-
throw_upper_overflow_error(val) = throw(OverflowError("offset should be <= $(typemax(T) - val) corresponding to the axis $r, received an offset $offset"))
139-
throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(T) - val) corresponding to the axis $r, received an offset $offset"))
138+
throw_upper_overflow_error(val) = throw(OverflowError("offset should be <= $(typemax(Int) - val) corresponding to the axis $r, received an offset $offset"))
139+
throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(Int) - val) corresponding to the axis $r, received an offset $offset"))
140140

141141
# With ranges in the picture, first(r) might not necessarily be < last(r)
142142
# we therefore use the min and max of first(r) and last(r) to check for overflow
143143
firstlast_min, firstlast_max = minmax(first(r), last(r))
144144

145-
if offset > 0 && firstlast_max > typemax(T) - offset
145+
if offset > 0 && firstlast_max > typemax(Int) - offset
146146
throw_upper_overflow_error(firstlast_max)
147-
elseif offset < 0 && firstlast_min < typemin(T) - offset
147+
elseif offset < 0 && firstlast_min < typemin(Int) - offset
148148
throw_lower_overflow_error(firstlast_min)
149149
end
150150
return nothing
151151
end
152-
# checks if the two offsets may be added together without overflowing
153-
function overflow_check(offset_preexisting::Integer, offset_new::T) where {T<:Integer}
154-
Base.hastypemax(T) || return nothing
155-
throw_upper_overflow_error() = throw(OverflowError("offset should be <= $(typemax(eltype(offset_preexisting)) - offset_preexisting) given a pre-existing offset of $offset_preexisting, received an offset $offset_new"))
156-
throw_lower_overflow_error() = throw(OverflowError("offset should be >= $(typemin(eltype(offset_preexisting)) - offset_preexisting) given a pre-existing offset of $offset_preexisting, received an offset $offset_new"))
157-
158-
if offset_preexisting > 0 && offset_new > typemax(T) - offset_preexisting
159-
throw_upper_overflow_error()
160-
elseif offset_preexisting < 0 && offset_new < typemin(T) - offset_preexisting
161-
throw_lower_overflow_error()
162-
end
163-
return nothing
164-
end
165152

166153
# Tuples of integers are treated as offsets
167154
# Empty Tuples are handled here
168-
@inline function OffsetArray(A::AbstractArray, offsets::Tuple{Vararg{Integer}})
155+
@inline function OffsetArray(A::AbstractArray, offsets::Tuple{Vararg{Integer}}; kw...)
169156
_checkindices(A, offsets, "offsets")
170-
OffsetArray{eltype(A), ndims(A), typeof(A)}(A, offsets)
157+
OffsetArray{eltype(A), ndims(A), typeof(A)}(A, offsets; kw...)
171158
end
172159

173160
# These methods are necessary to disallow incompatible dimensions for
174161
# the OffsetVector and the OffsetMatrix constructors
175162
for (FT, ND) in ((:OffsetVector, :1), (:OffsetMatrix, :2))
176-
@eval @inline function $FT(A::AbstractArray{<:Any,$ND}, offsets::Tuple{Vararg{Integer}})
163+
@eval @inline function $FT(A::AbstractArray{<:Any,$ND}, offsets::Tuple{Vararg{Integer}}; kw...)
177164
_checkindices(A, offsets, "offsets")
178-
OffsetArray{eltype(A), $ND, typeof(A)}(A, offsets)
165+
OffsetArray{eltype(A), $ND, typeof(A)}(A, offsets; kw...)
179166
end
180167
FTstr = string(FT)
181-
@eval @inline function $FT(A::AbstractArray, offsets::Tuple{Vararg{Integer}})
168+
@eval @inline function $FT(A::AbstractArray, offsets::Tuple{Vararg{Integer}}; kw...)
182169
throw(ArgumentError($FTstr*" requires a "*string($ND)*"D array"))
183170
end
184171
end
@@ -187,95 +174,98 @@ end
187174
for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix)
188175
# Nested OffsetArrays may strip off the wrapper and collate the offsets
189176
# empty tuples are handled here
190-
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Vararg{Int}})
177+
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Vararg{Int}}; checkoverflow = true)
191178
_checkindices(A, offsets, "offsets")
192179
# ensure that the offsets may be added together without an overflow
193-
map(overflow_check, A.offsets, offsets)
194-
$FT(parent(A), map(+, A.offsets, offsets))
180+
checkoverflow && map(overflow_check, axes(A), offsets)
181+
I = map(+, _offsets(A, parent(A)), offsets)
182+
$FT(parent(A), I, checkoverflow = false)
195183
end
196-
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Integer,Vararg{Integer}})
197-
$FT(A, map(Int, offsets))
184+
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Integer,Vararg{Integer}}; kw...)
185+
$FT(A, map(Int, offsets); kw...)
198186
end
199187

200188
# In general, indices get converted to AbstractUnitRanges.
201189
# CartesianIndices{N} get converted to N ranges
202-
@eval @inline function $FT(A::AbstractArray, inds::Tuple{Any,Vararg{Any}})
203-
$FT(A, _toAbstractUnitRanges(to_indices(A, axes(A), inds)))
190+
@eval @inline function $FT(A::AbstractArray, inds::Tuple{Any,Vararg{Any}}; kw...)
191+
$FT(A, _toAbstractUnitRanges(to_indices(A, axes(A), inds)); kw...)
204192
end
205193

206194
# convert ranges to offsets
207-
@eval @inline function $FT(A::AbstractArray, inds::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}})
195+
@eval @inline function $FT(A::AbstractArray, inds::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}; kw...)
208196
_checkindices(A, inds, "indices")
209197
# Performance gain by wrapping the error in a function: see https://github.com/JuliaLang/julia/issues/37558
210198
throw_dimerr(lA, lI) = throw(DimensionMismatch("supplied axes do not agree with the size of the array (got size $lA for the array and $lI for the indices"))
211199
lA = size(A)
212200
lI = map(length, inds)
213201
lA == lI || throw_dimerr(lA, lI)
214-
$FT(A, map(_offset, axes(A), inds))
202+
$FT(A, map(_offset, axes(A), inds); kw...)
215203
end
216204

217-
@eval @inline $FT(A::AbstractArray, inds::Vararg) = $FT(A, inds)
218-
@eval @inline $FT(A::AbstractArray) = $FT(A, ntuple(zero, Val(ndims(A))))
205+
@eval @inline $FT(A::AbstractArray, inds::Vararg; kw...) = $FT(A, inds; kw...)
206+
@eval @inline $FT(A::AbstractArray; checkoverflow = false) = $FT(A, ntuple(zero, Val(ndims(A))), checkoverflow = checkoverflow)
219207

220-
@eval @inline $FT(A::AbstractArray, origin::Origin) = $FT(A, origin(A))
208+
@eval @inline $FT(A::AbstractArray, origin::Origin; checkoverflow = true) = $FT(A, origin(A); checkoverflow = checkoverflow)
221209
end
222210

223211
# conversion-related methods
224-
@inline OffsetArray{T}(M::AbstractArray, I...) where {T} = OffsetArray{T,ndims(M)}(M, I...)
212+
@inline OffsetArray{T}(M::AbstractArray, I...; kw...) where {T} = OffsetArray{T,ndims(M)}(M, I...; kw...)
225213

226-
@inline function OffsetArray{T,N}(M::AbstractArray{<:Any,N}, I...) where {T,N}
214+
@inline function OffsetArray{T,N}(M::AbstractArray{<:Any,N}, I...; kw...) where {T,N}
227215
M2 = _of_eltype(T, M)
228-
OffsetArray{T,N,typeof(M2)}(M2, I...)
216+
OffsetArray{T,N}(M2, I...; kw...)
229217
end
218+
@inline OffsetArray{T,N}(M::OffsetArray{T,N}, I...; kw...) where {T,N} = OffsetArray(M, I...; kw...)
219+
@inline OffsetArray{T,N}(M::AbstractArray{T,N}, I...; kw...) where {T,N} = OffsetArray{T,N,typeof(M)}(M, I...; kw...)
230220

231-
@inline OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Vararg) where {T,N,A<:AbstractArray{T,N}} = OffsetArray{T,N,A}(M, I)
232-
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::NTuple{N,Int}) where {T,N,A<:AbstractArray{T,N}}
233-
map(overflow_check, axes(M), I)
221+
@inline OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Vararg; kw...) where {T,N,A<:AbstractArray{T,N}} = OffsetArray{T,N,A}(M, I; kw...)
222+
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::NTuple{N,Int}; checkoverflow = true) where {T,N,A<:AbstractArray{T,N}}
223+
checkoverflow && map(overflow_check, axes(M), I)
234224
Mv = no_offset_view(M)
235225
MvA = convert(A, Mv)::A
236226
Iof = map(+, _offsets(M), I)
237-
OffsetArray{T,N,A}(MvA, Iof)
227+
OffsetArray{T,N,A}(MvA, Iof, checkoverflow = false)
238228
end
239-
@inline function OffsetArray{T, N, AA}(parent::AbstractArray{<:Any,N}, offsets::NTuple{N, Integer}) where {T, N, AA<:AbstractArray{T,N}}
240-
OffsetArray{T, N, AA}(parent, map(Int, offsets)::NTuple{N,Int})
229+
@inline function OffsetArray{T, N, AA}(parent::AbstractArray{<:Any,N}, offsets::NTuple{N, Integer}; kw...) where {T, N, AA<:AbstractArray{T,N}}
230+
OffsetArray{T, N, AA}(parent, map(Int, offsets)::NTuple{N,Int}; kw...)
241231
end
242-
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}) where {T,N,A<:AbstractArray{T,N}}
232+
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}; kw...) where {T,N,A<:AbstractArray{T,N}}
243233
_checkindices(M, I, "indices")
244234
# Performance gain by wrapping the error in a function: see https://github.com/JuliaLang/julia/issues/37558
245235
throw_dimerr(lA, lI) = throw(DimensionMismatch("supplied axes do not agree with the size of the array (got size $lA for the array and $lI for the indices"))
246236
lM = size(M)
247237
lI = map(length, I)
248238
lM == lI || throw_dimerr(lM, lI)
249-
OffsetArray{T,N,A}(M, map(_offset, axes(M), I))
239+
OffsetArray{T,N,A}(M, map(_offset, axes(M), I); kw...)
250240
end
251-
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Tuple) where {T,N,A<:AbstractArray{T,N}}
252-
OffsetArray{T,N,A}(M, _toAbstractUnitRanges(to_indices(M, axes(M), I)))
241+
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Tuple; kw...) where {T,N,A<:AbstractArray{T,N}}
242+
OffsetArray{T,N,A}(M, _toAbstractUnitRanges(to_indices(M, axes(M), I)); kw...)
253243
end
254-
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}) where {T,N,A<:AbstractArray{T,N}}
244+
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}; kw...) where {T,N,A<:AbstractArray{T,N}}
255245
Mv = no_offset_view(M)
256246
MvA = convert(A, Mv)::A
257-
OffsetArray{T,N,A}(MvA, _offsets(M))
247+
OffsetArray{T,N,A}(MvA, _offsets(M); kw...)
258248
end
259-
@inline OffsetArray{T,N,A}(M::A) where {T,N,A<:AbstractArray{T,N}} = OffsetArray{T,N,A}(M, ntuple(zero, Val(N)))
249+
@inline OffsetArray{T,N,A}(M::A; checkoverflow = false) where {T,N,A<:AbstractArray{T,N}} = OffsetArray{T,N,A}(M, ntuple(zero, Val(N)); checkoverflow = checkoverflow)
260250

261251
Base.convert(::Type{T}, M::AbstractArray) where {T<:OffsetArray} = M isa T ? M : T(M)
262252

263253
# array initialization
264-
@inline function OffsetArray{T,N}(init::ArrayInitializer, inds::Tuple{Vararg{OffsetAxisKnownLength}}) where {T,N}
254+
@inline function OffsetArray{T,N}(init::ArrayInitializer, inds::Tuple{Vararg{OffsetAxisKnownLength}}; kw...) where {T,N}
265255
_checkindices(N, inds, "indices")
266256
AA = Array{T,N}(init, map(_indexlength, inds))
267-
OffsetArray{T, N, typeof(AA)}(AA, map(_indexoffset, inds))
257+
OffsetArray{T, N, typeof(AA)}(AA, map(_indexoffset, inds); kw...)
268258
end
269-
@inline function OffsetArray{T, N}(init::ArrayInitializer, inds::Tuple) where {T, N}
270-
OffsetArray{T, N}(init, _toAbstractUnitRanges(inds))
259+
@inline function OffsetArray{T, N}(init::ArrayInitializer, inds::Tuple; kw...) where {T, N}
260+
OffsetArray{T, N}(init, _toAbstractUnitRanges(inds); kw...)
271261
end
272-
@inline OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg) where {T,N} = OffsetArray{T,N}(init, inds)
262+
@inline OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg; kw...) where {T,N} = OffsetArray{T,N}(init, inds; kw...)
273263

274-
@inline OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds)
275-
@inline function OffsetArray{T}(init::ArrayInitializer, inds::Tuple) where {T}
276-
OffsetArray{T}(init, _toAbstractUnitRanges(inds))
264+
@inline OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}; kw...) where {T,N} = OffsetArray{T,N}(init, inds; kw...)
265+
@inline function OffsetArray{T}(init::ArrayInitializer, inds::Tuple; kw...) where {T}
266+
OffsetArray{T}(init, _toAbstractUnitRanges(inds); kw...)
277267
end
278-
@inline OffsetArray{T}(init::ArrayInitializer, inds::Vararg) where {T} = OffsetArray{T}(init, inds)
268+
@inline OffsetArray{T}(init::ArrayInitializer, inds::Vararg; kw...) where {T} = OffsetArray{T}(init, inds; kw...)
279269

280270
Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA))
281271
parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA
@@ -308,7 +298,7 @@ end
308298

309299
# Utils to translate a function to the parent while preserving offsets
310300
unwrap(x) = x, identity
311-
unwrap(x::OffsetArray) = parent(x), data -> OffsetArray(data, x.offsets)
301+
unwrap(x::OffsetArray) = parent(x), data -> OffsetArray(data, x.offsets, checkoverflow = false)
312302
function parent_call(f, x)
313303
parent, wrap_offset = unwrap(x)
314304
wrap_offset(f(parent))
@@ -469,7 +459,7 @@ end
469459
# An OffsetUnitRange might use the rapid getindex(::Array, ::AbstractUnitRange{Int}) for contiguous indexing
470460
@propagate_inbounds function Base.getindex(A::Array, r::OffsetUnitRange{Int})
471461
B = A[_contiguousindexingtype(parent(r))]
472-
OffsetArray(B, axes(r))
462+
OffsetArray(B, axes(r), checkoverflow = false)
473463
end
474464

475465
# avoid hitting the slow method getindex(::Array, ::AbstractRange{Int})

‎src/utils.jl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ _offset(axparent::AbstractUnitRange, ax::AbstractUnitRange) = first(ax) - first(
1414
_offset(axparent::AbstractUnitRange, ::Union{Integer, Colon}) = 1 - first(axparent)
1515

1616
_offsets(A::AbstractArray) = map(ax -> first(ax) - 1, axes(A))
17+
_offsets(A::AbstractArray, B::AbstractArray) = map(_offset, axes(B), axes(A))
1718

1819
"""
1920
OffsetArrays.AxisConversionStyle(typeof(indices))

‎test/runtests.jl‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -538,12 +538,19 @@ Base.Int(a::WeirdInteger) = a
538538
@test axes(OffsetVector(v, typemax(Int)-length(v))) == (IdOffsetRange(axes(v)[1], typemax(Int)-length(v)), )
539539
@test_throws OverflowError OffsetVector(v, typemax(Int)-length(v)+1)
540540
ao = OffsetArray(v, typemin(Int))
541-
@test_nowarn OffsetArray{Float64, 1, typeof(ao)}(ao, (-1, ))
541+
ao2 = OffsetArray{Float64, 1, typeof(ao)}(ao, (-1, ))
542+
@test axes(ao2, 1) == typemin(Int) .+ (0:length(v)-1)
543+
ao2 = OffsetArray(ao, (-1,))
544+
@test axes(ao2, 1) == typemin(Int) .+ (0:length(v)-1)
542545
@test_throws OverflowError OffsetArray{Float64, 1, typeof(ao)}(ao, (-2, )) # inner Constructor
543546
@test_throws OverflowError OffsetArray(ao, (-2, )) # convinient constructor accumulate offsets
544547
@test_throws OverflowError OffsetVector(1:0, typemax(Int))
545548
@test_throws OverflowError OffsetVector(OffsetVector(1:0, 0), typemax(Int))
546549
@test_throws OverflowError OffsetArray(zeros(Int, typemax(Int):typemax(Int)), 2)
550+
@test_throws OverflowError OffsetArray(v, OffsetArrays.Origin(typemax(Int)))
551+
552+
b = OffsetArray(OffsetArray(big(1):2, 1), typemax(Int)-1)
553+
@test axes(b, 1) == big(typemax(Int)) .+ (1:2)
547554

548555
@testset "OffsetRange" begin
549556
for r in Any[1:100, big(1):big(2)]
@@ -1916,10 +1923,10 @@ end
19161923
b = map(BigInt, a)
19171924
@test eltype(b) == BigInt
19181925
@test b == a
1919-
@test b isa OffsetArrays.OffsetRange
1926+
@test parent(b) isa AbstractRange
19201927

19211928
for ri in Any[2:3, Base.OneTo(2)]
1922-
for r in [IdentityUnitRange(ri), IdOffsetRange(ri), IdOffsetRange(ri, 1)]
1929+
for r in [IdentityUnitRange(ri), IdOffsetRange(ri), IdOffsetRange(ri, 1), OffsetArray(ri), OffsetArray(ri, 2)]
19231930
for T in [Int8, Int16, Int32, Int64, Int128, BigInt, Float32, Float64, BigFloat]
19241931
r2 = map(T, r)
19251932
@test eltype(r2) == T
@@ -1931,7 +1938,7 @@ end
19311938

19321939
@testset "Bool" begin
19331940
for ri in Any[0:0, 0:1, 1:0, 1:1, Base.OneTo(0), Base.OneTo(1)]
1934-
for r = Any[IdentityUnitRange(ri), IdOffsetRange(ri), IdOffsetRange(ri .- 1, 1)]
1941+
for r = Any[IdentityUnitRange(ri), IdOffsetRange(ri), IdOffsetRange(ri .- 1, 1), OffsetVector(ri)]
19351942
r2 = map(Bool, r)
19361943
@test eltype(r2) == Bool
19371944
@test axes(r2) == axes(r)

0 commit comments

Comments
 (0)