From 6ab1b16c54b63d1c4217b6fcea026400029fcc72 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 27 Apr 2014 18:29:44 -0700 Subject: [PATCH] Avoid undefined behavior in signed left shift. This ports to GCC 4.9.0 with -fsanitize=undefined. * alloc.c (bool_vector_fill, SETMARKBIT, UNSETMARKBIT): * data.c (Fash): * regex.c (extract_number): * lisp.h (make_number, XINT): Do not shift a 1 bit left into a sign bit. * alloc.c (struct cons_block, struct float_block): Use unsigned, not int, for gcmarkbits. All uses changed. --- src/ChangeLog | 12 ++++++++++++ src/alloc.c | 18 +++++++++--------- src/data.c | 2 +- src/lisp.h | 19 ++++++++++++++++--- src/regex.c | 3 ++- 5 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 9d4fd2d6d3..90223d6d10 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,15 @@ +2014-04-28 Paul Eggert + + Avoid undefined behavior in signed left shift. + This ports to GCC 4.9.0 with -fsanitize=undefined. + * alloc.c (bool_vector_fill, SETMARKBIT, UNSETMARKBIT): + * data.c (Fash): + * regex.c (extract_number): + * lisp.h (make_number, XINT): + Do not shift a 1 bit left into a sign bit. + * alloc.c (struct cons_block, struct float_block): Use unsigned, + not int, for gcmarkbits. All uses changed. + 2014-04-25 Eli Zaretskii * search.c (Fnewline_cache_check): Don't try to count newlines diff --git a/src/alloc.c b/src/alloc.c index 6bee0c990c..32d3333cea 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -2131,7 +2131,7 @@ bool_vector_fill (Lisp_Object a, Lisp_Object init) unsigned char *data = bool_vector_uchar_data (a); int pattern = NILP (init) ? 0 : (1 << BOOL_VECTOR_BITS_PER_CHAR) - 1; ptrdiff_t nbytes = bool_vector_bytes (nbits); - int last_mask = ~ (~0 << ((nbits - 1) % BOOL_VECTOR_BITS_PER_CHAR + 1)); + int last_mask = ~ (~0u << ((nbits - 1) % BOOL_VECTOR_BITS_PER_CHAR + 1)); memset (data, pattern, nbytes - 1); data[nbytes - 1] = pattern & last_mask; } @@ -2336,17 +2336,17 @@ make_formatted_string (char *buf, const char *format, ...) / (sizeof (struct Lisp_Float) * CHAR_BIT + 1)) #define GETMARKBIT(block,n) \ - (((block)->gcmarkbits[(n) / (sizeof (int) * CHAR_BIT)] \ - >> ((n) % (sizeof (int) * CHAR_BIT))) \ + (((block)->gcmarkbits[(n) / (sizeof (unsigned) * CHAR_BIT)] \ + >> ((n) % (sizeof (unsigned) * CHAR_BIT))) \ & 1) #define SETMARKBIT(block,n) \ - (block)->gcmarkbits[(n) / (sizeof (int) * CHAR_BIT)] \ - |= 1 << ((n) % (sizeof (int) * CHAR_BIT)) + ((block)->gcmarkbits[(n) / (sizeof (unsigned) * CHAR_BIT)] \ + |= 1u << ((n) % (sizeof (unsigned) * CHAR_BIT))) #define UNSETMARKBIT(block,n) \ - (block)->gcmarkbits[(n) / (sizeof (int) * CHAR_BIT)] \ - &= ~(1 << ((n) % (sizeof (int) * CHAR_BIT))) + ((block)->gcmarkbits[(n) / (sizeof (unsigned) * CHAR_BIT)] \ + &= ~(1u << ((n) % (sizeof (unsigned) * CHAR_BIT)))) #define FLOAT_BLOCK(fptr) \ ((struct float_block *) (((uintptr_t) (fptr)) & ~(BLOCK_ALIGN - 1))) @@ -2358,7 +2358,7 @@ struct float_block { /* Place `floats' at the beginning, to ease up FLOAT_INDEX's job. */ struct Lisp_Float floats[FLOAT_BLOCK_SIZE]; - int gcmarkbits[1 + FLOAT_BLOCK_SIZE / (sizeof (int) * CHAR_BIT)]; + unsigned gcmarkbits[1 + FLOAT_BLOCK_SIZE / (sizeof (unsigned) * CHAR_BIT)]; struct float_block *next; }; @@ -2452,7 +2452,7 @@ struct cons_block { /* Place `conses' at the beginning, to ease up CONS_INDEX's job. */ struct Lisp_Cons conses[CONS_BLOCK_SIZE]; - int gcmarkbits[1 + CONS_BLOCK_SIZE / (sizeof (int) * CHAR_BIT)]; + unsigned gcmarkbits[1 + CONS_BLOCK_SIZE / (sizeof (unsigned) * CHAR_BIT)]; struct cons_block *next; }; diff --git a/src/data.c b/src/data.c index 965ddd215f..bf863aaed7 100644 --- a/src/data.c +++ b/src/data.c @@ -2895,7 +2895,7 @@ In this case, the sign bit is duplicated. */) if (XINT (count) >= BITS_PER_EMACS_INT) XSETINT (val, 0); else if (XINT (count) > 0) - XSETINT (val, XINT (value) << XFASTINT (count)); + XSETINT (val, XUINT (value) << XFASTINT (count)); else if (XINT (count) <= -BITS_PER_EMACS_INT) XSETINT (val, XINT (value) < 0 ? -1 : 0); else diff --git a/src/lisp.h b/src/lisp.h index 4c310f6966..5e0f141cc9 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -354,7 +354,8 @@ error !; # define lisp_h_check_cons_list() ((void) 0) #endif #if USE_LSB_TAG -# define lisp_h_make_number(n) XIL ((EMACS_INT) (n) << INTTYPEBITS) +# define lisp_h_make_number(n) \ + XIL ((EMACS_INT) ((EMACS_UINT) (n) << INTTYPEBITS)) # define lisp_h_XFASTINT(a) XINT (a) # define lisp_h_XINT(a) (XLI (a) >> INTTYPEBITS) # define lisp_h_XTYPE(a) ((enum Lisp_Type) (XLI (a) & ~VALMASK)) @@ -665,7 +666,14 @@ LISP_MACRO_DEFUN (XUNTAG, void *, (Lisp_Object a, int type), (a, type)) INLINE Lisp_Object make_number (EMACS_INT n) { - return XIL (USE_LSB_TAG ? n << INTTYPEBITS : n & INTMASK); + if (USE_LSB_TAG) + { + EMACS_UINT u = n; + n = u << INTTYPEBITS; + } + else + n &= INTMASK; + return XIL (n); } /* Extract A's value as a signed integer. */ @@ -673,7 +681,12 @@ INLINE EMACS_INT XINT (Lisp_Object a) { EMACS_INT i = XLI (a); - return (USE_LSB_TAG ? i : i << INTTYPEBITS) >> INTTYPEBITS; + if (! USE_LSB_TAG) + { + EMACS_UINT u = i; + i = u << INTTYPEBITS; + } + return i >> INTTYPEBITS; } /* Like XINT (A), but may be faster. A must be nonnegative. diff --git a/src/regex.c b/src/regex.c index 244924058a..622db708b9 100644 --- a/src/regex.c +++ b/src/regex.c @@ -713,7 +713,8 @@ typedef enum static int extract_number (re_char *source) { - return (SIGN_EXTEND_CHAR (source[1]) << 8) + source[0]; + unsigned leading_byte = SIGN_EXTEND_CHAR (source[1]); + return (leading_byte << 8) + source[0]; } /* Same as EXTRACT_NUMBER, except increment SOURCE to after the number. -- 2.20.1