From 8a80439484e7ef46a65cabf927f3e45f724ea518 Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Mon, 14 Aug 2023 13:37:10 +0200 Subject: [PATCH] Fix integer overflow in `isapprox` (#50730) 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 https://github.com/JuliaLang/julia/issues/50380. (cherry picked from commit 5f03a18c615526348ef06bd0144a1498cb0b13a7) --- base/floatfuncs.jl | 15 ++++++++++++++- test/floatfuncs.jl | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/base/floatfuncs.jl b/base/floatfuncs.jl index b3db0f087d211..0c261de2b41c4 100644 --- a/base/floatfuncs.jl +++ b/base/floatfuncs.jl @@ -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 """ diff --git a/test/floatfuncs.jl b/test/floatfuncs.jl index 7e9d8021ac5df..321f1881371a3 100644 --- a/test/floatfuncs.jl +++ b/test/floatfuncs.jl @@ -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