From 967d2c55ef3908fd378e05b2a0070663ae45f6de Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 13 Jun 2018 13:30:29 -0700 Subject: [PATCH] Remove some wrong 8-byte alignment assumptions Do not assume that 8-byte alignment suffices for all C objects, as some platforms require 16-byte alignment for some objects, and this will start to bite us as time goes on (e.g., if an Emacs module ever uses an object containing a long double, which requires 16-byte alignment on x86-64). Conversely, on !USE_LSB_TAG platforms, do not insist on aligning Lisp objects to a multiple of 8, as this is not needed for high-order tag bits. * src/alloc.c (LISP_ALIGNMENT, MALLOC_IS_LISP_ALIGNED): New constants. (XMALLOC_BASE_ALIGNMENT, XMALLOC_HEADER_ALIGNMENT): Removed. All uses replaced by LISP_ALIGNMENT. (aligned_alloc, laligned, lmalloc, lrealloc, union aligned_Lisp_Misc) (maybe_lisp_pointer, pure_alloc): Use LISP_ALIGNMENT rather than GCALIGNMENT. (aligned_alloc): Do not worry about an alignment of LISP_ALIGNMENT when MALLOC_IS_LISP_ALIGNED, as the code never uses aligned_alloc with alignment == LISP_ALIGNMENT in that case. (__alignof__): Remove. All uses removed. (MALLOC_IS_GC_ALIGNED): Remove. All uses replaced with MALLOC_IS_LISP_ALIGNED. (vector_alignment): Remove. All uses replaced with LISP_ALIGNMENT. * src/alloc.c (mark_maybe_pointer): * src/emacs-module.c (value_to_lisp_bits): Do not assume GCALIGNMENT == 1 << GCTYPEBITS, as GCALIGNMENT is 1 on !USE_LSB_TAG platforms now. * src/lisp.h (GCALIGNMENT) [!USE_LSB_TAG]: Now 1. (struct Lisp_Symbol, union vectorlike_header, struct Lisp_Cons) (struct Lisp_String): Simplify test for verifying alignment. --- src/alloc.c | 98 ++++++++++++++++++++++------------------------ src/emacs-module.c | 2 +- src/lisp.h | 29 +++++++------- 3 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index cde2e4b3407..e5fc6ebeb1a 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -633,6 +633,27 @@ buffer_memory_full (ptrdiff_t nbytes) #define COMMON_MULTIPLE(a, b) \ ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) +/* LISP_ALIGNMENT is the alignment of Lisp objects. It must be at + least GCALIGNMENT so that pointers can be tagged. It also must be + at least as strict as the alignment of all the C types used to + implement Lisp objects; since pseudovectors can contain any C type, + this is max_align_t. On recent GNU/Linux x86 and x86-64 this can + often waste up to 8 bytes, since alignof (max_align_t) is 16 but + typical vectors need only an alignment of 8. However, it is not + worth the hassle to avoid this waste. */ +enum { LISP_ALIGNMENT = alignof (union { max_align_t x; GCALIGNED_UNION }) }; +verify (LISP_ALIGNMENT % GCALIGNMENT == 0); + +/* True if malloc (N) is known to return storage suitably aligned for + Lisp objects whenever N is a multiple of LISP_ALIGNMENT. In + practice this is true whenever alignof (max_align_t) is also a + multiple of LISP_ALIGNMENT. This works even for x86, where some + platform combinations (e.g., GCC 7 and later, glibc 2.25 and + earlier) have bugs where alignof (max_align_t) is 16 even though + the malloc alignment is only 8, and where Emacs still works because + it never does anything that requires an alignment of 16. */ +enum { MALLOC_IS_LISP_ALIGNED = alignof (max_align_t) % LISP_ALIGNMENT == 0 }; + #ifndef XMALLOC_OVERRUN_CHECK #define XMALLOC_OVERRUN_CHECK_OVERHEAD 0 #else @@ -653,18 +674,13 @@ buffer_memory_full (ptrdiff_t nbytes) #define XMALLOC_OVERRUN_CHECK_OVERHEAD \ (2 * XMALLOC_OVERRUN_CHECK_SIZE + XMALLOC_OVERRUN_SIZE_SIZE) -#define XMALLOC_BASE_ALIGNMENT alignof (max_align_t) - -#define XMALLOC_HEADER_ALIGNMENT \ - COMMON_MULTIPLE (GCALIGNMENT, XMALLOC_BASE_ALIGNMENT) - /* Define XMALLOC_OVERRUN_SIZE_SIZE so that (1) it's large enough to hold a size_t value and (2) the header size is a multiple of the alignment that Emacs needs for C types and for USE_LSB_TAG. */ #define XMALLOC_OVERRUN_SIZE_SIZE \ (((XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t) \ - + XMALLOC_HEADER_ALIGNMENT - 1) \ - / XMALLOC_HEADER_ALIGNMENT * XMALLOC_HEADER_ALIGNMENT) \ + + LISP_ALIGNMENT - 1) \ + / LISP_ALIGNMENT * LISP_ALIGNMENT) \ - XMALLOC_OVERRUN_CHECK_SIZE) static char const xmalloc_overrun_check_header[XMALLOC_OVERRUN_CHECK_SIZE] = @@ -1165,9 +1181,11 @@ aligned_alloc (size_t alignment, size_t size) Verify this for all arguments this function is given. */ verify (BLOCK_ALIGN % sizeof (void *) == 0 && POWER_OF_2 (BLOCK_ALIGN / sizeof (void *))); - verify (GCALIGNMENT % sizeof (void *) == 0 - && POWER_OF_2 (GCALIGNMENT / sizeof (void *))); - eassert (alignment == BLOCK_ALIGN || alignment == GCALIGNMENT); + verify (MALLOC_IS_LISP_ALIGNED + || (LISP_ALIGNMENT % sizeof (void *) == 0 + && POWER_OF_2 (LISP_ALIGNMENT / sizeof (void *)))); + eassert (alignment == BLOCK_ALIGN + || (!MALLOC_IS_LISP_ALIGNED && alignment == LISP_ALIGNMENT)); void *p; return posix_memalign (&p, alignment, size) == 0 ? p : 0; @@ -1399,31 +1417,15 @@ lisp_align_free (void *block) MALLOC_UNBLOCK_INPUT; } -#if !defined __GNUC__ && !defined __alignof__ -# define __alignof__(type) alignof (type) -#endif - -/* True if malloc (N) is known to return a multiple of GCALIGNMENT - whenever N is also a multiple. In practice this is true if - __alignof__ (max_align_t) is a multiple as well, assuming - GCALIGNMENT is 8; other values of GCALIGNMENT have not been looked - into. Use __alignof__ if available, as otherwise - MALLOC_IS_GC_ALIGNED would be false on GCC x86 even though the - alignment is OK there. - - This is a macro, not an enum constant, for portability to HP-UX - 10.20 cc and AIX 3.2.5 xlc. */ -#define MALLOC_IS_GC_ALIGNED \ - (GCALIGNMENT == 8 && __alignof__ (max_align_t) % GCALIGNMENT == 0) - /* True if a malloc-returned pointer P is suitably aligned for SIZE, - where Lisp alignment may be needed if SIZE is Lisp-aligned. */ + where Lisp object alignment may be needed if SIZE is a multiple of + LISP_ALIGNMENT. */ static bool laligned (void *p, size_t size) { - return (MALLOC_IS_GC_ALIGNED || (intptr_t) p % GCALIGNMENT == 0 - || size % GCALIGNMENT != 0); + return (MALLOC_IS_LISP_ALIGNED || (intptr_t) p % LISP_ALIGNMENT == 0 + || size % LISP_ALIGNMENT != 0); } /* Like malloc and realloc except that if SIZE is Lisp-aligned, make @@ -1446,8 +1448,8 @@ static void * lmalloc (size_t size) { #ifdef USE_ALIGNED_ALLOC - if (! MALLOC_IS_GC_ALIGNED && size % GCALIGNMENT == 0) - return aligned_alloc (GCALIGNMENT, size); + if (! MALLOC_IS_LISP_ALIGNED && size % LISP_ALIGNMENT == 0) + return aligned_alloc (LISP_ALIGNMENT, size); #endif while (true) @@ -1456,7 +1458,7 @@ lmalloc (size_t size) if (laligned (p, size)) return p; free (p); - size_t bigger = size + GCALIGNMENT; + size_t bigger = size + LISP_ALIGNMENT; if (size < bigger) size = bigger; } @@ -1470,7 +1472,7 @@ lrealloc (void *p, size_t size) p = realloc (p, size); if (laligned (p, size)) return p; - size_t bigger = size + GCALIGNMENT; + size_t bigger = size + LISP_ALIGNMENT; if (size < bigger) size = bigger; } @@ -2931,16 +2933,8 @@ set_next_vector (struct Lisp_Vector *v, struct Lisp_Vector *p) #define VECTOR_BLOCK_SIZE 4096 -/* Alignment of struct Lisp_Vector objects. Because pseudovectors - can contain any C type, align at least as strictly as - max_align_t. On x86 and x86-64 this can waste up to 8 bytes - for typical vectors, since alignof (max_align_t) is 16 but - typical vectors need only an alignment of 8. However, it is - not worth the hassle to avoid wasting those bytes. */ -enum {vector_alignment = COMMON_MULTIPLE (alignof (max_align_t), GCALIGNMENT)}; - /* Vector size requests are a multiple of this. */ -enum { roundup_size = COMMON_MULTIPLE (vector_alignment, word_size) }; +enum { roundup_size = COMMON_MULTIPLE (LISP_ALIGNMENT, word_size) }; /* Verify assumptions described above. */ verify (VECTOR_BLOCK_SIZE % roundup_size == 0); @@ -3007,7 +3001,7 @@ struct large_vector enum { - large_vector_offset = ROUNDUP (sizeof (struct large_vector), vector_alignment) + large_vector_offset = ROUNDUP (sizeof (struct large_vector), LISP_ALIGNMENT) }; static struct Lisp_Vector * @@ -3656,8 +3650,8 @@ Its value is void, and its function definition and property list are nil. */) union aligned_Lisp_Misc { union Lisp_Misc m; - unsigned char c[(sizeof (union Lisp_Misc) + GCALIGNMENT - 1) - & -GCALIGNMENT]; + unsigned char c[(sizeof (union Lisp_Misc) + LISP_ALIGNMENT - 1) + & -LISP_ALIGNMENT]; }; /* Allocation of markers and other objects that share that structure. @@ -4851,14 +4845,16 @@ mark_maybe_object (Lisp_Object obj) } } -/* Return true if P can point to Lisp data, and false otherwise. +/* Return true if P might point to Lisp data that can be garbage + collected, and false otherwise (i.e., false if it is easy to see + that P cannot point to Lisp data that can be garbage collected). Symbols are implemented via offsets not pointers, but the offsets - are also multiples of GCALIGNMENT. */ + are also multiples of LISP_ALIGNMENT. */ static bool maybe_lisp_pointer (void *p) { - return (uintptr_t) p % GCALIGNMENT == 0; + return (uintptr_t) p % LISP_ALIGNMENT == 0; } #ifndef HAVE_MODULES @@ -4887,7 +4883,7 @@ mark_maybe_pointer (void *p) { /* For the wide-int case, also mark emacs_value tagged pointers, which can be generated by emacs-module.c's value_to_lisp. */ - p = (void *) ((uintptr_t) p & ~(GCALIGNMENT - 1)); + p = (void *) ((uintptr_t) p & ~((1 << GCTYPEBITS) - 1)); } m = mem_find (p); @@ -5358,7 +5354,7 @@ pure_alloc (size_t size, int type) { /* Allocate space for a Lisp object from the beginning of the free space with taking account of alignment. */ - result = pointer_align (purebeg + pure_bytes_used_lisp, GCALIGNMENT); + result = pointer_align (purebeg + pure_bytes_used_lisp, LISP_ALIGNMENT); pure_bytes_used_lisp = ((char *)result - (char *)purebeg) + size; } else diff --git a/src/emacs-module.c b/src/emacs-module.c index 956706cf9f5..c18c7ab308b 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -924,7 +924,7 @@ value_to_lisp_bits (emacs_value v) makes TAG_PTR faster. */ intptr_t i = (intptr_t) v; - EMACS_UINT tag = i & (GCALIGNMENT - 1); + EMACS_UINT tag = i & ((1 << GCTYPEBITS) - 1); EMACS_UINT untagged = i - tag; switch (tag) { diff --git a/src/lisp.h b/src/lisp.h index d4499846053..aaad90b2dad 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -233,10 +233,6 @@ extern bool suppress_checking EXTERNALLY_VISIBLE; enum Lisp_Bits { - /* 2**GCTYPEBITS. This must be a macro that expands to a literal - integer constant, for older versions of GCC (through at least 4.9). */ -#define GCALIGNMENT 8 - /* Number of bits in a Lisp_Object value, not counting the tag. */ VALBITS = EMACS_INT_WIDTH - GCTYPEBITS, @@ -247,10 +243,6 @@ enum Lisp_Bits FIXNUM_BITS = VALBITS + 1 }; -#if GCALIGNMENT != 1 << GCTYPEBITS -# error "GCALIGNMENT and GCTYPEBITS are inconsistent" -#endif - /* The maximum value that can be stored in a EMACS_INT, assuming all bits other than the type bits contribute to a nonnegative signed value. This can be used in #if, e.g., '#if USE_LSB_TAG' below expands to an @@ -277,12 +269,21 @@ DEFINE_GDB_SYMBOL_END (VALMASK) error !; #endif +/* Minimum alignment requirement for Lisp objects, imposed by the + internal representation of tagged pointers. It is 2**GCTYPEBITS if + USE_LSB_TAG, 1 otherwise. It must be a literal integer constant, + for older versions of GCC (through at least 4.9). */ #if USE_LSB_TAG -# define GCALIGNED_UNION char alignas (GCALIGNMENT) gcaligned; +# define GCALIGNMENT 8 +# if GCALIGNMENT != 1 << GCTYPEBITS +# error "GCALIGNMENT and GCTYPEBITS are inconsistent" +# endif #else -# define GCALIGNED_UNION +# define GCALIGNMENT 1 #endif +#define GCALIGNED_UNION char alignas (GCALIGNMENT) gcaligned; + /* Lisp_Word is a scalar word suitable for holding a tagged pointer or integer. Usually it is a pointer to a deliberately-incomplete type 'union Lisp_X'. However, it is EMACS_INT when Lisp_Objects and @@ -774,7 +775,7 @@ struct Lisp_Symbol GCALIGNED_UNION } u; }; -verify (!USE_LSB_TAG || alignof (struct Lisp_Symbol) % GCALIGNMENT == 0); +verify (alignof (struct Lisp_Symbol) % GCALIGNMENT == 0); /* Declare a Lisp-callable function. The MAXARGS parameter has the same meaning as in the DEFUN macro, and is used to construct a prototype. */ @@ -888,7 +889,7 @@ union vectorlike_header ptrdiff_t size; GCALIGNED_UNION }; -verify (!USE_LSB_TAG || alignof (union vectorlike_header) % GCALIGNMENT == 0); +verify (alignof (union vectorlike_header) % GCALIGNMENT == 0); INLINE bool (SYMBOLP) (Lisp_Object x) @@ -1249,7 +1250,7 @@ struct Lisp_Cons GCALIGNED_UNION } u; }; -verify (!USE_LSB_TAG || alignof (struct Lisp_Cons) % GCALIGNMENT == 0); +verify (alignof (struct Lisp_Cons) % GCALIGNMENT == 0); INLINE bool (NILP) (Lisp_Object x) @@ -1371,7 +1372,7 @@ struct Lisp_String GCALIGNED_UNION } u; }; -verify (!USE_LSB_TAG || alignof (struct Lisp_String) % GCALIGNMENT == 0); +verify (alignof (struct Lisp_String) % GCALIGNMENT == 0); INLINE bool STRINGP (Lisp_Object x) -- 2.39.2