From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Changes in GC and in pure space (was: [Emacs-diffs] master 5d4dd55: Fix lifetime error in previous patch) Date: Wed, 21 Aug 2019 22:36:26 -0700 Organization: UCLA Computer Science Department Message-ID: References: <20190721193221.1964.53182@vcs0.savannah.gnu.org> <20190721193222.8C19E20BE2@vcs0.savannah.gnu.org> <83blxmqfkq.fsf@gnu.org> <9568ca7d-854f-1971-bbe8-03ba8c64af42@cs.ucla.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------30BD3E8209E5AFE5CC0E1816" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="50478"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 Cc: Eli Zaretskii , Daniel Colascione , emacs-devel@gnu.org To: Pip Cet Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Aug 22 07:36:43 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1i0fm6-000CwP-LA for ged-emacs-devel@m.gmane.org; Thu, 22 Aug 2019 07:36:42 +0200 Original-Received: from localhost ([::1]:38348 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i0fm4-0006hM-Ug for ged-emacs-devel@m.gmane.org; Thu, 22 Aug 2019 01:36:40 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:57129) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i0flx-0006hD-Ee for emacs-devel@gnu.org; Thu, 22 Aug 2019 01:36:35 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1i0flv-0006dN-PS for emacs-devel@gnu.org; Thu, 22 Aug 2019 01:36:33 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:37732) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1i0flt-0006VB-EB; Thu, 22 Aug 2019 01:36:29 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id EB73E1626EB; Wed, 21 Aug 2019 22:36:27 -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 wNjG8xAhZIAu; Wed, 21 Aug 2019 22:36:26 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id B4E2B162729; Wed, 21 Aug 2019 22:36:26 -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 MwWz_C8F90Of; Wed, 21 Aug 2019 22:36:26 -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 73C511626EB; Wed, 21 Aug 2019 22:36:26 -0700 (PDT) In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:239493 Archived-At: This is a multi-part message in MIME format. --------------30BD3E8209E5AFE5CC0E1816 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit >>> Here are things I would consider urgent enough to warrant looking at >>> for Emacs 27: >>> - remove code like we have in `Ffset' to detect GC bugs that >>> presumably have been fixed by now >> >> Are you talking about just the code introduced in commit >> 2014-04-03T00:18:08Z!dancol@dancol.org >> (01ae0fbf30b74e2490144fceabbf5bc5d96f1ba7), or is some other code involved? I'll >> CC this to dancol to see if he has thoughts. > > There's also reference to a GC bug in describe_vector. Thanks for mentioning these two items. I installed the attached two patches into master. The first arranges for SUSPICIOUS_OBJECT_CHECKING to not be defined unless one compiles with ENABLE_CHECKING, which should remove the Ffset hack in production code; the second removes that 25-year-old GC workaround that hasn't been needed for quite some time. Actually, all that SUSPICIOUS_OBJECT_CHECKING code can be removed. But one step at a time. --------------30BD3E8209E5AFE5CC0E1816 Content-Type: text/x-patch; name="0001-Don-t-debug-fset-by-default.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Don-t-debug-fset-by-default.patch" >From 093515ae0db64058b0948dac5a42e9f72e06bcaa Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 21 Aug 2019 22:19:03 -0700 Subject: [PATCH 1/2] =?UTF-8?q?Don=E2=80=99t=20debug=20fset=20by=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This GC bug seems to have been fixed, so the check is no longer needed in production code. From a suggestion by Pip Cet in: https://lists.gnu.org/r/emacs-devel/2019-08/msg00316.html * src/alloc.c (SUSPICIOUS_OBJECT_CHECKING) [!ENABLE_CHECKING]: Do not define. (find_suspicious_object_in_range, detect_suspicious_free): Expand to proper dummy expressions if !SUSPICIOUS_OBJECT_CHECKING. * src/data.c (Ffset): Convert test to an eassert. --- src/alloc.c | 12 ++++-------- src/data.c | 5 +---- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 53af7325f4..39964c4b29 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -302,15 +302,11 @@ #define PUREBEG (char *) pure const char *pending_malloc_warning; -#if 0 /* Normally, pointer sanity only on request... */ +/* Pointer sanity only on request. FIXME: Code depending on + SUSPICIOUS_OBJECT_CHECKING is obsolete; remove it entirely. */ #ifdef ENABLE_CHECKING #define SUSPICIOUS_OBJECT_CHECKING 1 #endif -#endif - -/* ... but unconditionally use SUSPICIOUS_OBJECT_CHECKING while the GC - bug is unresolved. */ -#define SUSPICIOUS_OBJECT_CHECKING 1 #ifdef SUSPICIOUS_OBJECT_CHECKING struct suspicious_free_record @@ -327,8 +323,8 @@ #define SUSPICIOUS_OBJECT_CHECKING 1 static void *find_suspicious_object_in_range (void *begin, void *end); static void detect_suspicious_free (void *ptr); #else -# define find_suspicious_object_in_range(begin, end) NULL -# define detect_suspicious_free(ptr) (void) +# define find_suspicious_object_in_range(begin, end) ((void *) NULL) +# define detect_suspicious_free(ptr) ((void) 0) #endif /* Maximum amount of C stack to save when a GC happens. */ diff --git a/src/data.c b/src/data.c index 8344bfdd3d..2797adfcdc 100644 --- a/src/data.c +++ b/src/data.c @@ -771,10 +771,7 @@ DEFUN ("fset", Ffset, Sfset, 2, 2, 0, if (AUTOLOADP (function)) Fput (symbol, Qautoload, XCDR (function)); - /* Convert to eassert or remove after GC bug is found. In the - meantime, check unconditionally, at a slight perf hit. */ - if (! valid_lisp_object_p (definition)) - emacs_abort (); + eassert (valid_lisp_object_p (definition)); set_symbol_function (symbol, definition); -- 2.17.1 --------------30BD3E8209E5AFE5CC0E1816 Content-Type: text/x-patch; name="0002-Remove-no-longer-needed-workaround-for-GC-bug.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Remove-no-longer-needed-workaround-for-GC-bug.patch" >From 6c5f753c28a024d8808323d08c8466f9d8e86e68 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 21 Aug 2019 22:29:35 -0700 Subject: [PATCH 2/2] Remove no-longer-needed workaround for GC bug * src/keymap.c (describe_vector): Remove old workaround for GC bug. This workaround, introduced in 1993-02-19T05:43:54Z!rms@gnu.org, has not been needed for some time. Problem reported by Pip Cet in: https://lists.gnu.org/r/emacs-devel/2019-08/msg00316.html --- src/keymap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/keymap.c b/src/keymap.c index 6762915f70..b1e09a92f2 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -3371,12 +3371,10 @@ describe_vector (Lisp_Object vector, Lisp_Object prefix, Lisp_Object args, if (!keymap_p) { - /* Call Fkey_description first, to avoid GC bug for the other string. */ if (!NILP (prefix) && XFIXNAT (Flength (prefix)) > 0) { - Lisp_Object tem = Fkey_description (prefix, Qnil); AUTO_STRING (space, " "); - elt_prefix = concat2 (tem, space); + elt_prefix = concat2 (Fkey_description (prefix, Qnil), space); } prefix = Qnil; } -- 2.17.1 --------------30BD3E8209E5AFE5CC0E1816--