From a1ec8ff493879c680ac92952d6c5ea50ea650314 Mon Sep 17 00:00:00 2001 From: ilammy Date: Tue, 24 Mar 2015 01:17:49 +0200 Subject: [PATCH] Avoid overflow when doing sexp_fx_abs() Naturally, fixed-width integer arithmetics can overflow. Chibi handles it pretty well in general, but one case was missing: it is negation of the minimal negative number that can be represented as a fixnum. That is, sexp_fx_neg() must not be applied to sexp_make_fixnum(SEXP_MIN_FIXNUM) because it overflows and returns an identical fixnum back. sexp_fx_neg() itself seems to be used right in the current code, but sexp_fx_abs()--which is defined in terms of sexp_fx_neg()--could be applied to the forbidden number when used to retrieve an unboxed value via the sexp_unbox_fixnum(sexp_fx_abs(x)) pattern. So I have added a separate macro that safely calculates unboxed absolute value of a fixnum, and replaced sexp_unbox_fixnum(sexp_fx_abs(x)) usages with it. Current implementation uses two-bit tag for fixnums, plus we need one bit for the sign, so fixnums have (machine word - 3) significant bits. Regression tests cover word sizes of 16, 32, 64, and 128 bits (for the sake of past- and future-proofness). sexp_bignum_expt() does not have a regression test because we need to check it with negative exponents like -2^29, so the base must be over at least 2^(2^29) for the differences to be visible. Fun fact: bignum representation of such number takes around 1/32 of the available user- space memory, which makes testing on anything except 32-bit systems unreasonable (4 TB of RAM anyone?) --- bignum.c | 12 ++++---- include/chibi/sexp.h | 2 ++ tests/numeric-tests.scm | 62 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/bignum.c b/bignum.c index da24dff4..b20fb7c1 100644 --- a/bignum.c +++ b/bignum.c @@ -29,7 +29,7 @@ sexp sexp_make_bignum (sexp ctx, sexp_uint_t len) { sexp sexp_fixnum_to_bignum (sexp ctx, sexp a) { sexp res = sexp_make_bignum(ctx, 1); if (!sexp_exceptionp(res)) { - sexp_bignum_data(res)[0] = sexp_unbox_fixnum(sexp_fx_abs(a)); + sexp_bignum_data(res)[0] = sexp_unbox_fx_abs(a); sexp_bignum_sign(res) = sexp_fx_sign(a); } return res; @@ -326,9 +326,9 @@ sexp sexp_bignum_add_fixnum (sexp ctx, sexp a, sexp b) { sexp_gc_preserve1(ctx, c); c = sexp_copy_bignum(ctx, NULL, a, 0); if (sexp_bignum_sign(c) == sexp_fx_sign(b)) - c = sexp_bignum_fxadd(ctx, c, sexp_unbox_fixnum(sexp_fx_abs(b))); + c = sexp_bignum_fxadd(ctx, c, sexp_unbox_fx_abs(b)); else - c = sexp_bignum_fxsub(ctx, c, sexp_unbox_fixnum(sexp_fx_abs(b))); + c = sexp_bignum_fxsub(ctx, c, sexp_unbox_fx_abs(b)); sexp_gc_release1(ctx); return c; } @@ -599,7 +599,7 @@ sexp sexp_bignum_remainder (sexp ctx, sexp a, sexp b) { } sexp sexp_bignum_expt (sexp ctx, sexp a, sexp b) { - sexp_sint_t e = sexp_unbox_fixnum(sexp_fx_abs(b)); + sexp_sint_t e = sexp_unbox_fx_abs(b); sexp_gc_var2(res, acc); sexp_gc_preserve2(ctx, res, acc); res = sexp_fixnum_to_bignum(ctx, SEXP_ONE); @@ -1390,7 +1390,7 @@ sexp sexp_mul (sexp ctx, sexp a, sexp b) { r = (a==SEXP_ZERO ? a : sexp_make_flonum(ctx, sexp_fixnum_to_double(a)*sexp_flonum_value(b))); break; case SEXP_NUM_FIX_BIG: - r = sexp_bignum_fxmul(ctx, NULL, b, sexp_unbox_fixnum(sexp_fx_abs(a)), 0); + r = sexp_bignum_fxmul(ctx, NULL, b, sexp_unbox_fx_abs(a), 0); sexp_bignum_sign(r) = sexp_fx_sign(a) * sexp_bignum_sign(b); r = sexp_bignum_normalize(r); break; @@ -1473,7 +1473,7 @@ sexp sexp_div (sexp ctx, sexp a, sexp b) { tmp = sexp_make_ratio(ctx, a, b); r = sexp_ratio_normalize(ctx, tmp, SEXP_FALSE); #else - r = sexp_make_flonum(ctx, sexp_fixnum_to_double(a)/sexp_bignum_to_double(b)); + r = sexp_make_flonum(ctx, sexp_fixnum_to_double(a)/sexp_bignum_to_double(b)); #endif break; case SEXP_NUM_FLO_FIX: diff --git a/include/chibi/sexp.h b/include/chibi/sexp.h index 78cbde36..be094ad6 100755 --- a/include/chibi/sexp.h +++ b/include/chibi/sexp.h @@ -1182,6 +1182,8 @@ SEXP_API sexp sexp_symbol_table[SEXP_SYMBOL_TABLE_SIZE]; #define sexp_fx_neg(a) (sexp_make_fixnum(-(sexp_unbox_fixnum(a)))) #define sexp_fx_abs(a) ((((sexp_sint_t)a) < 0) ? sexp_fx_neg(a) : a) +#define sexp_unbox_fx_abs(a) ((((sexp_sint_t)a) < 0) ? -sexp_unbox_fixnum(a) : sexp_unbox_fixnum(a)) + #define sexp_fp_add(x,a,b) (sexp_make_flonum(x, sexp_flonum_value(a) + sexp_flonum_value(b))) #define sexp_fp_sub(x,a,b) (sexp_make_flonum(x, sexp_flonum_value(a) - sexp_flonum_value(b))) #define sexp_fp_mul(x,a,b) (sexp_make_flonum(x, sexp_flonum_value(a) * sexp_flonum_value(b))) diff --git a/tests/numeric-tests.scm b/tests/numeric-tests.scm index 6fc7c06e..526810b5 100644 --- a/tests/numeric-tests.scm +++ b/tests/numeric-tests.scm @@ -143,6 +143,68 @@ -9223372036854775808)) (sign-combinations M7 (+ 1 (expt 2 64)))) +;; fixnum-bignum boundaries (machine word - 1 bit for sign - 2 bits for tag) + +(test 8191 (- -8191)) +(test 8192 (- -8192)) +(test 8193 (- -8193)) + +(test 536870911 (- -536870911)) +(test 536870912 (- -536870912)) +(test 536870913 (- -536870913)) + +(test 2305843009213693951 (- -2305843009213693951)) +(test 2305843009213693952 (- -2305843009213693952)) +(test 2305843009213693953 (- -2305843009213693953)) + +(test 42535295865117307932921825928971026431 (- -42535295865117307932921825928971026431)) +(test 42535295865117307932921825928971026432 (- -42535295865117307932921825928971026432)) +(test 42535295865117307932921825928971026433 (- -42535295865117307932921825928971026433)) + +(test '((536879104 -536862720 4398046511104 0 8192) + (536862720 -536879104 -4398046511104 0 -8192) + (-536862720 536879104 -4398046511104 0 8192) + (-536879104 536862720 4398046511104 0 -8192)) + (sign-combinations (expt 2 13) (expt 2 29))) + +(test '((536879104 536862720 4398046511104 65536 0) + (-536862720 -536879104 -4398046511104 -65536 0) + (536862720 536879104 -4398046511104 -65536 0) + (-536879104 -536862720 4398046511104 65536 0)) + (sign-combinations (expt 2 29) (expt 2 13))) + +(test '((2305843009750564864 -2305843008676823040 1237940039285380274899124224 0 536870912) + (2305843008676823040 -2305843009750564864 -1237940039285380274899124224 0 -536870912) + (-2305843008676823040 2305843009750564864 -1237940039285380274899124224 0 536870912) + (-2305843009750564864 2305843008676823040 1237940039285380274899124224 0 -536870912)) + (sign-combinations (expt 2 29) (expt 2 61))) + +(test '((2305843009750564864 2305843008676823040 1237940039285380274899124224 4294967296 0) + (-2305843008676823040 -2305843009750564864 -1237940039285380274899124224 -4294967296 0) + (2305843008676823040 2305843009750564864 -1237940039285380274899124224 -4294967296 0) + (-2305843009750564864 -2305843008676823040 1237940039285380274899124224 4294967296 0)) + (sign-combinations (expt 2 61) (expt 2 29))) + +(test '((42535295865117307935227668938184720384 -42535295865117307930615982919757332480 + 98079714615416886934934209737619787751599303819750539264 0 2305843009213693952) + (42535295865117307930615982919757332480 -42535295865117307935227668938184720384 + -98079714615416886934934209737619787751599303819750539264 0 -2305843009213693952) + (-42535295865117307930615982919757332480 42535295865117307935227668938184720384 + -98079714615416886934934209737619787751599303819750539264 0 2305843009213693952) + (-42535295865117307935227668938184720384 42535295865117307930615982919757332480 + 98079714615416886934934209737619787751599303819750539264 0 -2305843009213693952)) + (sign-combinations (expt 2 61) (expt 2 125))) + +(test '((42535295865117307935227668938184720384 42535295865117307930615982919757332480 + 98079714615416886934934209737619787751599303819750539264 18446744073709551616 0) + (-42535295865117307930615982919757332480 -42535295865117307935227668938184720384 + -98079714615416886934934209737619787751599303819750539264 -18446744073709551616 0) + (42535295865117307930615982919757332480 42535295865117307935227668938184720384 + -98079714615416886934934209737619787751599303819750539264 -18446744073709551616 0) + (-42535295865117307935227668938184720384 -42535295865117307930615982919757332480 + 98079714615416886934934209737619787751599303819750539264 18446744073709551616 0)) + (sign-combinations (expt 2 125) (expt 2 61))) + (test #f (< +nan.0 +nan.0)) (test #f (<= +nan.0 +nan.0)) (test #f (= +nan.0 +nan.0))