Re: Changes in GC and in pure space (was: [Emacs-diffs] master 5d4dd55: Fix lifetime error in previous patch)

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

>>> 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.

[-- Attachment #2: 0001-Don-t-debug-fset-by-default.patch --]
[-- Type: text/x-patch, Size: 2567 bytes --]

From 093515ae0db64058b0948dac5a42e9f72e06bcaa Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
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


[-- Attachment #3: 0002-Remove-no-longer-needed-workaround-for-GC-bug.patch --]
[-- Type: text/x-patch, Size: 1174 bytes --]

From 6c5f753c28a024d8808323d08c8466f9d8e86e68 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
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