emacs-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

master 3abf76d: Refix aborts due to GC losing pseudovectors


From: Paul Eggert
Subject: master 3abf76d: Refix aborts due to GC losing pseudovectors
Date: Tue, 26 May 2020 01:06:49 -0400 (EDT)

branch: master
commit 3abf76da564ff8526bbcba6b92dfb7b97cb87779
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    Refix aborts due to GC losing pseudovectors
    
    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 602282e..89fe96a 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 937052f..85bdc17 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 */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]