>From 2bc416753ea9dbdeb27719957076ceb909886bd1 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 24 Jun 2015 20:10:03 -0700 Subject: [PATCH] Fix GC bugs --with-wide-int and Qnil == 0 Use the same alignment for the !USE_LSB_TAG case as for the more-typical USE_LSB_TAG case. The attempt to support arbitrary alignments with !USE_LSB_TAG had subtle bugs in garbage collection once we changed the representation of symbols so that Qnil == 0. Problem reported by Eli Zaretskii (Bug#20862). * src/alloc.c (XMALLOC_HEADER_ALIGNMENT) [XMALLOC_OVERRUN_CHECK]: * src/alloc.c (vector_alignment, union aligned_Lisp_Symbol) (union aligned_Lisp_Misc, maybe_lisp_pointer, pure_alloc): Use same alignment for !USE_LSB_TAG as for USE_LSB_TAG. * src/alloc.c (POINTERS_MIGHT_HIDE_IN_OBJECTS): Remove. This optimization in the !USE_LSB_TAG case is no longer valid when symbols are represented via offsets. Change the only use to assume that pointers might hide in objects. * src/lisp.h (alignas) [!USE_LSB_TAG]: Require support in this case, too. (TAG_SYMOFFSET, XSYMBOL) [!USE_LSB_TAG]: Do not shift the offset. This is OK, because the !USE_LSB_TAG case now applies only when Lisp_Object is wider than void *, so there's no longer any need to shift the offset. Not shifting the offset means that symbol representations have the same alignment as pointers, which the GC assumes. --- src/alloc.c | 59 ++++++++++------------------------------------------------- src/lisp.h | 11 ++--------- 2 files changed, 12 insertions(+), 58 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index a956e95..c9bdcc2 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -528,12 +528,8 @@ buffer_memory_full (ptrdiff_t nbytes) alignment that Emacs needs for C types and for USE_LSB_TAG. */ #define XMALLOC_BASE_ALIGNMENT alignof (max_align_t) -#if USE_LSB_TAG -# define XMALLOC_HEADER_ALIGNMENT \ - COMMON_MULTIPLE (GCALIGNMENT, XMALLOC_BASE_ALIGNMENT) -#else -# define XMALLOC_HEADER_ALIGNMENT XMALLOC_BASE_ALIGNMENT -#endif +#define XMALLOC_HEADER_ALIGNMENT \ + COMMON_MULTIPLE (GCALIGNMENT, XMALLOC_BASE_ALIGNMENT) #define XMALLOC_OVERRUN_SIZE_SIZE \ (((XMALLOC_OVERRUN_CHECK_SIZE + sizeof (size_t) \ + XMALLOC_HEADER_ALIGNMENT - 1) \ @@ -2730,7 +2726,7 @@ enum { /* Alignment of struct Lisp_Vector objects. */ vector_alignment = COMMON_MULTIPLE (ALIGNOF_STRUCT_LISP_VECTOR, - USE_LSB_TAG ? GCALIGNMENT : 1), + GCALIGNMENT), /* Vector size requests are a multiple of this. */ roundup_size = COMMON_MULTIPLE (vector_alignment, word_size) @@ -3299,15 +3295,13 @@ usage: (make-byte-code ARGLIST BYTE-CODE CONSTANTS DEPTH &optional DOCSTRING INT ***********************************************************************/ /* Like struct Lisp_Symbol, but padded so that the size is a multiple - of the required alignment if LSB tags are used. */ + of the required alignment. */ union aligned_Lisp_Symbol { struct Lisp_Symbol s; -#if USE_LSB_TAG unsigned char c[(sizeof (struct Lisp_Symbol) + GCALIGNMENT - 1) & -GCALIGNMENT]; -#endif }; /* Each symbol_block is just under 1020 bytes long, since malloc @@ -3411,15 +3405,13 @@ Its value is void, and its function definition and property list are nil. */) ***********************************************************************/ /* Like union Lisp_Misc, but padded so that its size is a multiple of - the required alignment when LSB tags are used. */ + the required alignment. */ union aligned_Lisp_Misc { union Lisp_Misc m; -#if USE_LSB_TAG unsigned char c[(sizeof (union Lisp_Misc) + GCALIGNMENT - 1) & -GCALIGNMENT]; -#endif }; /* Allocation of markers and other objects that share that structure. @@ -4628,13 +4620,13 @@ mark_maybe_object (Lisp_Object obj) } /* Return true if P can point to Lisp data, and false otherwise. - USE_LSB_TAG needs Lisp data to be aligned on multiples of GCALIGNMENT. - Otherwise, assume that Lisp data is aligned on even addresses. */ + Symbols are implemented via offsets not pointers, but the offsets + are also multiples of GCALIGNMENT. */ static bool maybe_lisp_pointer (void *p) { - return !((intptr_t) p % (USE_LSB_TAG ? GCALIGNMENT : 2)); + return (uintptr_t) p % GCALIGNMENT == 0; } /* If P points to Lisp data, mark that as live if it isn't already @@ -4722,27 +4714,6 @@ mark_maybe_pointer (void *p) miss objects if __alignof__ were used. */ #define GC_POINTER_ALIGNMENT alignof (void *) -/* Define POINTERS_MIGHT_HIDE_IN_OBJECTS to 1 if marking via C pointers does - not suffice, which is the typical case. A host where a Lisp_Object is - wider than a pointer might allocate a Lisp_Object in non-adjacent halves. - If USE_LSB_TAG, the bottom half is not a valid pointer, but it should - suffice to widen it to to a Lisp_Object and check it that way. */ -#if USE_LSB_TAG || VAL_MAX < UINTPTR_MAX -# if !USE_LSB_TAG && VAL_MAX < UINTPTR_MAX >> GCTYPEBITS - /* If tag bits straddle pointer-word boundaries, neither mark_maybe_pointer - nor mark_maybe_object can follow the pointers. This should not occur on - any practical porting target. */ -# error "MSB type bits straddle pointer-word boundaries" -# endif - /* Marking via C pointers does not suffice, because Lisp_Objects contain - pointer words that hold pointers ORed with type bits. */ -# define POINTERS_MIGHT_HIDE_IN_OBJECTS 1 -#else - /* Marking via C pointers suffices, because Lisp_Objects contain pointer - words that hold unmodified pointers. */ -# define POINTERS_MIGHT_HIDE_IN_OBJECTS 0 -#endif - /* Mark Lisp objects referenced from the address range START+OFFSET..END or END+OFFSET..START. */ @@ -4788,8 +4759,7 @@ mark_memory (void *start, void *end) { void *p = *(void **) ((char *) pp + i); mark_maybe_pointer (p); - if (POINTERS_MIGHT_HIDE_IN_OBJECTS) - mark_maybe_object (XIL ((intptr_t) p)); + mark_maybe_object (XIL ((intptr_t) p)); } } @@ -5148,22 +5118,13 @@ static void * pure_alloc (size_t size, int type) { void *result; -#if USE_LSB_TAG - size_t alignment = GCALIGNMENT; -#else - size_t alignment = alignof (EMACS_INT); - - /* Give Lisp_Floats an extra alignment. */ - if (type == Lisp_Float) - alignment = alignof (struct Lisp_Float); -#endif again: if (type >= 0) { /* Allocate space for a Lisp object from the beginning of the free space with taking account of alignment. */ - result = ALIGN (purebeg + pure_bytes_used_lisp, alignment); + result = ALIGN (purebeg + pure_bytes_used_lisp, GCALIGNMENT); pure_bytes_used_lisp = ((char *)result - (char *)purebeg) + size; } else diff --git a/src/lisp.h b/src/lisp.h index 198f116..c3289c9 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -278,10 +278,7 @@ error !; #endif #ifndef alignas -# define alignas(alignment) /* empty */ -# if USE_LSB_TAG -# error "USE_LSB_TAG requires alignas" -# endif +# error "alignas not defined" #endif #ifdef HAVE_STRUCT_ATTRIBUTE_ALIGNED @@ -731,9 +728,7 @@ struct Lisp_Symbol /* Yield an integer that contains a symbol tag along with OFFSET. OFFSET should be the offset in bytes from 'lispsym' to the symbol. */ -#define TAG_SYMOFFSET(offset) \ - TAG_PTR (Lisp_Symbol, \ - ((uintptr_t) (offset) >> (USE_LSB_TAG ? 0 : GCTYPEBITS))) +#define TAG_SYMOFFSET(offset) TAG_PTR (Lisp_Symbol, offset) /* XLI_BUILTIN_LISPSYM (iQwhatever) is equivalent to XLI (builtin_lisp_symbol (Qwhatever)), @@ -899,8 +894,6 @@ INLINE struct Lisp_Symbol * XSYMBOL (Lisp_Object a) { uintptr_t i = (uintptr_t) XUNTAG (a, Lisp_Symbol); - if (! USE_LSB_TAG) - i <<= GCTYPEBITS; void *p = (char *) lispsym + i; return p; } -- 2.1.0