bug#41321: 27.0.91; Emacs aborts due to invalid pseudovector objects

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

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

On 5/26/20 8:17 AM, Eli Zaretskii wrote:
>>  static bool
>>  maybe_lisp_pointer (void *p)
>>  {
>> -  return (uintptr_t) p % LISP_ALIGNMENT == 0;
>> +  return (uintptr_t) p % GCALIGNMENT == 0;
>>  }
> On non-USE_LSB_TAG systems, GCALIGNMENT is 1, so this doesn't look
> right (or maybe I'm missing something).

Good point; I'd neglected that. I.e., on !USE_LSB_TAG systems the proposed
emacs-27 patch is overly-conservative, as it causes maybe_lisp_pointer to always
return true. Although this hurts GC performance it doesn't affect correctness
and the patch does fix a crash on USE_LSB_TAG systems, so it (or something like
it) is needed for emacs-27.

I installed the attached patch into master to fix the !USE_LSB_TAG performance
issue you raised.  This patch does not fix crashes; it's merely a performance tweak.

I am planning on looking into related crashes for Lisp_Symbol next. Perhaps we
should wait on that before worrying about what exact patch should go into emacs-27.

[-- Attachment #2: 0001-Tweak-GC-performance-if-USE_LSB_TAG.patch --]
[-- Type: text/x-patch, Size: 2104 bytes --]

From 7a83c4f66cb945d43dcaf8c37f4af1334d34f501 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 26 May 2020 15:47:59 -0700
Subject: [PATCH] Tweak GC performance if !USE_LSB_TAG

Performance issue reported by Eli Zaretskii (Bug#41321#149).
* src/alloc.c (GC_OBJECT_ALIGNMENT_MINIMUM): New constant.
(maybe_lisp_pointer): Use it instead of GCALIGNMENT.
---
 src/alloc.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index f8609398a3..e241b9933a 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -4687,16 +4687,33 @@ mark_maybe_objects (Lisp_Object const *array, ptrdiff_t nelts)
     mark_maybe_object (*array);
 }
 
+/* A lower bound on the alignment of Lisp objects that need marking.
+   Although 1 is safe, higher values speed up mark_maybe_pointer.
+   If USE_LSB_TAG, this value is typically GCALIGNMENT; otherwise,
+   it's determined by the natural alignment of Lisp structs.
+   All vectorlike objects have alignment at least that of union
+   vectorlike_header and it's unlikely they all have alignment greater,
+   so use the union as a safe and likely-accurate standin for
+   vectorlike objects.  */
+
+enum { GC_OBJECT_ALIGNMENT_MINIMUM
+         = max (GCALIGNMENT,
+		min (alignof (union vectorlike_header),
+		     min (min (alignof (struct Lisp_Cons),
+			       alignof (struct Lisp_Float)),
+			  min (alignof (struct Lisp_String),
+			       alignof (struct Lisp_Symbol))))) };
+
 /* 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 GCALIGNMENT.  */
+   are also multiples of GC_OBJECT_ALIGNMENT_MINIMUM.  */
 
 static bool
 maybe_lisp_pointer (void *p)
 {
-  return (uintptr_t) p % GCALIGNMENT == 0;
+  return (uintptr_t) p % GC_OBJECT_ALIGNMENT_MINIMUM == 0;
 }
 
 /* If P points to Lisp data, mark that as live if it isn't already
-- 
2.17.1