[Emacs-diffs] master 39fee20: Be more careful about pointers to bignum vals

[Paul Eggert]

branch: master
commit 39fee209942ab7c35b4789f0010264cd6a52197b
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    Be more careful about pointers to bignum vals
    
    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 bb8e97f..53af732 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 90b1ebe..167b73e 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 9a32ffb..bf7b366 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 cf9f8e5..8344bfd 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 a913aad..0a85df4 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 920adde..6c47b3e 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 326a346..73a50ce 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 3c4c15b..6c9473f 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;
        }
     }