Skip to content

Commit

Permalink
Fix integer overflow in isapprox (#50730)
Browse files Browse the repository at this point in the history
Ensure that `isapprox` gives correct results when comparing an integer
with another integer or with a float. For comparison between integers,
the fix only works when keeping default values for `rtol` and `norm`,
and with `atol < 1`.

It is not possible to handle the (atypical) case where `norm !== abs`,
but that's OK since the user is responsible for providing a safe
function.

It would be possible to handle the case where `rtol > 0` or `atol >= 1`,
but with complex code which would check for overflow and handle all
possible corner cases; it would work only for types defined in Base and
would not be extensible by packages. So I'm not sure that's worth it. At
least with PR fixes the most common case.

Fixes #50380.

(cherry picked from commit 5f03a18)
  • Loading branch information
nalimilan authored and IanButterworth committed Aug 19, 2023
1 parent cf29443 commit c50c579
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
15 changes: 14 additions & 1 deletion base/floatfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,20 @@ true
function isapprox(x::Number, y::Number;
atol::Real=0, rtol::Real=rtoldefault(x,y,atol),
nans::Bool=false, norm::Function=abs)
x == y || (isfinite(x) && isfinite(y) && norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))) || (nans && isnan(x) && isnan(y))
x′, y′ = promote(x, y) # to avoid integer overflow
x == y ||
(isfinite(x) && isfinite(y) && norm(x-y) <= max(atol, rtol*max(norm(x′), norm(y′)))) ||
(nans && isnan(x) && isnan(y))
end

function isapprox(x::Integer, y::Integer;
atol::Real=0, rtol::Real=rtoldefault(x,y,atol),
nans::Bool=false, norm::Function=abs)
if norm === abs && atol < 1 && rtol == 0
return x == y
else
return norm(x - y) <= max(atol, rtol*max(norm(x), norm(y)))
end
end

"""
Expand Down
44 changes: 44 additions & 0 deletions test/floatfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,47 @@ end
struct CustomNumber <: Number end
@test !isnan(CustomNumber())
end

@testset "isapprox and integer overflow" begin
for T in (Int8, Int16, Int32)
T === Int && continue
@test !isapprox(typemin(T), T(0))
@test !isapprox(typemin(T), unsigned(T)(0))
@test !isapprox(typemin(T), 0)
@test !isapprox(typemin(T), T(0), atol=0.99)
@test !isapprox(typemin(T), unsigned(T)(0), atol=0.99)
@test !isapprox(typemin(T), 0, atol=0.99)
@test_broken !isapprox(typemin(T), T(0), atol=1)
@test_broken !isapprox(typemin(T), unsigned(T)(0), atol=1)
@test !isapprox(typemin(T), 0, atol=1)

@test !isapprox(typemin(T)+T(10), T(10))
@test !isapprox(typemin(T)+T(10), unsigned(T)(10))
@test !isapprox(typemin(T)+T(10), 10)
@test !isapprox(typemin(T)+T(10), T(10), atol=0.99)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=0.99)
@test !isapprox(typemin(T)+T(10), 10, atol=0.99)
@test_broken !isapprox(typemin(T)+T(10), T(10), atol=1)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=1)
@test !isapprox(typemin(T)+T(10), 10, atol=1)

@test isapprox(typemin(T), 0.0, rtol=1)
end
for T in (Int, Int64, Int128)
@test !isapprox(typemin(T), T(0))
@test !isapprox(typemin(T), unsigned(T)(0))
@test !isapprox(typemin(T), T(0), atol=0.99)
@test !isapprox(typemin(T), unsigned(T)(0), atol=0.99)
@test_broken !isapprox(typemin(T), T(0), atol=1)
@test_broken !isapprox(typemin(T), unsigned(T)(0), atol=1)

@test !isapprox(typemin(T)+T(10), T(10))
@test !isapprox(typemin(T)+T(10), unsigned(T)(10))
@test !isapprox(typemin(T)+T(10), T(10), atol=0.99)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=0.99)
@test_broken !isapprox(typemin(T)+T(10), T(10), atol=1)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=1)

@test isapprox(typemin(T), 0.0, rtol=1)
end
end

0 comments on commit c50c579

Please sign in to comment.