From 3abf76da564ff8526bbcba6b92dfb7b97cb87779 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 25 May 2020 22:06:25 -0700 Subject: [PATCH] Refix aborts due to GC losing pseudovectors MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This is simpler, and fixes a bug in the previous fix. * src/alloc.c (MALLOC_ALIGNMENT_BOUND): Simplify by using max_align_t, since the buggy implementations won’t break this simpler implementation. (LISP_ALIGNMENT): Simplify by just using GCALIGNMENT, since the fancier implementation wasn’t correct anyway, and fixing it isn’t worth the trouble on practical platforms. * src/lisp.h (union emacs_align_type): Remove. --- src/alloc.c | 32 +++++++++++++------------------- src/lisp.h | 43 +------------------------------------------ 2 files changed, 14 insertions(+), 61 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 602282e5704..89fe96a2349 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -655,26 +655,20 @@ buffer_memory_full (ptrdiff_t nbytes) #define COMMON_MULTIPLE(a, b) \ ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) -/* A lower bound on the alignment of malloc. For better performance - this bound should be tighter. For glibc 2.26 and later a tighter - bound is known. */ -#if 2 < __GLIBC__ + (26 <= __GLIBC_MINOR__) -enum { MALLOC_ALIGNMENT_BOUND = MALLOC_ALIGNMENT }; -#else -/* A bound known to work for all Emacs porting targets. Tightening - this looser bound by using max_align_t instead of long long int - would break buggy malloc implementations like MinGW circa 2020. */ -enum { MALLOC_ALIGNMENT_BOUND = alignof (long long int) }; -#endif - -/* A lower bound on the alignment of Lisp objects. All Lisp objects - must have an address that is a multiple of LISP_ALIGNMENT; +/* A lower bound on the alignment of malloc. Although this bound is + incorrect for some buggy malloc implementations (e.g., MinGW circa + 2020), the bugs should not matter for the way this bound is used + since the correct bound is also a multiple of LISP_ALIGNMENT on the + buggy platforms. */ +enum { MALLOC_ALIGNMENT_BOUND = alignof (max_align_t) }; + +/* A lower bound on the alignment of Lisp objects allocated on the heap. + All such objects must have an address that is a multiple of LISP_ALIGNMENT; otherwise maybe_lisp_pointer can issue false negatives, causing crashes. - It's good to make this bound tight: if Lisp objects are always - aligned more strictly than LISP_ALIGNMENT, maybe_lisp_pointer will - issue more false positives, hurting performance. */ -enum { LISP_ALIGNMENT = max (max (GCALIGNMENT, MALLOC_ALIGNMENT_BOUND), - alignof (union emacs_align_type)) }; + On all practical Emacs targets, sizeof (struct Lisp_Float) == 8 and + since GCALIGNMENT also equals 8 there's little point to optimizing + for impractical targets. */ +enum { LISP_ALIGNMENT = GCALIGNMENT }; /* True if malloc (N) is known to return storage suitably aligned for Lisp objects whenever N is a multiple of LISP_ALIGNMENT. */ diff --git a/src/lisp.h b/src/lisp.h index 937052f6df8..85bdc172b20 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -277,8 +277,7 @@ error !; allocation in a containing union that has GCALIGNED_UNION_MEMBER) and does not contain a GC-aligned struct or union, putting GCALIGNED_STRUCT after its closing '}' can help the compiler - generate better code. Also, such structs should be added to the - emacs_align_type union. + generate better code. Although these macros are reasonably portable, they are not guaranteed on non-GCC platforms, as C11 does not require support @@ -5060,46 +5059,6 @@ maybe_gc (void) maybe_garbage_collect (); } -/* A type with alignment at least as large as any object that Emacs - allocates. This is not max_align_t because some platforms (e.g., - mingw) have buggy malloc implementations that do not align for - max_align_t. This union contains types of all GCALIGNED_STRUCT - components visible here. */ -union emacs_align_type -{ - struct Lisp_Bool_Vector Lisp_Bool_Vector; - struct Lisp_Char_Table Lisp_Char_Table; - struct Lisp_CondVar Lisp_CondVar; - struct Lisp_Finalizer Lisp_Finalizer; - struct Lisp_Float Lisp_Float; - struct Lisp_Hash_Table Lisp_Hash_Table; - struct Lisp_Marker Lisp_Marker; - struct Lisp_Misc_Ptr Lisp_Misc_Ptr; - struct Lisp_Mutex Lisp_Mutex; - struct Lisp_Overlay Lisp_Overlay; - struct Lisp_Sub_Char_Table Lisp_Sub_Char_Table; - struct Lisp_Subr Lisp_Subr; - struct Lisp_User_Ptr Lisp_User_Ptr; - struct Lisp_Vector Lisp_Vector; - struct thread_state thread_state; - - /* Omit the following since they would require including bignum.h, - frame.h etc., and in practice their alignments never exceed that - of the structs already listed. */ -#if 0 - struct frame frame; - struct Lisp_Bignum Lisp_Bignum; - struct Lisp_Module_Function Lisp_Module_Function; - struct Lisp_Process Lisp_Process; - struct save_window_data save_window_data; - struct scroll_bar scroll_bar; - struct terminal terminal; - struct window window; - struct xwidget xwidget; - struct xwidget_view xwidget_view; -#endif -}; - INLINE_HEADER_END #endif /* EMACS_LISP_H */ -- 2.39.5