From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#41321: 27.0.91; Emacs aborts due to invalid pseudovector objects Date: Mon, 25 May 2020 23:46:02 -0700 Organization: UCLA Computer Science Department Message-ID: References: <83zha8cgpi.fsf@gnu.org> <83r1vibmyj.fsf@gnu.org> <83imgublku.fsf@gnu.org> <831rncjuwf.fsf@gnu.org> <83h7w5xvfa.fsf@gnu.org> <83y2phwb9x.fsf@gnu.org> <83r1v9w9vi.fsf@gnu.org> <83mu5xw50d.fsf@gnu.org> <83k110wxte.fsf@gnu.org> <4bab5f55-95fe-cf34-e490-1d4319728395@cs.ucla.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------65049CAFD0B1249059991EAB" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="19854"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 Cc: 41321@debbugs.gnu.org, Stefan Monnier To: Pip Cet , Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue May 26 08:47:15 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jdTMp-00057l-BQ for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 26 May 2020 08:47:15 +0200 Original-Received: from localhost ([::1]:44340 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jdTMo-0005P4-8R for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 26 May 2020 02:47:14 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:37016) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jdTMc-0005Ox-BO for bug-gnu-emacs@gnu.org; Tue, 26 May 2020 02:47:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:60515) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jdTMc-0006wg-2P for bug-gnu-emacs@gnu.org; Tue, 26 May 2020 02:47:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jdTMc-0004we-1j for bug-gnu-emacs@gnu.org; Tue, 26 May 2020 02:47:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 26 May 2020 06:47:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 41321 X-GNU-PR-Package: emacs Original-Received: via spool by 41321-submit@debbugs.gnu.org id=B41321.159047557218948 (code B ref 41321); Tue, 26 May 2020 06:47:01 +0000 Original-Received: (at 41321) by debbugs.gnu.org; 26 May 2020 06:46:12 +0000 Original-Received: from localhost ([127.0.0.1]:43828 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jdTLn-0004vY-Dg for submit@debbugs.gnu.org; Tue, 26 May 2020 02:46:12 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:59790) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jdTLl-0004vL-Ht for 41321@debbugs.gnu.org; Tue, 26 May 2020 02:46:10 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 04B7E160052; Mon, 25 May 2020 23:46:04 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id OBiqjiW_2gJS; Mon, 25 May 2020 23:46:02 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 93E691600E0; Mon, 25 May 2020 23:46:02 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id iN1M5z2fITmA; Mon, 25 May 2020 23:46:02 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 553C7160052; Mon, 25 May 2020 23:46:02 -0700 (PDT) Autocrypt: addr=eggert@cs.ucla.edu; prefer-encrypt=mutual; keydata= LS0tLS1CRUdJTiBQR1AgUFVCTElDIEtFWSBCTE9DSy0tLS0tCgptUUlOQkV5QWNtUUJFQURB QXlIMnhvVHU3cHBHNUQzYThGTVpFb243NGRDdmM0K3ExWEEySjJ0QnkycHdhVHFmCmhweHhk R0E5Smo1MFVKM1BENGJTVUVnTjh0TFowc2FuNDdsNVhUQUZMaTI0NTZjaVNsNW04c0thSGxH ZHQ5WG0KQUF0bVhxZVpWSVlYL1VGUzk2ZkR6ZjR4aEVtbS95N0xiWUVQUWRVZHh1NDd4QTVL aFRZcDVibHRGM1dZRHoxWQpnZDdneDA3QXV3cDdpdzdlTnZub0RUQWxLQWw4S1lEWnpiRE5D UUdFYnBZM2VmWkl2UGRlSStGV1FONFcra2doCnkrUDZhdTZQcklJaFlyYWV1YTdYRGRiMkxT MWVuM1NzbUUzUWpxZlJxSS9BMnVlOEpNd3N2WGUvV0szOEV6czYKeDc0aVRhcUkzQUZINmls QWhEcXBNbmQvbXNTRVNORnQ3NkRpTzFaS1FNcjlhbVZQa25qZlBtSklTcWRoZ0IxRApsRWR3 MzRzUk9mNlY4bVp3MHhmcVQ2UEtFNDZMY0ZlZnpzMGtiZzRHT1JmOHZqRzJTZjF0azVlVThN Qml5Ti9iClowM2JLTmpOWU1wT0REUVF3dVA4NGtZTGtYMndCeHhNQWhCeHdiRFZadWR6eERa SjFDMlZYdWpDT0pWeHEya2wKakJNOUVUWXVVR3FkNzVBVzJMWHJMdzYrTXVJc0hGQVlBZ1Jy NytLY3dEZ0JBZndoU In-Reply-To: <4bab5f55-95fe-cf34-e490-1d4319728395@cs.ucla.edu> Content-Language: en-US X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:181021 Archived-At: This is a multi-part message in MIME format. --------------65049CAFD0B1249059991EAB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 5/25/20 8:33 PM, Paul Eggert wrote: > On 5/25/20 4:28 AM, Pip Cet wrote: > >> And I just noticed strings aren't aligned to LISP_ALIGNMENT on >> x86_64-pc-linux-gnu. > > Could you explain? Oh, never mind, I figured it out. Sorry about the noise. I installed the first attached patch to fix the bug on master (as a series of commits, the leading ones not quite right unfortunately). This patch does what you proposed, and also tightens up some of the related alignment checks. I propose the second patch for emacs-27; it's limited to what you proposed, namely, it weakens maybe_lisp_pointer to check only for GC_ALIGNMENT. --------------65049CAFD0B1249059991EAB Content-Type: text/x-patch; charset=UTF-8; name="emacs.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="emacs.diff" diff --git a/src/alloc.c b/src/alloc.c index d5a6d9167e..f8609398a3 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -104,6 +104,46 @@ Copyright (C) 1985-1986, 1988, 1993-1995, 1997-2020 Free Software #include "w32heap.h" /* for sbrk */ #endif +/* A type with alignment at least as large as any object that Emacs + allocates. This is not max_align_t because some platforms (e.g., + mingw) have buggy malloc implementations that do not align for + max_align_t. This union contains types of all GCALIGNED_STRUCT + components visible here. */ +union emacs_align_type +{ + struct frame frame; + struct Lisp_Bignum Lisp_Bignum; + struct Lisp_Bool_Vector Lisp_Bool_Vector; + struct Lisp_Char_Table Lisp_Char_Table; + struct Lisp_CondVar Lisp_CondVar; + struct Lisp_Finalizer Lisp_Finalizer; + struct Lisp_Float Lisp_Float; + struct Lisp_Hash_Table Lisp_Hash_Table; + struct Lisp_Marker Lisp_Marker; + struct Lisp_Misc_Ptr Lisp_Misc_Ptr; + struct Lisp_Mutex Lisp_Mutex; + struct Lisp_Overlay Lisp_Overlay; + struct Lisp_Sub_Char_Table Lisp_Sub_Char_Table; + struct Lisp_Subr Lisp_Subr; + struct Lisp_User_Ptr Lisp_User_Ptr; + struct Lisp_Vector Lisp_Vector; + struct terminal terminal; + struct thread_state thread_state; + struct window window; + + /* Omit the following since they would require including process.h + etc. In practice their alignments never exceed that of the + structs already listed. */ +#if 0 + struct Lisp_Module_Function Lisp_Module_Function; + struct Lisp_Process Lisp_Process; + struct save_window_data save_window_data; + struct scroll_bar scroll_bar; + struct xwidget_view xwidget_view; + struct xwidget xwidget; +#endif +}; + /* MALLOC_SIZE_NEAR (N) is a good number to pass to malloc when allocating a block of memory with size close to N bytes. For best results N should be a power of 2. @@ -112,9 +152,9 @@ Copyright (C) 1985-1986, 1988, 1993-1995, 1997-2020 Free Software adds sizeof (size_t) to SIZE for internal overhead, and then rounds up to a multiple of MALLOC_ALIGNMENT. Emacs can improve performance a bit on GNU platforms by arranging for the resulting - size to be a power of two. This heuristic is good for glibc 2.0 - (1997) through at least glibc 2.31 (2020), and does not affect - correctness on other platforms. */ + size to be a power of two. This heuristic is good for glibc 2.26 + (2017) and later, and does not affect correctness on other + platforms. */ #define MALLOC_SIZE_NEAR(n) \ (ROUNDUP (max (n, sizeof (size_t)), MALLOC_ALIGNMENT) - sizeof (size_t)) @@ -655,25 +695,19 @@ buffer_memory_full (ptrdiff_t nbytes) #define COMMON_MULTIPLE(a, b) \ ((a) % (b) == 0 ? (a) : (b) % (a) == 0 ? (b) : (a) * (b)) -/* LISP_ALIGNMENT is the alignment of Lisp objects. It must be at - least GCALIGNMENT so that pointers can be tagged. It also must be - at least as strict as the alignment of all the C types used to - implement Lisp objects; since pseudovectors can contain any C type, - this is max_align_t. On recent GNU/Linux x86 and x86-64 this can - often waste up to 8 bytes, since alignof (max_align_t) is 16 but - typical vectors need only an alignment of 8. Although shrinking - the alignment to 8 would save memory, it cost a 20% hit to Emacs - CPU performance on Fedora 28 x86-64 when compiled with gcc -m32. */ -enum { LISP_ALIGNMENT = alignof (union { max_align_t x; +/* Alignment needed for memory blocks that are allocated via malloc + and that contain Lisp objects. On typical hosts malloc already + aligns sufficiently, but extra work is needed on oddball hosts + where Emacs would crash if malloc returned a non-GCALIGNED pointer. */ +enum { LISP_ALIGNMENT = alignof (union { union emacs_align_type x; GCALIGNED_UNION_MEMBER }) }; verify (LISP_ALIGNMENT % GCALIGNMENT == 0); /* True if malloc (N) is known to return storage suitably aligned for Lisp objects whenever N is a multiple of LISP_ALIGNMENT. In practice this is true whenever alignof (max_align_t) is also a - multiple of LISP_ALIGNMENT. This works even for x86, where some - platform combinations (e.g., GCC 7 and later, glibc 2.25 and - earlier) have bugs where alignof (max_align_t) is 16 even though + multiple of LISP_ALIGNMENT. This works even for buggy platforms + like MinGW circa 2020, where alignof (max_align_t) is 16 even though the malloc alignment is only 8, and where Emacs still works because it never does anything that requires an alignment of 16. */ enum { MALLOC_IS_LISP_ALIGNED = alignof (max_align_t) % LISP_ALIGNMENT == 0 }; @@ -4657,12 +4691,12 @@ mark_maybe_objects (Lisp_Object const *array, ptrdiff_t nelts) collected, and false otherwise (i.e., false if it is easy to see that P cannot point to Lisp data that can be garbage collected). Symbols are implemented via offsets not pointers, but the offsets - are also multiples of LISP_ALIGNMENT. */ + are also multiples of GCALIGNMENT. */ static bool maybe_lisp_pointer (void *p) { - return (uintptr_t) p % LISP_ALIGNMENT == 0; + return (uintptr_t) p % GCALIGNMENT == 0; } /* If P points to Lisp data, mark that as live if it isn't already @@ -4885,9 +4919,10 @@ test_setjmp (void) as a stack scan limit. */ typedef union { - /* Align the stack top properly. Even if !HAVE___BUILTIN_UNWIND_INIT, - jmp_buf may not be aligned enough on darwin-ppc64. */ - max_align_t o; + /* Make sure stack_top and m_stack_bottom are properly aligned as GC + expects. */ + Lisp_Object o; + void *p; #ifndef HAVE___BUILTIN_UNWIND_INIT sys_jmp_buf j; char c; diff --git a/src/lisp.h b/src/lisp.h index 85bdc172b2..8bd83a888c 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -277,7 +277,8 @@ DEFINE_GDB_SYMBOL_END (VALMASK) allocation in a containing union that has GCALIGNED_UNION_MEMBER) and does not contain a GC-aligned struct or union, putting GCALIGNED_STRUCT after its closing '}' can help the compiler - generate better code. + generate better code. Also, such structs should be added to the + emacs_align_type union in alloc.c. Although these macros are reasonably portable, they are not guaranteed on non-GCC platforms, as C11 does not require support diff --git a/src/thread.c b/src/thread.c index df1a705382..b638dd77f8 100644 --- a/src/thread.c +++ b/src/thread.c @@ -717,12 +717,17 @@ run_thread (void *state) { /* Make sure stack_top and m_stack_bottom are properly aligned as GC expects. */ - max_align_t stack_pos; + union + { + Lisp_Object o; + void *p; + char c; + } stack_pos; struct thread_state *self = state; struct thread_state **iter; - self->m_stack_bottom = self->stack_top = (char *) &stack_pos; + self->m_stack_bottom = self->stack_top = &stack_pos.c; self->thread_id = sys_thread_self (); if (self->thread_name) --------------65049CAFD0B1249059991EAB Content-Type: text/x-patch; charset=UTF-8; name="0001-Fix-aborts-due-to-GC-losing-pseudovectors.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Fix-aborts-due-to-GC-losing-pseudovectors.patch" >From 7466aeeb4ac3dace283ecef00c2b38148b56b3b3 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 25 May 2020 23:39:37 -0700 Subject: [PATCH] Fix aborts due to GC losing pseudovectors Problem reported by Eli Zaretskii (Bug#41321). * src/alloc.c (maybe_lisp_pointer): Modulo GCALIGNMENT, not modulo LISP_ALIGNMENT. Master has a more-elaborate fix. Do not merge to master. --- src/alloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 1c6b664b22..c7a4a3ee86 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4589,12 +4589,12 @@ mark_maybe_objects (Lisp_Object const *array, ptrdiff_t nelts) collected, and false otherwise (i.e., false if it is easy to see that P cannot point to Lisp data that can be garbage collected). Symbols are implemented via offsets not pointers, but the offsets - are also multiples of LISP_ALIGNMENT. */ + are also multiples of GCALIGNMENT. */ static bool maybe_lisp_pointer (void *p) { - return (uintptr_t) p % LISP_ALIGNMENT == 0; + return (uintptr_t) p % GCALIGNMENT == 0; } /* If P points to Lisp data, mark that as live if it isn't already -- 2.17.1 --------------65049CAFD0B1249059991EAB--