From 082c041b4233b17b80129d4ac6b33a28014442b0 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Mon, 25 May 2020 20:13:47 +0200 Subject: [PATCH] bio printf: Avoid using rounding errors in range check There is a problem casting ULONG_MAX to double which clang-10 is warning about. ULONG_MAX typically cannot be exactly represented as a double. ULONG_MAX + 1 can be and this fix uses the latter, however since ULONG_MAX cannot be represented exactly as a double number we subtract 65535 from this number, and the result has at most 48 leading one bits, and can therefore be represented as a double integer without rounding error. By adding 65536.0 to this number we achive the correct result, which should avoid the warning. The addresses a symptom of the underlying problem: we print doubles via an unsigned long integer. Doubles have a far greater range and should be printed better. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/11955) --- crypto/bio/b_print.c | 8 +++++++- test/bioprinttest.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/crypto/bio/b_print.c b/crypto/bio/b_print.c index 0d6fafcc2d..6b995f8233 100644 --- a/crypto/bio/b_print.c +++ b/crypto/bio/b_print.c @@ -635,7 +635,13 @@ fmtfp(char **sbuffer, fvalue = tmpvalue; } ufvalue = abs_val(fvalue); - if (ufvalue > ULONG_MAX) { + /* + * By subtracting 65535 (2^16-1) we cancel the low order 15 bits + * of ULONG_MAX to avoid using imprecise floating point values. + * The second condition is necessary to catch NaN values. + */ + if (ufvalue >= (double)(ULONG_MAX - 65535) + 65536.0 + || !(ufvalue == ufvalue) /* NaN */) { /* Number too big */ return 0; } diff --git a/test/bioprinttest.c b/test/bioprinttest.c index 14f0bfe52d..3dd5b3efa2 100644 --- a/test/bioprinttest.c +++ b/test/bioprinttest.c @@ -241,14 +241,48 @@ static int test_fp(int i) return r; } +extern double zero_value; +double zero_value = 0.0; + static int test_big(void) { char buf[80]; + double d, z, inf, nan; /* Test excessively big number. Should fail */ if (!TEST_int_eq(BIO_snprintf(buf, sizeof(buf), "%f\n", 2 * (double)ULONG_MAX), -1)) return 0; + + d = 1.0; + z = zero_value; + inf = d / z; + nan = z / z; + + /* + * Test +/-inf, nan. Should fail. + * Test +/-1.0, +/-0.0. Should work. + */ + if (!TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", inf), -1) + || !TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", -inf), -1) + || !TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", nan), -1) + || !TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", d), 8) + || !TEST_str_eq(buf, "1.000000") + || !TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", z), 8) + || !TEST_str_eq(buf, "0.000000") + || !TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", -d), 9) + || !TEST_str_eq(buf, "-1.000000") + || !TEST_int_eq(BIO_snprintf(buf, sizeof(buf), + "%f", -z), 8) + || !TEST_str_eq(buf, "0.000000")) + return 0; + return 1; } -- 2.25.1