diff options
| author | Paul Eggert | 2020-05-25 22:06:25 -0700 |
|---|---|---|
| committer | Paul Eggert | 2020-05-25 22:06:44 -0700 |
| commit | 3abf76da564ff8526bbcba6b92dfb7b97cb87779 (patch) | |
| tree | 5378b937bd67b0dcbfa2a3ec1d5fb0a62485f06a | |
| parent | 0dc529175dc027c1567fb9b7cd529d29236aad44 (diff) | |
| download | emacs-3abf76da564ff8526bbcba6b92dfb7b97cb87779.tar.gz emacs-3abf76da564ff8526bbcba6b92dfb7b97cb87779.zip | |
Refix aborts due to GC losing pseudovectors
This is simpler, and fixes a bug in the previous fix.
* src/alloc.c (MALLOC_ALIGNMENT_BOUND): Simplify by
using max_align_t, since the buggy implementations won’t
break this simpler implementation.
(LISP_ALIGNMENT): Simplify by just using GCALIGNMENT, since the
fancier implementation wasn’t correct anyway, and fixing it
isn’t worth the trouble on practical platforms.
* src/lisp.h (union emacs_align_type): Remove.
| -rw-r--r-- | src/alloc.c | 32 | ||||
| -rw-r--r-- | src/lisp.h | 43 |
2 files changed, 14 insertions, 61 deletions
diff --git a/src/alloc.c b/src/alloc.c index 602282e5704..89fe96a2349 100644 --- a/src/alloc.c +++ b/src/alloc.c | |||
| @@ -655,26 +655,20 @@ buffer_memory_full (ptrdiff_t nbytes) | |||
| 655 | #define COMMON_MULTIPLE(a, b) \ | 655 | #define COMMON_MULTIPLE(a, b) \ |
| 656 | ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) | 656 | ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) |
| 657 | 657 | ||
| 658 | /* A lower bound on the alignment of malloc. For better performance | 658 | /* A lower bound on the alignment of malloc. Although this bound is |
| 659 | this bound should be tighter. For glibc 2.26 and later a tighter | 659 | incorrect for some buggy malloc implementations (e.g., MinGW circa |
| 660 | bound is known. */ | 660 | 2020), the bugs should not matter for the way this bound is used |
| 661 | #if 2 < __GLIBC__ + (26 <= __GLIBC_MINOR__) | 661 | since the correct bound is also a multiple of LISP_ALIGNMENT on the |
| 662 | enum { MALLOC_ALIGNMENT_BOUND = MALLOC_ALIGNMENT }; | 662 | buggy platforms. */ |
| 663 | #else | 663 | enum { MALLOC_ALIGNMENT_BOUND = alignof (max_align_t) }; |
| 664 | /* A bound known to work for all Emacs porting targets. Tightening | 664 | |
| 665 | this looser bound by using max_align_t instead of long long int | 665 | /* A lower bound on the alignment of Lisp objects allocated on the heap. |
| 666 | would break buggy malloc implementations like MinGW circa 2020. */ | 666 | All such objects must have an address that is a multiple of LISP_ALIGNMENT; |
| 667 | enum { MALLOC_ALIGNMENT_BOUND = alignof (long long int) }; | ||
| 668 | #endif | ||
| 669 | |||
| 670 | /* A lower bound on the alignment of Lisp objects. All Lisp objects | ||
| 671 | must have an address that is a multiple of LISP_ALIGNMENT; | ||
| 672 | otherwise maybe_lisp_pointer can issue false negatives, causing crashes. | 667 | otherwise maybe_lisp_pointer can issue false negatives, causing crashes. |
| 673 | It's good to make this bound tight: if Lisp objects are always | 668 | On all practical Emacs targets, sizeof (struct Lisp_Float) == 8 and |
| 674 | aligned more strictly than LISP_ALIGNMENT, maybe_lisp_pointer will | 669 | since GCALIGNMENT also equals 8 there's little point to optimizing |
| 675 | issue more false positives, hurting performance. */ | 670 | for impractical targets. */ |
| 676 | enum { LISP_ALIGNMENT = max (max (GCALIGNMENT, MALLOC_ALIGNMENT_BOUND), | 671 | enum { LISP_ALIGNMENT = GCALIGNMENT }; |
| 677 | alignof (union emacs_align_type)) }; | ||
| 678 | 672 | ||
| 679 | /* True if malloc (N) is known to return storage suitably aligned for | 673 | /* True if malloc (N) is known to return storage suitably aligned for |
| 680 | Lisp objects whenever N is a multiple of LISP_ALIGNMENT. */ | 674 | Lisp objects whenever N is a multiple of LISP_ALIGNMENT. */ |
diff --git a/src/lisp.h b/src/lisp.h index 937052f6df8..85bdc172b20 100644 --- a/src/lisp.h +++ b/src/lisp.h | |||
| @@ -277,8 +277,7 @@ error !; | |||
| 277 | allocation in a containing union that has GCALIGNED_UNION_MEMBER) | 277 | allocation in a containing union that has GCALIGNED_UNION_MEMBER) |
| 278 | and does not contain a GC-aligned struct or union, putting | 278 | and does not contain a GC-aligned struct or union, putting |
| 279 | GCALIGNED_STRUCT after its closing '}' can help the compiler | 279 | GCALIGNED_STRUCT after its closing '}' can help the compiler |
| 280 | generate better code. Also, such structs should be added to the | 280 | generate better code. |
| 281 | emacs_align_type union. | ||
| 282 | 281 | ||
| 283 | Although these macros are reasonably portable, they are not | 282 | Although these macros are reasonably portable, they are not |
| 284 | guaranteed on non-GCC platforms, as C11 does not require support | 283 | guaranteed on non-GCC platforms, as C11 does not require support |
| @@ -5060,46 +5059,6 @@ maybe_gc (void) | |||
| 5060 | maybe_garbage_collect (); | 5059 | maybe_garbage_collect (); |
| 5061 | } | 5060 | } |
| 5062 | 5061 | ||
| 5063 | /* A type with alignment at least as large as any object that Emacs | ||
| 5064 | allocates. This is not max_align_t because some platforms (e.g., | ||
| 5065 | mingw) have buggy malloc implementations that do not align for | ||
| 5066 | max_align_t. This union contains types of all GCALIGNED_STRUCT | ||
| 5067 | components visible here. */ | ||
| 5068 | union emacs_align_type | ||
| 5069 | { | ||
| 5070 | struct Lisp_Bool_Vector Lisp_Bool_Vector; | ||
| 5071 | struct Lisp_Char_Table Lisp_Char_Table; | ||
| 5072 | struct Lisp_CondVar Lisp_CondVar; | ||
| 5073 | struct Lisp_Finalizer Lisp_Finalizer; | ||
| 5074 | struct Lisp_Float Lisp_Float; | ||
| 5075 | struct Lisp_Hash_Table Lisp_Hash_Table; | ||
| 5076 | struct Lisp_Marker Lisp_Marker; | ||
| 5077 | struct Lisp_Misc_Ptr Lisp_Misc_Ptr; | ||
| 5078 | struct Lisp_Mutex Lisp_Mutex; | ||
| 5079 | struct Lisp_Overlay Lisp_Overlay; | ||
| 5080 | struct Lisp_Sub_Char_Table Lisp_Sub_Char_Table; | ||
| 5081 | struct Lisp_Subr Lisp_Subr; | ||
| 5082 | struct Lisp_User_Ptr Lisp_User_Ptr; | ||
| 5083 | struct Lisp_Vector Lisp_Vector; | ||
| 5084 | struct thread_state thread_state; | ||
| 5085 | |||
| 5086 | /* Omit the following since they would require including bignum.h, | ||
| 5087 | frame.h etc., and in practice their alignments never exceed that | ||
| 5088 | of the structs already listed. */ | ||
| 5089 | #if 0 | ||
| 5090 | struct frame frame; | ||
| 5091 | struct Lisp_Bignum Lisp_Bignum; | ||
| 5092 | struct Lisp_Module_Function Lisp_Module_Function; | ||
| 5093 | struct Lisp_Process Lisp_Process; | ||
| 5094 | struct save_window_data save_window_data; | ||
| 5095 | struct scroll_bar scroll_bar; | ||
| 5096 | struct terminal terminal; | ||
| 5097 | struct window window; | ||
| 5098 | struct xwidget xwidget; | ||
| 5099 | struct xwidget_view xwidget_view; | ||
| 5100 | #endif | ||
| 5101 | }; | ||
| 5102 | |||
| 5103 | INLINE_HEADER_END | 5062 | INLINE_HEADER_END |
| 5104 | 5063 | ||
| 5105 | #endif /* EMACS_LISP_H */ | 5064 | #endif /* EMACS_LISP_H */ |