From 39fee209942ab7c35b4789f0010264cd6a52197b Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 21 Aug 2019 00:06:00 -0700 Subject: [PATCH] Be more careful about pointers to bignum vals MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This uses ‘const’ to be better at catching bugs that mistakenly attempt to modify a bignum value. Lisp bignums are supposed to be immutable. * src/alloc.c (make_pure_bignum): * src/fns.c (sxhash_bignum): Accept Lisp_Object instead of struct Lisp_Bignum *, as that’s simpler now. Caller changed. * src/bignum.h (bignum_val, xbignum_val): New inline functions. Prefer them to &i->value and XBIGNUM (i)->value, since they apply ‘const’ to the result. * src/timefns.c (lisp_to_timespec): Use mpz_t const * to point to a bignum value. --- src/alloc.c | 11 ++++++----- src/bignum.c | 10 +++++----- src/bignum.h | 17 +++++++++++++++-- src/data.c | 30 +++++++++++++++--------------- src/floatfns.c | 6 +++--- src/fns.c | 22 +++++++++++----------- src/pdumper.c | 15 +++++++-------- src/timefns.c | 13 +++++++------ 8 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index bb8e97f8737..53af7325f47 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -5290,9 +5290,10 @@ make_pure_float (double num) space. */ static Lisp_Object -make_pure_bignum (struct Lisp_Bignum *value) +make_pure_bignum (Lisp_Object value) { - size_t i, nlimbs = mpz_size (value->value); + mpz_t const *n = xbignum_val (value); + size_t i, nlimbs = mpz_size (*n); size_t nbytes = nlimbs * sizeof (mp_limb_t); mp_limb_t *pure_limbs; mp_size_t new_size; @@ -5303,10 +5304,10 @@ make_pure_bignum (struct Lisp_Bignum *value) int limb_alignment = alignof (mp_limb_t); pure_limbs = pure_alloc (nbytes, - limb_alignment); for (i = 0; i < nlimbs; ++i) - pure_limbs[i] = mpz_getlimbn (value->value, i); + pure_limbs[i] = mpz_getlimbn (*n, i); new_size = nlimbs; - if (mpz_sgn (value->value) < 0) + if (mpz_sgn (*n) < 0) new_size = -new_size; mpz_roinit_n (b->value, pure_limbs, new_size); @@ -5456,7 +5457,7 @@ purecopy (Lisp_Object obj) return obj; } else if (BIGNUMP (obj)) - obj = make_pure_bignum (XBIGNUM (obj)); + obj = make_pure_bignum (obj); else { AUTO_STRING (fmt, "Don't know how to purify: %S"); diff --git a/src/bignum.c b/src/bignum.c index 90b1ebea876..167b73eee02 100644 --- a/src/bignum.c +++ b/src/bignum.c @@ -63,7 +63,7 @@ init_bignum (void) double bignum_to_double (Lisp_Object n) { - return mpz_get_d_rounded (XBIGNUM (n)->value); + return mpz_get_d_rounded (*xbignum_val (n)); } /* Return D, converted to a Lisp integer. Discard any fraction. @@ -264,13 +264,13 @@ intmax_t bignum_to_intmax (Lisp_Object x) { intmax_t i; - return mpz_to_intmax (XBIGNUM (x)->value, &i) ? i : 0; + return mpz_to_intmax (*xbignum_val (x), &i) ? i : 0; } uintmax_t bignum_to_uintmax (Lisp_Object x) { uintmax_t i; - return mpz_to_uintmax (XBIGNUM (x)->value, &i) ? i : 0; + return mpz_to_uintmax (*xbignum_val (x), &i) ? i : 0; } /* Yield an upper bound on the buffer size needed to contain a C @@ -284,7 +284,7 @@ mpz_bufsize (mpz_t const num, int base) ptrdiff_t bignum_bufsize (Lisp_Object num, int base) { - return mpz_bufsize (XBIGNUM (num)->value, base); + return mpz_bufsize (*xbignum_val (num), base); } /* Convert NUM to a nearest double, as opposed to mpz_get_d which @@ -318,7 +318,7 @@ ptrdiff_t bignum_to_c_string (char *buf, ptrdiff_t size, Lisp_Object num, int base) { eassert (bignum_bufsize (num, abs (base)) == size); - mpz_get_str (buf, base, XBIGNUM (num)->value); + mpz_get_str (buf, base, *xbignum_val (num)); ptrdiff_t n = size - 2; return !buf[n - 1] ? n - 1 : n + !!buf[n]; } diff --git a/src/bignum.h b/src/bignum.h index 9a32ffb0374..bf7b3669537 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -80,6 +80,19 @@ mpz_set_uintmax (mpz_t result, uintmax_t v) mpz_set_uintmax_slow (result, v); } +/* Return a pointer to the mpz_t value represented by the bignum I. + It is const because the value should not change. */ +INLINE mpz_t const * +bignum_val (struct Lisp_Bignum const *i) +{ + return &i->value; +} +INLINE mpz_t const * +xbignum_val (Lisp_Object i) +{ + return bignum_val (XBIGNUM (i)); +} + /* Return a pointer to an mpz_t that is equal to the Lisp integer I. If I is a bignum this returns a pointer to I's representation; otherwise this sets *TMP to I's value and returns TMP. */ @@ -91,7 +104,7 @@ bignum_integer (mpz_t *tmp, Lisp_Object i) mpz_set_intmax (*tmp, XFIXNUM (i)); return tmp; } - return &XBIGNUM (i)->value; + return xbignum_val (i); } /* Set RESULT to the value stored in the Lisp integer I. If I is a @@ -103,7 +116,7 @@ mpz_set_integer (mpz_t result, Lisp_Object i) if (FIXNUMP (i)) mpz_set_intmax (result, XFIXNUM (i)); else - mpz_set (result, XBIGNUM (i)->value); + mpz_set (result, *xbignum_val (i)); } INLINE_HEADER_END diff --git a/src/data.c b/src/data.c index cf9f8e56133..8344bfdd3d5 100644 --- a/src/data.c +++ b/src/data.c @@ -525,7 +525,7 @@ DEFUN ("natnump", Fnatnump, Snatnump, 1, 1, 0, (Lisp_Object object) { return ((FIXNUMP (object) ? 0 <= XFIXNUM (object) - : BIGNUMP (object) && 0 <= mpz_sgn (XBIGNUM (object)->value)) + : BIGNUMP (object) && 0 <= mpz_sgn (*xbignum_val (object))) ? Qt : Qnil); } @@ -2481,7 +2481,7 @@ arithcompare (Lisp_Object num1, Lisp_Object num2, else if (isnan (f1)) lt = eq = gt = false; else - i2 = mpz_cmp_d (XBIGNUM (num2)->value, f1); + i2 = mpz_cmp_d (*xbignum_val (num2), f1); } else if (FIXNUMP (num1)) { @@ -2502,7 +2502,7 @@ arithcompare (Lisp_Object num1, Lisp_Object num2, i2 = XFIXNUM (num2); } else - i2 = mpz_sgn (XBIGNUM (num2)->value); + i2 = mpz_sgn (*xbignum_val (num2)); } else if (FLOATP (num2)) { @@ -2510,12 +2510,12 @@ arithcompare (Lisp_Object num1, Lisp_Object num2, if (isnan (f2)) lt = eq = gt = false; else - i1 = mpz_cmp_d (XBIGNUM (num1)->value, f2); + i1 = mpz_cmp_d (*xbignum_val (num1), f2); } else if (FIXNUMP (num2)) - i1 = mpz_sgn (XBIGNUM (num1)->value); + i1 = mpz_sgn (*xbignum_val (num1)); else - i1 = mpz_cmp (XBIGNUM (num1)->value, XBIGNUM (num2)->value); + i1 = mpz_cmp (*xbignum_val (num1), *xbignum_val (num2)); if (eq) { @@ -3005,7 +3005,7 @@ usage: (- &optional NUMBER-OR-MARKER &rest MORE-NUMBERS-OR-MARKERS) */) return make_int (-XFIXNUM (a)); if (FLOATP (a)) return make_float (-XFLOAT_DATA (a)); - mpz_neg (mpz[0], XBIGNUM (a)->value); + mpz_neg (mpz[0], *xbignum_val (a)); return make_integer_mpz (); } return arith_driver (Asub, nargs, args, a); @@ -3214,7 +3214,7 @@ representation. */) if (BIGNUMP (value)) { - mpz_t *nonneg = &XBIGNUM (value)->value; + mpz_t const *nonneg = xbignum_val (value); if (mpz_sgn (*nonneg) < 0) { mpz_com (mpz[0], *nonneg); @@ -3245,10 +3245,10 @@ In this case, the sign bit is duplicated. */) { if (EQ (value, make_fixnum (0))) return value; - if (mpz_sgn (XBIGNUM (count)->value) < 0) + if (mpz_sgn (*xbignum_val (count)) < 0) { EMACS_INT v = (FIXNUMP (value) ? XFIXNUM (value) - : mpz_sgn (XBIGNUM (value)->value)); + : mpz_sgn (*xbignum_val (value))); return make_fixnum (v < 0 ? -1 : 0); } overflow_error (); @@ -3291,8 +3291,8 @@ expt_integer (Lisp_Object x, Lisp_Object y) if (TYPE_RANGED_FIXNUMP (unsigned long, y)) exp = XFIXNUM (y); else if (MOST_POSITIVE_FIXNUM < ULONG_MAX && BIGNUMP (y) - && mpz_fits_ulong_p (XBIGNUM (y)->value)) - exp = mpz_get_ui (XBIGNUM (y)->value); + && mpz_fits_ulong_p (*xbignum_val (y))) + exp = mpz_get_ui (*xbignum_val (y)); else overflow_error (); @@ -3311,7 +3311,7 @@ Markers are converted to integers. */) return make_int (XFIXNUM (number) + 1); if (FLOATP (number)) return (make_float (1.0 + XFLOAT_DATA (number))); - mpz_add_ui (mpz[0], XBIGNUM (number)->value, 1); + mpz_add_ui (mpz[0], *xbignum_val (number), 1); return make_integer_mpz (); } @@ -3326,7 +3326,7 @@ Markers are converted to integers. */) return make_int (XFIXNUM (number) - 1); if (FLOATP (number)) return (make_float (-1.0 + XFLOAT_DATA (number))); - mpz_sub_ui (mpz[0], XBIGNUM (number)->value, 1); + mpz_sub_ui (mpz[0], *xbignum_val (number), 1); return make_integer_mpz (); } @@ -3337,7 +3337,7 @@ DEFUN ("lognot", Flognot, Slognot, 1, 1, 0, CHECK_INTEGER (number); if (FIXNUMP (number)) return make_fixnum (~XFIXNUM (number)); - mpz_com (mpz[0], XBIGNUM (number)->value); + mpz_com (mpz[0], *xbignum_val (number)); return make_integer_mpz (); } diff --git a/src/floatfns.c b/src/floatfns.c index a913aad5aac..0a85df47dec 100644 --- a/src/floatfns.c +++ b/src/floatfns.c @@ -268,9 +268,9 @@ DEFUN ("abs", Fabs, Sabs, 1, 1, 0, } else { - if (mpz_sgn (XBIGNUM (arg)->value) < 0) + if (mpz_sgn (*xbignum_val (arg)) < 0) { - mpz_neg (mpz[0], XBIGNUM (arg)->value); + mpz_neg (mpz[0], *xbignum_val (arg)); arg = make_integer_mpz (); } } @@ -315,7 +315,7 @@ This is the same as the exponent of a float. */) value = ivalue - 1; } else if (!FIXNUMP (arg)) - value = mpz_sizeinbase (XBIGNUM (arg)->value, 2) - 1; + value = mpz_sizeinbase (*xbignum_val (arg), 2) - 1; else { EMACS_INT i = XFIXNUM (arg); diff --git a/src/fns.c b/src/fns.c index 920addeaf13..6c47b3e5b1c 100644 --- a/src/fns.c +++ b/src/fns.c @@ -47,7 +47,6 @@ static void sort_vector_copy (Lisp_Object, ptrdiff_t, enum equal_kind { EQUAL_NO_QUIT, EQUAL_PLAIN, EQUAL_INCLUDING_PROPERTIES }; static bool internal_equal (Lisp_Object, Lisp_Object, enum equal_kind, int, Lisp_Object); -static EMACS_UINT sxhash_bignum (struct Lisp_Bignum *); DEFUN ("identity", Fidentity, Sidentity, 1, 1, 0, doc: /* Return the argument unchanged. */ @@ -1444,7 +1443,7 @@ DEFUN ("nthcdr", Fnthcdr, Snthcdr, 2, 2, 0, } else { - if (mpz_sgn (XBIGNUM (n)->value) < 0) + if (mpz_sgn (*xbignum_val (n)) < 0) return tail; num = large_num; } @@ -1482,11 +1481,11 @@ DEFUN ("nthcdr", Fnthcdr, Snthcdr, 2, 2, 0, CYCLE_LENGTH. */ /* Add N mod CYCLE_LENGTH to NUM. */ if (cycle_length <= ULONG_MAX) - num += mpz_tdiv_ui (XBIGNUM (n)->value, cycle_length); + num += mpz_tdiv_ui (*xbignum_val (n), cycle_length); else { mpz_set_intmax (mpz[0], cycle_length); - mpz_tdiv_r (mpz[0], XBIGNUM (n)->value, mpz[0]); + mpz_tdiv_r (mpz[0], *xbignum_val (n), mpz[0]); intptr_t iz; mpz_export (&iz, NULL, -1, sizeof iz, 0, 0, mpz[0]); num += iz; @@ -1595,7 +1594,7 @@ The value is actually the tail of LIST whose car is ELT. */) { Lisp_Object tem = XCAR (tail); if (BIGNUMP (tem) - && mpz_cmp (XBIGNUM (elt)->value, XBIGNUM (tem)->value) == 0) + && mpz_cmp (*xbignum_val (elt), *xbignum_val (tem)) == 0) return tail; } } @@ -2307,7 +2306,7 @@ This differs from numeric comparison: (eql 0.0 -0.0) returns nil and return FLOATP (obj2) && same_float (obj1, obj2) ? Qt : Qnil; else if (BIGNUMP (obj1)) return ((BIGNUMP (obj2) - && mpz_cmp (XBIGNUM (obj1)->value, XBIGNUM (obj2)->value) == 0) + && mpz_cmp (*xbignum_val (obj1), *xbignum_val (obj2)) == 0) ? Qt : Qnil); else return EQ (obj1, obj2) ? Qt : Qnil; @@ -2437,7 +2436,7 @@ internal_equal (Lisp_Object o1, Lisp_Object o2, enum equal_kind equal_kind, if (ASIZE (o2) != size) return false; if (BIGNUMP (o1)) - return mpz_cmp (XBIGNUM (o1)->value, XBIGNUM (o2)->value) == 0; + return mpz_cmp (*xbignum_val (o1), *xbignum_val (o2)) == 0; if (OVERLAYP (o1)) { if (!internal_equal (OVERLAY_START (o1), OVERLAY_START (o2), @@ -4640,13 +4639,14 @@ sxhash_bool_vector (Lisp_Object vec) /* Return a hash for a bignum. */ static EMACS_UINT -sxhash_bignum (struct Lisp_Bignum *bignum) +sxhash_bignum (Lisp_Object bignum) { - size_t i, nlimbs = mpz_size (bignum->value); + mpz_t const *n = xbignum_val (bignum); + size_t i, nlimbs = mpz_size (*n); EMACS_UINT hash = 0; for (i = 0; i < nlimbs; ++i) - hash = sxhash_combine (hash, mpz_getlimbn (bignum->value, i)); + hash = sxhash_combine (hash, mpz_getlimbn (*n, i)); return SXHASH_REDUCE (hash); } @@ -4680,7 +4680,7 @@ sxhash (Lisp_Object obj, int depth) /* This can be everything from a vector to an overlay. */ case Lisp_Vectorlike: if (BIGNUMP (obj)) - hash = sxhash_bignum (XBIGNUM (obj)); + hash = sxhash_bignum (obj); else if (VECTORP (obj) || RECORDP (obj)) /* According to the CL HyperSpec, two arrays are equal only if they are `eq', except for strings and bit-vectors. In diff --git a/src/pdumper.c b/src/pdumper.c index 326a346a632..73a50cee53a 100644 --- a/src/pdumper.c +++ b/src/pdumper.c @@ -2211,7 +2211,7 @@ dump_bignum (struct dump_context *ctx, Lisp_Object object) const struct Lisp_Bignum *bignum = XBIGNUM (object); START_DUMP_PVEC (ctx, &bignum->header, struct Lisp_Bignum, out); verify (sizeof (out->value) >= sizeof (struct bignum_reload_info)); - dump_field_fixup_later (ctx, out, bignum, &bignum->value); + dump_field_fixup_later (ctx, out, bignum, xbignum_val (object)); dump_off bignum_offset = finish_dump_pvec (ctx, &out->header); if (ctx->flags.dump_object_contents) { @@ -3397,19 +3397,18 @@ dump_cold_buffer (struct dump_context *ctx, Lisp_Object data) static void dump_cold_bignum (struct dump_context *ctx, Lisp_Object object) { - const struct Lisp_Bignum *bignum = XBIGNUM (object); - size_t sz_nlimbs = mpz_size (bignum->value); + mpz_t const *n = xbignum_val (object); + size_t sz_nlimbs = mpz_size (*n); eassert (sz_nlimbs < DUMP_OFF_MAX); dump_align_output (ctx, alignof (mp_limb_t)); dump_off nlimbs = (dump_off) sz_nlimbs; Lisp_Object descriptor = list2 (dump_off_to_lisp (ctx->offset), - dump_off_to_lisp ((mpz_sgn (bignum->value) < 0 - ? -nlimbs : nlimbs))); + dump_off_to_lisp (mpz_sgn (*n) < 0 ? -nlimbs : nlimbs)); Fputhash (object, descriptor, ctx->bignum_data); for (mp_size_t i = 0; i < nlimbs; ++i) { - mp_limb_t limb = mpz_getlimbn (bignum->value, i); + mp_limb_t limb = mpz_getlimbn (*n, i); dump_write (ctx, &limb, sizeof (limb)); } } @@ -5205,8 +5204,8 @@ dump_do_dump_relocation (const uintptr_t dump_base, { struct Lisp_Bignum *bignum = dump_ptr (dump_base, reloc_offset); struct bignum_reload_info reload_info; - verify (sizeof (reload_info) <= sizeof (bignum->value)); - memcpy (&reload_info, &bignum->value, sizeof (reload_info)); + verify (sizeof (reload_info) <= sizeof (*bignum_val (bignum))); + memcpy (&reload_info, bignum_val (bignum), sizeof (reload_info)); const mp_limb_t *limbs = dump_ptr (dump_base, reload_info.data_location); mpz_roinit_n (bignum->value, limbs, reload_info.nlimbs); diff --git a/src/timefns.c b/src/timefns.c index 3c4c15b6576..6c9473f22a6 100644 --- a/src/timefns.c +++ b/src/timefns.c @@ -91,7 +91,7 @@ static Lisp_Object timespec_hz; #define TRILLION 1000000000000 #if FIXNUM_OVERFLOW_P (TRILLION) static Lisp_Object trillion; -# define ztrillion (XBIGNUM (trillion)->value) +# define ztrillion (*xbignum_val (trillion)) #else # define trillion make_fixnum (TRILLION) # if ULONG_MAX < TRILLION || !FASTER_TIMEFNS @@ -534,7 +534,7 @@ lisp_time_hz_ticks (struct lisp_time t, Lisp_Object hz) return make_int (ticks / XFIXNUM (t.hz) - (ticks % XFIXNUM (t.hz) < 0)); } - else if (! (BIGNUMP (hz) && 0 < mpz_sgn (XBIGNUM (hz)->value))) + else if (! (BIGNUMP (hz) && 0 < mpz_sgn (*xbignum_val (hz)))) invalid_hz (hz); mpz_mul (mpz[0], @@ -906,6 +906,7 @@ lisp_to_timespec (struct lisp_time t) struct timespec result = invalid_timespec (); int ns; mpz_t *q = &mpz[0]; + mpz_t const *qt = q; if (FASTER_TIMEFNS && EQ (t.hz, timespec_hz)) { @@ -924,7 +925,7 @@ lisp_to_timespec (struct lisp_time t) return result; } else - ns = mpz_fdiv_q_ui (*q, XBIGNUM (t.ticks)->value, TIMESPEC_HZ); + ns = mpz_fdiv_q_ui (*q, *xbignum_val (t.ticks), TIMESPEC_HZ); } else if (FASTER_TIMEFNS && EQ (t.hz, make_fixnum (1))) { @@ -941,7 +942,7 @@ lisp_to_timespec (struct lisp_time t) return result; } else - q = &XBIGNUM (t.ticks)->value; + qt = xbignum_val (t.ticks); } else { @@ -953,7 +954,7 @@ lisp_to_timespec (struct lisp_time t) /* With some versions of MinGW, tv_sec is a 64-bit type, whereas time_t is a 32-bit type. */ time_t sec; - if (mpz_time (*q, &sec)) + if (mpz_time (*qt, &sec)) { result.tv_sec = sec; result.tv_nsec = ns; @@ -1038,7 +1039,7 @@ lispint_arith (Lisp_Object a, Lisp_Object b, bool subtract) if (eabs (XFIXNUM (b)) <= ULONG_MAX) { ((XFIXNUM (b) < 0) == subtract ? mpz_add_ui : mpz_sub_ui) - (mpz[0], XBIGNUM (a)->value, eabs (XFIXNUM (b))); + (mpz[0], *xbignum_val (a), eabs (XFIXNUM (b))); mpz_done = true; } } -- 2.39.2