Subject: Re: bug#29040: Emacs 25 hangs on windows arbitrarily during search of a unicode file

From: Paul Eggert <[email-masked]>
Date: Thu, 2 Nov 2017 13:35:16 -0700

On 11/02/2017 08:50 AM, Eli Zaretskii wrote:
> do you see a cleaner fix?

Yes, we can stop using alignas entirely, since it doesn't work the way I 
expected. I thought that it could only increase alignment, and that it 
was a no-op if it specified a decreased alignment: this is how 
__attribute__ ((aligned (8))) works. However, I now see that C11 says 
that a compiler is supposed to report an error if alignas specifies a 
decreased alignment. So I installed the attached patch to stop using 
alignas.

> We've stopped supporting MSVC long ago.
OK. Can we then simplify the source a little bit, in the 'master' 
branch, as a low-priority task? I was thinking of something like this:

* Remove my_endbss_static in lastfile.c, since we no longer need to 
worry about the Alpha MSVC linker.

* Remove the the _MSC_VER-specific code in lisp.h's definitions of 
ENUM_BF and DEFUN and in regex.c's definition of re_char and const_re_char.

>From f8e2d438bfd4923b30ff878e5834e1d8e4061397 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
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 <https://www.gnu.org/licenses/>.  */
 #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