procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix problems with logxor etc. and fixnums

Browse source 
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.
This commit is contained in:
Paul Eggert 2018-08-17 00:25:20 -07:00
parent 3b9017b5ba
commit 64eb9b71da
4 changed files with 23 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
src/alloc.c
View file

@ -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);
}

6
src/data.c
View file

@ -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);
}

13
src/lisp.h
View file

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

14
test/src/data-tests.el
View file

@ -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))