diff options
| author | Paul Eggert | 2018-10-19 09:06:52 -0700 |
|---|---|---|
| committer | Paul Eggert | 2018-10-19 09:26:50 -0700 |
| commit | d2a07b9a82a632e8baa179c667a98d275e5f6973 (patch) | |
| tree | aa9ba321dc84936722b5345f67bf089678840bc6 /src | |
| parent | fc3f93705543408b868feb7b93b8d77ab1c6ae53 (diff) | |
| download | emacs-d2a07b9a82a632e8baa179c667a98d275e5f6973.tar.gz emacs-d2a07b9a82a632e8baa179c667a98d275e5f6973.zip | |
Fix struct thread alignment on FreeBSD x86
Problem reported by Joseph Mingrone in:
https://lists.gnu.org/r/emacs-devel/2018-10/msg00238.html
While we’re at it, apply a similar fix to struct Lisp_Subr; this
removes the need for GCALIGNED_STRUCT_MEMBER and thus can shrink
struct Lisp_Subr a bit.
* configure.ac (HAVE_STRUCT_ATTRIBUTE_ALIGNED): Bring back this macro.
Although used only for performance (not to actually align
structures), we might as well take advantage of it.
* src/lisp.h (GCALIGNED_STRUCT_MEMBER): Remove; all uses removed.
(union Aligned_Lisp_Subr): New type, like struct Lisp_Subr but aligned.
* src/lisp.h (XSUBR, DEFUN):
* src/lread.c (defsubr): Use it. All callers changed.
* src/thread.c (union aligned_thread_state): New type.
(main_thread): Now of this type, so it’s aligned.
All uses changed.
* src/xmenu.c (syms_of_xmenu) [USE_GTK || USE_X_TOOLKIT]:
Adjust to union Aligned_Lisp_Subr change.
Diffstat (limited to 'src')
| -rw-r--r-- | src/lisp.h | 37 | ||||
| -rw-r--r-- | src/lread.c | 3 | ||||
| -rw-r--r-- | src/thread.c | 47 | ||||
| -rw-r--r-- | src/xmenu.c | 2 |
4 files changed, 49 insertions, 40 deletions
diff --git a/src/lisp.h b/src/lisp.h index 145901dff5e..eb6762678c7 100644 --- a/src/lisp.h +++ b/src/lisp.h | |||
| @@ -229,7 +229,7 @@ extern bool suppress_checking EXTERNALLY_VISIBLE; | |||
| 229 | USE_LSB_TAG not only requires the least 3 bits of pointers returned by | 229 | USE_LSB_TAG not only requires the least 3 bits of pointers returned by |
| 230 | malloc to be 0 but also needs to be able to impose a mult-of-8 alignment | 230 | malloc to be 0 but also needs to be able to impose a mult-of-8 alignment |
| 231 | on some non-GC Lisp_Objects, all of which are aligned via | 231 | on some non-GC Lisp_Objects, all of which are aligned via |
| 232 | GCALIGNED_UNION_MEMBER, GCALIGNED_STRUCT_MEMBER, and GCALIGNED_STRUCT. */ | 232 | GCALIGNED_UNION_MEMBER. */ |
| 233 | 233 | ||
| 234 | enum Lisp_Bits | 234 | enum Lisp_Bits |
| 235 | { | 235 | { |
| @@ -284,15 +284,14 @@ error !; | |||
| 284 | # define GCALIGNMENT 1 | 284 | # define GCALIGNMENT 1 |
| 285 | #endif | 285 | #endif |
| 286 | 286 | ||
| 287 | /* If a struct is always allocated by the GC and is therefore always | 287 | /* To cause a union to have alignment of at least GCALIGNMENT, put |
| 288 | GC-aligned, put GCALIGNED_STRUCT after its closing '}'; this can | 288 | GCALIGNED_UNION_MEMBER in its member list. |
| 289 | help the compiler generate better code. | ||
| 290 | 289 | ||
| 291 | To cause a union to have alignment of at least GCALIGNMENT, put | 290 | If a struct is always GC-aligned (either by the GC, or via |
| 292 | GCALIGNED_UNION_MEMBER in its member list. Similarly for a struct | 291 | allocation in a containing union that has GCALIGNED_UNION_MEMBER) |
| 293 | and GCALIGNED_STRUCT_MEMBER, although this may make the struct a | 292 | and does not contain a GC-aligned struct or union, putting |
| 294 | bit bigger on non-GCC platforms. Any struct using | 293 | GCALIGNED_STRUCT after its closing '}' can help the compiler |
| 295 | GCALIGNED_STRUCT_MEMBER should also use GCALIGNED_STRUCT. | 294 | generate better code. |
| 296 | 295 | ||
| 297 | Although these macros are reasonably portable, they are not | 296 | Although these macros are reasonably portable, they are not |
| 298 | guaranteed on non-GCC platforms, as C11 does not require support | 297 | guaranteed on non-GCC platforms, as C11 does not require support |
| @@ -306,10 +305,8 @@ error !; | |||
| 306 | 305 | ||
| 307 | #define GCALIGNED_UNION_MEMBER char alignas (GCALIGNMENT) gcaligned; | 306 | #define GCALIGNED_UNION_MEMBER char alignas (GCALIGNMENT) gcaligned; |
| 308 | #if HAVE_STRUCT_ATTRIBUTE_ALIGNED | 307 | #if HAVE_STRUCT_ATTRIBUTE_ALIGNED |
| 309 | # define GCALIGNED_STRUCT_MEMBER | ||
| 310 | # define GCALIGNED_STRUCT __attribute__ ((aligned (GCALIGNMENT))) | 308 | # define GCALIGNED_STRUCT __attribute__ ((aligned (GCALIGNMENT))) |
| 311 | #else | 309 | #else |
| 312 | # define GCALIGNED_STRUCT_MEMBER GCALIGNED_UNION_MEMBER | ||
| 313 | # define GCALIGNED_STRUCT | 310 | # define GCALIGNED_STRUCT |
| 314 | #endif | 311 | #endif |
| 315 | #define GCALIGNED(type) (alignof (type) % GCALIGNMENT == 0) | 312 | #define GCALIGNED(type) (alignof (type) % GCALIGNMENT == 0) |
| @@ -1970,9 +1967,13 @@ struct Lisp_Subr | |||
| 1970 | const char *symbol_name; | 1967 | const char *symbol_name; |
| 1971 | const char *intspec; | 1968 | const char *intspec; |
| 1972 | EMACS_INT doc; | 1969 | EMACS_INT doc; |
| 1973 | GCALIGNED_STRUCT_MEMBER | ||
| 1974 | } GCALIGNED_STRUCT; | 1970 | } GCALIGNED_STRUCT; |
| 1975 | verify (GCALIGNED (struct Lisp_Subr)); | 1971 | union Aligned_Lisp_Subr |
| 1972 | { | ||
| 1973 | struct Lisp_Subr s; | ||
| 1974 | GCALIGNED_UNION_MEMBER | ||
| 1975 | }; | ||
| 1976 | verify (GCALIGNED (union Aligned_Lisp_Subr)); | ||
| 1976 | 1977 | ||
| 1977 | INLINE bool | 1978 | INLINE bool |
| 1978 | SUBRP (Lisp_Object a) | 1979 | SUBRP (Lisp_Object a) |
| @@ -1984,7 +1985,7 @@ INLINE struct Lisp_Subr * | |||
| 1984 | XSUBR (Lisp_Object a) | 1985 | XSUBR (Lisp_Object a) |
| 1985 | { | 1986 | { |
| 1986 | eassert (SUBRP (a)); | 1987 | eassert (SUBRP (a)); |
| 1987 | return XUNTAG (a, Lisp_Vectorlike, struct Lisp_Subr); | 1988 | return &XUNTAG (a, Lisp_Vectorlike, union Aligned_Lisp_Subr)->s; |
| 1988 | } | 1989 | } |
| 1989 | 1990 | ||
| 1990 | enum char_table_specials | 1991 | enum char_table_specials |
| @@ -2952,15 +2953,15 @@ CHECK_FIXNUM_CDR (Lisp_Object x) | |||
| 2952 | /* This version of DEFUN declares a function prototype with the right | 2953 | /* This version of DEFUN declares a function prototype with the right |
| 2953 | arguments, so we can catch errors with maxargs at compile-time. */ | 2954 | arguments, so we can catch errors with maxargs at compile-time. */ |
| 2954 | #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc) \ | 2955 | #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc) \ |
| 2955 | static struct Lisp_Subr sname = \ | 2956 | static union Aligned_Lisp_Subr sname = \ |
| 2956 | { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS }, \ | 2957 | {{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS }, \ |
| 2957 | { .a ## maxargs = fnname }, \ | 2958 | { .a ## maxargs = fnname }, \ |
| 2958 | minargs, maxargs, lname, intspec, 0}; \ | 2959 | minargs, maxargs, lname, intspec, 0}}; \ |
| 2959 | Lisp_Object fnname | 2960 | Lisp_Object fnname |
| 2960 | 2961 | ||
| 2961 | /* defsubr (Sname); | 2962 | /* defsubr (Sname); |
| 2962 | is how we define the symbol for function `name' at start-up time. */ | 2963 | is how we define the symbol for function `name' at start-up time. */ |
| 2963 | extern void defsubr (struct Lisp_Subr *); | 2964 | extern void defsubr (union Aligned_Lisp_Subr *); |
| 2964 | 2965 | ||
| 2965 | enum maxargs | 2966 | enum maxargs |
| 2966 | { | 2967 | { |
diff --git a/src/lread.c b/src/lread.c index 62616cb6819..5f3871436df 100644 --- a/src/lread.c +++ b/src/lread.c | |||
| @@ -4409,8 +4409,9 @@ init_obarray (void) | |||
| 4409 | } | 4409 | } |
| 4410 | 4410 | ||
| 4411 | void | 4411 | void |
| 4412 | defsubr (struct Lisp_Subr *sname) | 4412 | defsubr (union Aligned_Lisp_Subr *aname) |
| 4413 | { | 4413 | { |
| 4414 | struct Lisp_Subr *sname = &aname->s; | ||
| 4414 | Lisp_Object sym, tem; | 4415 | Lisp_Object sym, tem; |
| 4415 | sym = intern_c_string (sname->symbol_name); | 4416 | sym = intern_c_string (sname->symbol_name); |
| 4416 | XSETPVECTYPE (sname, PVEC_SUBR); | 4417 | XSETPVECTYPE (sname, PVEC_SUBR); |
diff --git a/src/thread.c b/src/thread.c index 3674af0e47b..6612697b95e 100644 --- a/src/thread.c +++ b/src/thread.c | |||
| @@ -27,11 +27,18 @@ along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ | |||
| 27 | #include "syssignal.h" | 27 | #include "syssignal.h" |
| 28 | #include "keyboard.h" | 28 | #include "keyboard.h" |
| 29 | 29 | ||
| 30 | static struct thread_state main_thread; | 30 | union aligned_thread_state |
| 31 | { | ||
| 32 | struct thread_state s; | ||
| 33 | GCALIGNED_UNION_MEMBER | ||
| 34 | }; | ||
| 35 | verify (GCALIGNED (union aligned_thread_state)); | ||
| 36 | |||
| 37 | static union aligned_thread_state main_thread; | ||
| 31 | 38 | ||
| 32 | struct thread_state *current_thread = &main_thread; | 39 | struct thread_state *current_thread = &main_thread.s; |
| 33 | 40 | ||
| 34 | static struct thread_state *all_threads = &main_thread; | 41 | static struct thread_state *all_threads = &main_thread.s; |
| 35 | 42 | ||
| 36 | static sys_mutex_t global_lock; | 43 | static sys_mutex_t global_lock; |
| 37 | 44 | ||
| @@ -113,7 +120,7 @@ maybe_reacquire_global_lock (void) | |||
| 113 | /* SIGINT handler is always run on the main thread, see | 120 | /* SIGINT handler is always run on the main thread, see |
| 114 | deliver_process_signal, so reflect that in our thread-tracking | 121 | deliver_process_signal, so reflect that in our thread-tracking |
| 115 | variables. */ | 122 | variables. */ |
| 116 | current_thread = &main_thread; | 123 | current_thread = &main_thread.s; |
| 117 | 124 | ||
| 118 | if (current_thread->not_holding_lock) | 125 | if (current_thread->not_holding_lock) |
| 119 | { | 126 | { |
| @@ -659,7 +666,7 @@ mark_threads (void) | |||
| 659 | void | 666 | void |
| 660 | unmark_main_thread (void) | 667 | unmark_main_thread (void) |
| 661 | { | 668 | { |
| 662 | main_thread.header.size &= ~ARRAY_MARK_FLAG; | 669 | main_thread.s.header.size &= ~ARRAY_MARK_FLAG; |
| 663 | } | 670 | } |
| 664 | 671 | ||
| 665 | 672 | ||
| @@ -1043,23 +1050,23 @@ thread_check_current_buffer (struct buffer *buffer) | |||
| 1043 | static void | 1050 | static void |
| 1044 | init_main_thread (void) | 1051 | init_main_thread (void) |
| 1045 | { | 1052 | { |
| 1046 | main_thread.header.size | 1053 | main_thread.s.header.size |
| 1047 | = PSEUDOVECSIZE (struct thread_state, m_stack_bottom); | 1054 | = PSEUDOVECSIZE (struct thread_state, m_stack_bottom); |
| 1048 | XSETPVECTYPE (&main_thread, PVEC_THREAD); | 1055 | XSETPVECTYPE (&main_thread.s, PVEC_THREAD); |
| 1049 | main_thread.m_last_thing_searched = Qnil; | 1056 | main_thread.s.m_last_thing_searched = Qnil; |
| 1050 | main_thread.m_saved_last_thing_searched = Qnil; | 1057 | main_thread.s.m_saved_last_thing_searched = Qnil; |
| 1051 | main_thread.name = Qnil; | 1058 | main_thread.s.name = Qnil; |
| 1052 | main_thread.function = Qnil; | 1059 | main_thread.s.function = Qnil; |
| 1053 | main_thread.result = Qnil; | 1060 | main_thread.s.result = Qnil; |
| 1054 | main_thread.error_symbol = Qnil; | 1061 | main_thread.s.error_symbol = Qnil; |
| 1055 | main_thread.error_data = Qnil; | 1062 | main_thread.s.error_data = Qnil; |
| 1056 | main_thread.event_object = Qnil; | 1063 | main_thread.s.event_object = Qnil; |
| 1057 | } | 1064 | } |
| 1058 | 1065 | ||
| 1059 | bool | 1066 | bool |
| 1060 | main_thread_p (void *ptr) | 1067 | main_thread_p (void *ptr) |
| 1061 | { | 1068 | { |
| 1062 | return ptr == &main_thread; | 1069 | return ptr == &main_thread.s; |
| 1063 | } | 1070 | } |
| 1064 | 1071 | ||
| 1065 | bool | 1072 | bool |
| @@ -1080,11 +1087,11 @@ void | |||
| 1080 | init_threads (void) | 1087 | init_threads (void) |
| 1081 | { | 1088 | { |
| 1082 | init_main_thread (); | 1089 | init_main_thread (); |
| 1083 | sys_cond_init (&main_thread.thread_condvar); | 1090 | sys_cond_init (&main_thread.s.thread_condvar); |
| 1084 | sys_mutex_init (&global_lock); | 1091 | sys_mutex_init (&global_lock); |
| 1085 | sys_mutex_lock (&global_lock); | 1092 | sys_mutex_lock (&global_lock); |
| 1086 | current_thread = &main_thread; | 1093 | current_thread = &main_thread.s; |
| 1087 | main_thread.thread_id = sys_thread_self (); | 1094 | main_thread.s.thread_id = sys_thread_self (); |
| 1088 | } | 1095 | } |
| 1089 | 1096 | ||
| 1090 | void | 1097 | void |
| @@ -1130,7 +1137,7 @@ syms_of_threads (void) | |||
| 1130 | DEFVAR_LISP ("main-thread", Vmain_thread, | 1137 | DEFVAR_LISP ("main-thread", Vmain_thread, |
| 1131 | doc: /* The main thread of Emacs. */); | 1138 | doc: /* The main thread of Emacs. */); |
| 1132 | #ifdef THREADS_ENABLED | 1139 | #ifdef THREADS_ENABLED |
| 1133 | XSETTHREAD (Vmain_thread, &main_thread); | 1140 | XSETTHREAD (Vmain_thread, &main_thread.s); |
| 1134 | #else | 1141 | #else |
| 1135 | Vmain_thread = Qnil; | 1142 | Vmain_thread = Qnil; |
| 1136 | #endif | 1143 | #endif |
diff --git a/src/xmenu.c b/src/xmenu.c index 10e882af439..31034f71122 100644 --- a/src/xmenu.c +++ b/src/xmenu.c | |||
| @@ -2420,6 +2420,6 @@ syms_of_xmenu (void) | |||
| 2420 | #if defined (USE_GTK) || defined (USE_X_TOOLKIT) | 2420 | #if defined (USE_GTK) || defined (USE_X_TOOLKIT) |
| 2421 | defsubr (&Sx_menu_bar_open_internal); | 2421 | defsubr (&Sx_menu_bar_open_internal); |
| 2422 | Ffset (intern_c_string ("accelerate-menu"), | 2422 | Ffset (intern_c_string ("accelerate-menu"), |
| 2423 | intern_c_string (Sx_menu_bar_open_internal.symbol_name)); | 2423 | intern_c_string (Sx_menu_bar_open_internal.s.symbol_name)); |
| 2424 | #endif | 2424 | #endif |
| 2425 | } | 2425 | } |