From f8e2d438bfd4923b30ff878e5834e1d8e4061397 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 2 Nov 2017 13:06:38 -0700 Subject: [PATCH] Fix alignment portability problems Do not assume that the natural alignment of Lisp objects is a multiple of GCALIGNMENT. This improves on the portability of the recent fix for Bug#29040. * lib-src/make-docfile.c (close_emacs_globals): * src/buffer.c (buffer_defaults, buffer_local_symbols): * src/lisp.h (DEFUN): * src/thread.c (main_thread): Use GCALIGNED, not alignas (GCALIGNMENT). * src/alloc.c (COMMON_MULTIPLE): Move back here from lisp.h, since it is no longer used elsewhere. * src/lisp.h (GCALIGNMENT): No longer a macro, since we need not worry about MSVC. Omit no-longer-needed consistency check. * src/thread.c (THREAD_ALIGNMENT): Remove. --- lib-src/make-docfile.c | 2 +- src/alloc.c | 6 ++++++ src/buffer.c | 4 ++-- src/lisp.h | 25 ++++++++----------------- src/thread.c | 4 +--- 5 files changed, 18 insertions(+), 23 deletions(-) diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c index 69c7f37a17..0ea3f7b6b6 100644 --- a/lib-src/make-docfile.c +++ b/lib-src/make-docfile.c @@ -668,7 +668,7 @@ close_emacs_globals (ptrdiff_t num_symbols) "extern\n" "#endif\n" "struct {\n" - " struct Lisp_Symbol alignas (GCALIGNMENT) s;\n" + " struct Lisp_Symbol GCALIGNED s;\n" "} lispsym[%td];\n"), num_symbols); } diff --git a/src/alloc.c b/src/alloc.c index ff93956109..0fc79fe68a 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -621,6 +621,12 @@ buffer_memory_full (ptrdiff_t nbytes) #endif } +/* A common multiple of the positive integers A and B. Ideally this + would be the least common multiple, but there's no way to do that + as a constant expression in C, so do the best that we can easily do. */ +#define COMMON_MULTIPLE(a, b) \ + ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) + #ifndef XMALLOC_OVERRUN_CHECK #define XMALLOC_OVERRUN_CHECK_OVERHEAD 0 #else diff --git a/src/buffer.c b/src/buffer.c index 9635733fcf..15735a298a 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -61,7 +61,7 @@ struct buffer *all_buffers; Setting the default value also goes through the alist of buffers and stores into each buffer that does not say it has a local value. */ -struct buffer alignas (GCALIGNMENT) buffer_defaults; +struct buffer GCALIGNED buffer_defaults; /* This structure marks which slots in a buffer have corresponding default values in buffer_defaults. @@ -84,7 +84,7 @@ struct buffer buffer_local_flags; /* This structure holds the names of symbols whose values may be buffer-local. It is indexed and accessed in the same way as the above. */ -struct buffer alignas (GCALIGNMENT) buffer_local_symbols; +struct buffer GCALIGNED buffer_local_symbols; /* Return the symbol of the per-buffer variable at offset OFFSET in the buffer structure. */ diff --git a/src/lisp.h b/src/lisp.h index 43b3ec618f..bf9db591bd 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -228,14 +228,12 @@ extern bool suppress_checking EXTERNALLY_VISIBLE; USE_LSB_TAG not only requires the least 3 bits of pointers returned by malloc to be 0 but also needs to be able to impose a mult-of-8 alignment - on the few static Lisp_Objects used: lispsym, all the defsubr, and - the two special buffers buffer_defaults and buffer_local_symbols. */ + on the few static Lisp_Objects used, all of which are aligned via + the GCALIGN macro defined below. */ enum Lisp_Bits { - /* 2**GCTYPEBITS. This must be a macro that expands to a literal - integer constant, for MSVC. */ -#define GCALIGNMENT 8 + GCALIGNMENT = 1 << GCTYPEBITS, /* Number of bits in a Lisp_Object value, not counting the tag. */ VALBITS = EMACS_INT_WIDTH - GCTYPEBITS, @@ -247,10 +245,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,18 +271,15 @@ DEFINE_GDB_SYMBOL_END (VALMASK) error !; #endif +/* Declare an object to have an address that is a multiple of + GCALIGNMENT. alignas is not suitable here, as it fails if the + object's natural alignment exceeds GCALIGNMENT. */ #ifdef HAVE_STRUCT_ATTRIBUTE_ALIGNED # define GCALIGNED __attribute__ ((aligned (GCALIGNMENT))) #else # define GCALIGNED /* empty */ #endif -/* A common multiple of the positive integers A and B. Ideally this - would be the least common multiple, but there's no way to do that - as a constant expression in C, so do the best that we can easily do. */ -#define COMMON_MULTIPLE(a, b) \ - ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) - /* Some operations are so commonly executed that they are implemented as macros, not functions, because otherwise runtime performance would suffer too much when compiling with GCC without optimization. @@ -2946,7 +2937,7 @@ CHECK_NUMBER_CDR (Lisp_Object x) #ifdef _MSC_VER #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc) \ Lisp_Object fnname DEFUN_ARGS_ ## maxargs ; \ - static struct Lisp_Subr alignas (GCALIGNMENT) sname = \ + static struct Lisp_Subr GCALIGNED sname = \ { { (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS) \ | (sizeof (struct Lisp_Subr) / sizeof (EMACS_INT)) }, \ { (Lisp_Object (__cdecl *)(void))fnname }, \ @@ -2954,7 +2945,7 @@ CHECK_NUMBER_CDR (Lisp_Object x) Lisp_Object fnname #else /* not _MSC_VER */ #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc) \ - static struct Lisp_Subr alignas (GCALIGNMENT) sname = \ + static struct Lisp_Subr GCALIGNED sname = \ { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS }, \ { .a ## maxargs = fnname }, \ minargs, maxargs, lname, intspec, 0}; \ diff --git a/src/thread.c b/src/thread.c index 7a670ba410..03f5b31855 100644 --- a/src/thread.c +++ b/src/thread.c @@ -26,9 +26,7 @@ along with GNU Emacs. If not, see . */ #include "coding.h" #include "syssignal.h" -#define THREAD_ALIGNMENT COMMON_MULTIPLE (alignof (max_align_t), GCALIGNMENT) - -static struct thread_state alignas (THREAD_ALIGNMENT) main_thread; +static struct thread_state GCALIGNED main_thread; struct thread_state *current_thread = &main_thread; -- 2.13.6