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: Fri, 29 May 2020 13:24:55 -0700 Organization: UCLA Computer Science Department Message-ID: References: <83zha8cgpi.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> <837dwyvi74.fsf@gnu.org> <1484f569-c260-9fb0-bfe1-67897de289d3@cs.ucla.edu> <83blm9tn4j.fsf@gnu.org> <4aeb8963-4fd1-fcd4-e6e1-be409ab54775@cs.ucla.edu> <83tuzzrk30.fsf@gnu.org> <749bc7d0-6376-ec2e-7f84-dcd3a3cea465@cs.ucla.edu> <83sgfjqn49.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------00FF823AB9D0F52011EBB12D" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="63186"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 Cc: 41321@debbugs.gnu.org, monnier@iro.umontreal.ca, pipcet@gmail.com To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri May 29 22:26:11 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 1jelZz-000GNd-3d for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 29 May 2020 22:26:11 +0200 Original-Received: from localhost ([::1]:48948 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jelZx-0000ms-Lw for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 29 May 2020 16:26:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:44000) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jelZq-0000mU-7n for bug-gnu-emacs@gnu.org; Fri, 29 May 2020 16:26:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:44748) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jelZp-0006yh-UC for bug-gnu-emacs@gnu.org; Fri, 29 May 2020 16:26:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jelZp-0006GC-QP for bug-gnu-emacs@gnu.org; Fri, 29 May 2020 16:26:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 29 May 2020 20:26: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.159078390923993 (code B ref 41321); Fri, 29 May 2020 20:26:01 +0000 Original-Received: (at 41321) by debbugs.gnu.org; 29 May 2020 20:25:09 +0000 Original-Received: from localhost ([127.0.0.1]:56294 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jelYv-0006Ep-Dw for submit@debbugs.gnu.org; Fri, 29 May 2020 16:25:08 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:46782) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jelYt-0006EE-An for 41321@debbugs.gnu.org; Fri, 29 May 2020 16:25:03 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id AC26E1600CC; Fri, 29 May 2020 13:24:57 -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 GVfxDInY71S5; Fri, 29 May 2020 13:24:56 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 79D9F1600AF; Fri, 29 May 2020 13:24:56 -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 s4d-SP0b8JR9; Fri, 29 May 2020 13:24:56 -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 3FB2A1600C3; Fri, 29 May 2020 13:24:56 -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: <83sgfjqn49.fsf@gnu.org> 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:181207 Archived-At: This is a multi-part message in MIME format. --------------00FF823AB9D0F52011EBB12D Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 5/28/20 11:19 PM, Eli Zaretskii wrote: >> - return (uintptr_t) p % LISP_ALIGNMENT == 0; >> + return (uintptr_t) p % GCALIGNMENT == 0; >> } > ...replacing LISP_ALIGNMENT with GCALIGNMENT just here doesn't sound > right to me: by keeping the current value of LISP_ALIGNMENT, we > basically declare that Lisp objects shall be aligned on that boundary, > whereas that isn't really the case. Why not change the value of > LISP_ALIGNMENT instead? There are really two bugs here. 1. The idea of taking the address modulo LISP_ALIGNMENT is wrong, as a pointer can point into the middle of (say) a pseudovector and not be LISP_ALIGNMENT-aligned. Replacing LISP_ALIGNMENT with GCALIGNMENT does not fix this bug in general, because such a pointer might not be GCALIGNMENT-aligned either. This bug can cause crashes because it causes GC to think an object is garbage when it's not garbage. 2. LISP_ALIGNMENT is too large on MinGW and some other platforms. The patch I sent earlier attempted to be the simplest patch that would fix the bug you observed on MinGW, which is a special case of (1). It does not attempt to fix all plausible cases of (1), nor does it address (2). We can fix these two bugs separately, by installing the attached patches into emacs-27. The first patch fixes (1) and thus fixes the crash along with other plausible crashes. The second one fixes (2), and this fixes the MinGW crash in a different way but does not fix the crash on other plausible platforms. (1) probably has better performance than (2), though I doubt whether users will notice. --------------00FF823AB9D0F52011EBB12D Content-Type: text/x-patch; charset=UTF-8; name="0001-Remove-maybe_lisp_pointer.patch" Content-Disposition: attachment; filename="0001-Remove-maybe_lisp_pointer.patch" Content-Transfer-Encoding: quoted-printable >From 2c0bac868a7aefe7dafd2362cce42a7d3738319f Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 29 May 2020 12:56:16 -0700 Subject: [PATCH 1/2] =3D?UTF-8?q?Remove=3D20=3DE2=3D80=3D98maybe=3D5Flisp= =3D5Fpointer?=3D =3D?UTF-8?q?=3DE2=3D80=3D99?=3D MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit It=E2=80=99s an invalid optimization, since pointers can address the middle of Lisp_Object data. * src/alloc.c (maybe_lisp_pointer): Remove. Only use removed. Do not merge to master, as we=E2=80=99ll put in a better fix there. --- src/alloc.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 1c6b664b22..b8382aca5b 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4585,18 +4585,6 @@ mark_maybe_objects (Lisp_Object const *array, ptrd= iff_t nelts) mark_maybe_object (*array); } =20 -/* Return true if P might point to Lisp data that can be garbage - 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. */ - -static bool -maybe_lisp_pointer (void *p) -{ - return (uintptr_t) p % LISP_ALIGNMENT =3D=3D 0; -} - /* If P points to Lisp data, mark that as live if it isn't already marked. */ =20 @@ -4609,9 +4597,6 @@ mark_maybe_pointer (void *p) VALGRIND_MAKE_MEM_DEFINED (&p, sizeof (p)); #endif =20 - if (!maybe_lisp_pointer (p)) - return; - if (pdumper_object_p (p)) { int type =3D pdumper_find_object_type (p); --=20 2.17.1 --------------00FF823AB9D0F52011EBB12D Content-Type: text/x-patch; charset=UTF-8; name="0002-Don-t-overalign-Lisp-objects.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Don-t-overalign-Lisp-objects.patch" >From f620b5b802bf2afad033c7cc7856a71fd28b2c13 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 29 May 2020 13:02:32 -0700 Subject: [PATCH 2/2] =?UTF-8?q?Don=E2=80=99t=20overalign=20Lisp=20objects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport from master. * src/alloc.c (union emacs_align_type): New type, used for LISP_ALIGNMENT. (LISP_ALIGNMENT): Use it instead of max_align_t. --- src/alloc.c | 55 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index b8382aca5b..48e96863db 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 +}; + #ifdef DOUG_LEA_MALLOC /* Specify maximum number of areas to mmap. It would be nice to use a @@ -636,16 +676,11 @@ 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); -- 2.17.1 --------------00FF823AB9D0F52011EBB12D--