[Emacs-diffs] master 64eb9b7: Fix problems with logxor etc. and fixnums

[Paul Eggert]

branch: master
commit 64eb9b71da7c3c34541929c1b0dfb7f0c11d3d88
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    Fix problems with logxor etc. and fixnums
    
    These operations incorrectly treated negative fixnums as
    bignums greater than most-positive-fixnum.
    * src/alloc.c (mpz_set_intmax_slow): Avoid undefined
    behavior if signed unary negation overflows, while
    we’re in the neighborhood.
    (mpz_set_uintmax_slow): Remove.  All uses removed.
    * src/data.c (arith_driver): Treat fixnums as signed, not
    unsigned, even for logical operations.
    * src/lisp.h (mpz_set_uintmax): Remove.  All uses removed.
    * test/src/data-tests.el (data-tests-logand)
    (data-tests-logior, data-tests-logxor): New tests.
---
 src/alloc.c            | 23 ++++++-----------------
 src/data.c             |  6 +++---
 src/lisp.h             | 13 -------------
 test/src/data-tests.el | 14 ++++++++++++++
 4 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 6a93821..0cd3f0c 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3793,28 +3793,17 @@ mpz_set_intmax_slow (mpz_t result, intmax_t v)
   /* If long is larger then a faster path is taken.  */
   eassert (sizeof (intmax_t) > sizeof (long));
 
-  bool negate = false;
-  if (v < 0)
-    {
-      v = -v;
-      negate = true;
-    }
-  mpz_set_uintmax_slow (result, (uintmax_t) v);
-  if (negate)
-    mpz_neg (result, result);
-}
-
-void
-mpz_set_uintmax_slow (mpz_t result, uintmax_t v)
-{
-  /* If long is larger then a faster path is taken.  */
-  eassert (sizeof (uintmax_t) > sizeof (unsigned long));
+  bool complement = v < 0;
+  if (complement)
+    v = -1 - v;
 
   /* COUNT = 1 means just a single word of the given size.  ORDER = -1
      is arbitrary since there's only a single word.  ENDIAN = 0 means
      use the native endian-ness.  NAILS = 0 means use the whole
      word.  */
-  mpz_import (result, 1, -1, sizeof (uintmax_t), 0, 0, &v);
+  mpz_import (result, 1, -1, sizeof v, 0, 0, &v);
+  if (complement)
+    mpz_com (result, result);
 }
 
 
diff --git a/src/data.c b/src/data.c
index 66f508c..5a355d9 100644
--- a/src/data.c
+++ b/src/data.c
@@ -3006,7 +3006,7 @@ arith_driver (enum arithop code, ptrdiff_t nargs, 
Lisp_Object *args)
            {
              mpz_t tem;
              mpz_init (tem);
-             mpz_set_uintmax (tem, XUFIXNUM (val));
+             mpz_set_intmax (tem, XFIXNUM (val));
              mpz_and (accum, accum, tem);
              mpz_clear (tem);
            }
@@ -3018,7 +3018,7 @@ arith_driver (enum arithop code, ptrdiff_t nargs, 
Lisp_Object *args)
            {
              mpz_t tem;
              mpz_init (tem);
-             mpz_set_uintmax (tem, XUFIXNUM (val));
+             mpz_set_intmax (tem, XFIXNUM (val));
              mpz_ior (accum, accum, tem);
              mpz_clear (tem);
            }
@@ -3030,7 +3030,7 @@ arith_driver (enum arithop code, ptrdiff_t nargs, 
Lisp_Object *args)
            {
              mpz_t tem;
              mpz_init (tem);
-             mpz_set_uintmax (tem, XUFIXNUM (val));
+             mpz_set_intmax (tem, XFIXNUM (val));
              mpz_xor (accum, accum, tem);
              mpz_clear (tem);
            }
diff --git a/src/lisp.h b/src/lisp.h
index da93efd..f2cfe81 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3559,7 +3559,6 @@ extern Lisp_Object listn (enum constype, ptrdiff_t, 
Lisp_Object, ...);
 extern Lisp_Object make_bignum_str (const char *num, int base);
 extern Lisp_Object make_number (mpz_t value);
 extern void mpz_set_intmax_slow (mpz_t result, intmax_t v);
-extern void mpz_set_uintmax_slow (mpz_t result, uintmax_t v);
 
 INLINE void
 mpz_set_intmax (mpz_t result, intmax_t v)
@@ -3573,18 +3572,6 @@ mpz_set_intmax (mpz_t result, intmax_t v)
     mpz_set_si (result, v);
 }
 
-INLINE void
-mpz_set_uintmax (mpz_t result, uintmax_t v)
-{
-  /* mpz_set_ui works in terms of unsigned long, but Emacs may use a
-     wider integer type, and so sometimes will have to construct the
-     mpz_t by hand.  */
-  if (sizeof (uintmax_t) > sizeof (unsigned long) && (unsigned long) v != v)
-    mpz_set_uintmax_slow (result, v);
-  else
-    mpz_set_ui (result, v);
-}
-
 /* Build a frequently used 2/3/4-integer lists.  */
 
 INLINE Lisp_Object
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 8264902..a4c6b0e 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -597,9 +597,23 @@ comparing the subr with a much slower lisp implementation."
   (should (< (1- most-negative-fixnum) most-negative-fixnum))
   (should (fixnump (1- (1+ most-positive-fixnum)))))
 
+(ert-deftest data-tests-logand ()
+  (should (= -1 (logand -1)))
+  (let ((n (* 2 most-negative-fixnum)))
+    (should (= (logand -1 n) n))))
+
 (ert-deftest data-tests-logcount ()
   (should (= (logcount (read "#xffffffffffffffffffffffffffffffff")) 128)))
 
+(ert-deftest data-tests-logior ()
+  (should (= -1 (logior -1)))
+  (should (= -1 (logior most-positive-fixnum most-negative-fixnum))))
+
+(ert-deftest data-tests-logxor ()
+  (should (= -1 (logxor -1)))
+  (let ((n (1+ most-positive-fixnum)))
+    (should (= (logxor -1 n) (lognot n)))))
+
 (ert-deftest data-tests-minmax ()
   (let ((a (- most-negative-fixnum 1))
         (b (+ most-positive-fixnum 1))