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

Avoid integer overflow when constructing arrays #26

Merged
merged 6 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/FixedSizeArrays.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
const FixedSizeMatrix{T} = FixedSizeArray{T,2}

function FixedSizeArray{T,N}(::UndefInitializer, size::NTuple{N,Int}) where {T,N}
FixedSizeArray{T,N}(Memory{T}(undef, prod(size)), size)
FixedSizeArray{T,N}(Memory{T}(undef, checked_dims(size)), size)
end
function FixedSizeArray{T,N}(::UndefInitializer, size::Vararg{Int,N}) where {T,N}
FixedSizeArray{T,N}(undef, size)
Expand All @@ -38,6 +38,24 @@

Base.isassigned(a::FixedSizeArray, i::Int) = isassigned(a.mem, i)

# safe product of a tuple of integers, for calculating dimensions size

checked_dims_impl(a::Int, ::Tuple{}) = a
function checked_dims_impl(a::Int, t::Tuple{Int,Vararg{Int,N}}) where {N}
nsajko marked this conversation as resolved.
Show resolved Hide resolved
b = first(t)
(m, o) = Base.Checked.mul_with_overflow(a, b)
o && throw(ArgumentError("array dimensions too great, can't represent length"))
r = Base.tail(t)::NTuple{N,Int}
checked_dims_impl(m, r)::Int
end

checked_dims(::Tuple{}) = 1

Check warning on line 52 in src/FixedSizeArrays.jl

View check run for this annotation

Codecov / codecov/patch

src/FixedSizeArrays.jl#L52

Added line #L52 was not covered by tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't seem to be actually used? Or are there constructors I'm missing that'd call it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should call it, I'll add a test:

julia> using FixedSizeArrays

julia> FixedSizeArray{Int,0}(undef, ())
0-dimensional FixedSizeArray{Int64, 0}:
0

function checked_dims(t::Tuple{Int,Vararg{Int,N}}) where {N}
a = first(t)
r = Base.tail(t)::NTuple{N,Int}
checked_dims_impl(a, r)::Int
end

# broadcasting

function Base.BroadcastStyle(::Type{<:FixedSizeArray})
Expand Down
26 changes: 26 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,37 @@ using Test
using FixedSizeArrays
import Aqua

const checked_dims = FixedSizeArrays.checked_dims

function allocated(f::F, args::Vararg{Any,N}) where {F,N}
@allocated f(args...)
end

@testset "FixedSizeArrays" begin
@testset "Aqua.jl" begin
Aqua.test_all(FixedSizeArrays)
end

@testset "Constructors" begin
@test FixedSizeArray{Float64,0}(undef) isa FixedSizeArray{Float64,0}
@test_throws ArgumentError FixedSizeArray{Float64,1}(undef, typemin(Int))
@test_throws ArgumentError FixedSizeArray{Float64,2}(undef, typemax(Int), typemax(Int))
@test_throws ArgumentError FixedSizeArray{Float64,3}(undef, typemax(Int), typemax(Int), 2)
@test_throws ArgumentError FixedSizeArray{Float64,4}(undef, typemax(Int), typemax(Int), 2, 4)
end

@testset "safe computation of length from dimensions size" begin
for n ∈ 0:30
t = Tuple(1:n)
if 20 < n
@test_throws ArgumentError checked_dims(t)
else
@test factorial(n) == prod(t) == @inferred checked_dims(t)
@test iszero(allocated(checked_dims, t))
end
end
end

@testset "FixedSizeVector" begin
v = FixedSizeVector{Float64}(undef, 3)
@test length(v) == 3
Expand Down
Loading