From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 17 Aug 2018 07:25:20 +0000 (-0700)
Subject: Fix problems with logxor etc. and fixnums
X-Git-Tag: emacs-27.0.90~4553
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=64eb9b71da7c3c34541929c1b0dfb7f0c11d3d88;p=emacs.git

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.
---

diff --git a/src/alloc.c b/src/alloc.c
index 6a938211599..0cd3f0c0c3b 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 66f508c8f43..5a355d9787c 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 da93efdd934..f2cfe81ca75 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 82649022576..a4c6b0e4915 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))