[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
master 0dc5291: Fix aborts due to GC losing pseudovectors
From: |
Paul Eggert |
Subject: |
master 0dc5291: Fix aborts due to GC losing pseudovectors |
Date: |
Mon, 25 May 2020 23:29:54 -0400 (EDT) |
branch: master
commit 0dc529175dc027c1567fb9b7cd529d29236aad44
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>
Fix aborts due to GC losing pseudovectors
Problem reported by Eli Zaretskii (Bug#41321).
* src/alloc.c (MALLOC_ALIGNMENT_BOUND): New constant.
(LISP_ALIGNMENT): Lower it to avoid crashes on MinGW and similarly
buggy platforms where malloc returns pointers not aligned to
alignof (max_align_t). But keep it higher on platforms where this
is known to work, as it helps GC performance.
(MALLOC_IS_LISP_ALIGNED): Define in terms of the other two.
* src/alloc.c (stacktop_sentry):
* src/thread.c (run_thread):
Don’t overalign or oversize stack sentries; they need to be
aligned only for pointers and Lisp_Object, not for arbitrary
pseudovector contents.
* src/lisp.h (union emacs_align_type): New type, used for
LISP_ALIGNMENT.
---
src/alloc.c | 55 +++++++++++++++++++++++++++++--------------------------
src/lisp.h | 43 ++++++++++++++++++++++++++++++++++++++++++-
src/thread.c | 9 +++++++--
3 files changed, 78 insertions(+), 29 deletions(-)
diff --git a/src/alloc.c b/src/alloc.c
index d5a6d91..602282e 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -112,9 +112,9 @@ along with GNU Emacs. If not, see
<https://www.gnu.org/licenses/>. */
adds sizeof (size_t) to SIZE for internal overhead, and then rounds
up to a multiple of MALLOC_ALIGNMENT. Emacs can improve
performance a bit on GNU platforms by arranging for the resulting
- size to be a power of two. This heuristic is good for glibc 2.0
- (1997) through at least glibc 2.31 (2020), and does not affect
- correctness on other platforms. */
+ size to be a power of two. This heuristic is good for glibc 2.26
+ (2017) and later, and does not affect correctness on other
+ platforms. */
#define MALLOC_SIZE_NEAR(n) \
(ROUNDUP (max (n, sizeof (size_t)), MALLOC_ALIGNMENT) - sizeof (size_t))
@@ -655,28 +655,30 @@ 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. Although shrinking
- the alignment to 8 would save memory, it cost a 20% hit to Emacs
- CPU performance on Fedora 28 x86-64 when compiled with gcc -m32. */
-enum { LISP_ALIGNMENT = alignof (union { max_align_t x;
- GCALIGNED_UNION_MEMBER }) };
-verify (LISP_ALIGNMENT % GCALIGNMENT == 0);
+/* 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;
+ 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)) };
/* 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 };
+ Lisp objects whenever N is a multiple of LISP_ALIGNMENT. */
+enum { MALLOC_IS_LISP_ALIGNED = MALLOC_ALIGNMENT_BOUND % LISP_ALIGNMENT == 0 };
/* If compiled with XMALLOC_BLOCK_INPUT_CHECK, define a symbol
BLOCK_INPUT_IN_MEMORY_ALLOCATORS that is visible to the debugger.
@@ -4885,9 +4887,10 @@ test_setjmp (void)
as a stack scan limit. */
typedef union
{
- /* Align the stack top properly. Even if !HAVE___BUILTIN_UNWIND_INIT,
- jmp_buf may not be aligned enough on darwin-ppc64. */
- max_align_t o;
+ /* Make sure stack_top and m_stack_bottom are properly aligned as GC
+ expects. */
+ Lisp_Object o;
+ void *p;
#ifndef HAVE___BUILTIN_UNWIND_INIT
sys_jmp_buf j;
char c;
diff --git a/src/lisp.h b/src/lisp.h
index 85bdc17..937052f 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -277,7 +277,8 @@ 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.
+ generate better code. Also, such structs should be added to the
+ emacs_align_type union.
Although these macros are reasonably portable, they are not
guaranteed on non-GCC platforms, as C11 does not require support
@@ -5059,6 +5060,46 @@ 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 */
diff --git a/src/thread.c b/src/thread.c
index df1a705..b638dd7 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -717,12 +717,17 @@ run_thread (void *state)
{
/* Make sure stack_top and m_stack_bottom are properly aligned as GC
expects. */
- max_align_t stack_pos;
+ union
+ {
+ Lisp_Object o;
+ void *p;
+ char c;
+ } stack_pos;
struct thread_state *self = state;
struct thread_state **iter;
- self->m_stack_bottom = self->stack_top = (char *) &stack_pos;
+ self->m_stack_bottom = self->stack_top = &stack_pos.c;
self->thread_id = sys_thread_self ();
if (self->thread_name)
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- master 0dc5291: Fix aborts due to GC losing pseudovectors,
Paul Eggert <=