unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: John Mastro <john.b.mastro@gmail.com>
Cc: 29183@debbugs.gnu.org
Subject: bug#29183: 27.0.50; SIGSEGV on C-g on Windows
Date: Wed, 8 Nov 2017 20:56:24 -0800	[thread overview]
Message-ID: <a0a4ad80-39cb-3543-03af-3b58a547ba8d@cs.ucla.edu> (raw)
In-Reply-To: <CAOj2CQTKVMwaPr-8J_vaRn5ukvQ5yadxJy9EhQgH4h7CGzqZ=w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 817 bytes --]

John Mastro wrote:
> the only line in the diff between the two resulting
> files that mentions main_thread is that the output from the source with
> GCALIGNED says:
> 
>      .lcomm main_thread,592,8
> 
> Whereas the assembly output from the source without GCALIGNED says:
> 
>      .lcomm main_thread,592,32

Thanks, this helped me see the problem. It turns out that 'struct foo 
__attribute__ ((aligned (8)))' does not work with GCC, and that one must put the 
attribute somewhere else. This appears to be a bug in GCC, and I reported the 
GCC bug here:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82914

The workaround is easy: say 'struct __attribute__ ((aligned (8))) foo' instead. 
I installed the attached patch into the emacs-26 branch and merged it into 
master. Please give it a try.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-GCALIGNED-properly-for-GCC.patch --]
[-- Type: text/x-patch; name="0001-Use-GCALIGNED-properly-for-GCC.patch", Size: 4831 bytes --]

From 9e59de9449b53c3ecd85b624c11360ba9cafee75 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 8 Nov 2017 19:11:18 -0800
Subject: [PATCH] Use GCALIGNED properly for GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Apparently GCC requires that ‘__attribute__ ((aligned (8)))’ must
immediately follow the ‘struct’ keyword when aligning a structure.
The attribute silently does not work if it follows a tag after the
‘struct’ keyword.  Who knew?  Anyway, this patch is designed to
fix a SIGSEGV problem reported by John Mastro (Bug#29183).
* 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):
Put 'GCALIGNED' immediately after 'struct'.
---
 lib-src/make-docfile.c |  2 +-
 src/buffer.c           |  4 ++--
 src/lisp.h             | 16 ++++++++++------
 src/thread.c           |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c
index 0ea3f7b..ff84df9 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 GCALIGNED s;\n"
+	   "  struct GCALIGNED Lisp_Symbol s;\n"
 	   "} lispsym[%td];\n"),
 	  num_symbols);
 }
diff --git a/src/buffer.c b/src/buffer.c
index 15735a2..edeed55 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 GCALIGNED buffer_defaults;
+struct GCALIGNED buffer 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 GCALIGNED buffer_local_symbols;
+struct GCALIGNED buffer 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 4dd4720..0153468 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -277,10 +277,14 @@ DEFINE_GDB_SYMBOL_END (VALMASK)
 error !;
 #endif
 
-/* Declare an object to have an address that is a multiple of
-   GCALIGNMENT.  This is a no-op if the object's natural alignment is
-   already a multiple of GCALIGNMENT.  alignas is not suitable here,
-   as it fails if the object's natural alignment exceeds GCALIGNMENT.  */
+/* Use GCALIGNED immediately after the 'struct' keyword to require the
+   struct to have an address that is a multiple of GCALIGNMENT.  This
+   is a no-op if the struct's natural alignment is already a multiple
+   of GCALIGNMENT.  GCALIGNED's implementation uses the 'aligned'
+   attribute instead of 'alignas (GCALIGNMENT)', as the latter would
+   fail if an object's natural alignment exceeds GCALIGNMENT.  The
+   implementation hopes that natural alignment suffices on platforms
+   lacking 'aligned'.  */
 #ifdef HAVE_STRUCT_ATTRIBUTE_ALIGNED
 # define GCALIGNED __attribute__ ((aligned (GCALIGNMENT)))
 #else
@@ -2944,7 +2948,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 GCALIGNED sname =				\
+   static struct GCALIGNED Lisp_Subr sname =				\
    { { (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS)				\
        | (sizeof (struct Lisp_Subr) / sizeof (EMACS_INT)) },		\
       { (Lisp_Object (__cdecl *)(void))fnname },                        \
@@ -2952,7 +2956,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 GCALIGNED sname =				\
+   static struct GCALIGNED Lisp_Subr 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 03f5b31..7335833 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -26,7 +26,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "coding.h"
 #include "syssignal.h"
 
-static struct thread_state GCALIGNED main_thread;
+static struct GCALIGNED thread_state main_thread;
 
 struct thread_state *current_thread = &main_thread;
 
-- 
2.7.4


  reply	other threads:[~2017-11-09  4:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-06 21:58 bug#29183: 27.0.50; SIGSEGV on C-g on Windows John Mastro
2017-11-07  3:36 ` Eli Zaretskii
2017-11-07 18:14   ` John Mastro
2017-11-07 19:41     ` Eli Zaretskii
2017-11-07 21:24       ` John Mastro
2017-11-08  3:43         ` Eli Zaretskii
2017-11-08 18:35           ` John Mastro
2017-11-08 18:53             ` Eli Zaretskii
2017-11-08 22:46               ` Paul Eggert
2017-11-08 23:41                 ` John Mastro
2017-11-09  4:56                   ` Paul Eggert [this message]
2017-11-09 17:59                     ` John Mastro
2017-11-09 14:59 ` Davor Rotim
2017-11-09 17:04   ` Eli Zaretskii
2017-11-09 18:10     ` Davor Rotim
2017-12-01 10:00   ` Noam Postavsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0a4ad80-39cb-3543-03af-3b58a547ba8d@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=29183@debbugs.gnu.org \
    --cc=john.b.mastro@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).