GNU bug report logs - #75318
Lisp_Subr header and layout

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: 75318 <at> debbugs.gnu.org
Cc: monnier <at> iro.umontreal.ca
Subject: bug#75318: Lisp_Subr header and layout
Date: Tue, 21 Jan 2025 23:46:45 +0000

Pip Cet <pipcet <at> protonmail.com> writes:

> "Pip Cet via \"Bug reports for GNU Emacs, the Swiss army knife of text editors\"" <bug-gnu-emacs <at> gnu.org> writes:
>
>> I'll prepare patches for those two changes (remove command_modes;
>> correct pvec headers for subrs); not the third (reorder Lisp_Subr
>> fields) because it's a nativecomp change, and there may be a small
>> performance impact.
>
> The second patch fixes Lisp_Subr initialization: the old code set the

This third, small patch moves the command_modes field into the
HAVE_NATIVE_COMP #ifdef.  It adjusts Fcommand_modes to return Qnil for
builtin subrs in the !HAVE_NATIVE_COMP case, and adjusts pdumper, fixing
a latent bug: it used dump_field_emacs_ptr instead of dump_field_lv.  As
that might have been tricky to debug, I briefly confirmed that no
similar bugs remain by compiling with this and --enable-check-lisp-object-type:

#define CHECK_POINTER(p) ((0) ? (void)(*(p)) : (void) (0))
#define dump_field_emacs_ptr(ctx, out, in_start, in_field) (CHECK_POINTER(*(in_field)), dump_field_emacs_ptr(ctx, out, in_start, in_field))

(similarly for other functions using a void * which itself must point to
a pointer, not a Lisp_Object).

This change isn't about saving memory; on my system, it doesn't, since
GCC aligns all Lisp Subrs to 32-byte boundaries on my system.

From 7b1384a7e71f783f3946e7aaad092e8095386387 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet <at> protonmail.com>
Subject: [PATCH] Remove unused command_modes from Lisp_Subr structure
 (bug#75318)

This field isn't marked by the garbage collector in !HAVE_NATIVE_COMP
builds, so using it in those would have led to very tricky bugs.

* src/data.c (Fcommand_modes) [!HAVE_NATIVE_COMP]: Return Qnil for
built-in subrs.
* src/lisp.h (struct Lisp_Subr) [!HAVE_NATIVE_COMP]:
(SUBR_NATIVECOMP_INITIALIZER):
(SUBR_INITIALIZER): Remove command_modes field.
* src/pdumper.c (dump_subr): Don't dump command_modes field unless
necessary.  When it is necessary, use dump_field_lv, not
dump_field_emacs_ptr.
---
 src/data.c    | 2 ++
 src/lisp.h    | 4 ++--
 src/pdumper.c | 3 +--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/data.c b/src/data.c
index dcaa5756ebe..bb7238065f5 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1234,7 +1234,9 @@ DEFUN ("command-modes", Fcommand_modes, Scommand_modes, 1, 1, 0,
 
   if (SUBRP (fun))
     {
+#ifdef HAVE_NATIVE_COMP
       return XSUBR (fun)->command_modes;
+#endif
     }
   else if (CLOSUREP (fun))
     {
diff --git a/src/lisp.h b/src/lisp.h
index 9c31630cc1a..46e940dad09 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -2226,12 +2226,12 @@ CHAR_TABLE_SET (Lisp_Object ct, int idx, Lisp_Object val)
       const char *string;
       Lisp_Object native;
     } intspec;
-    Lisp_Object command_modes;
     /* Positive values: offset into etc/DOC.  Negative values: one's
        complement of index into the native comp unit's vector of
        documentation strings.  */
     EMACS_INT doc;
 #ifdef HAVE_NATIVE_COMP
+    Lisp_Object command_modes;
     Lisp_Object native_comp_u;
     char *native_c_name;
     Lisp_Object lambda_list;
@@ -3486,6 +3486,7 @@ #define SUBR_HEADER { .size =						\
 
 #ifdef HAVE_NATIVE_COMP
 #define SUBR_NATIVECOMP_INITIALIZER			\
+    .command_modes = LISPSYM_INITIALLY (Qnil),		\
     .native_comp_u = LISPSYM_INITIALLY (Qnil),		\
     .lambda_list = LISPSYM_INITIALLY (Qnil),		\
     .type = LISPSYM_INITIALLY (Qnil),
@@ -3500,7 +3501,6 @@ #define SUBR_INITIALIZER(lname, fnname, minargs, maxargs, intspec_value) \
     .max_args = maxargs,						\
     .symbol_name = lname,						\
     .intspec = { .string = intspec_value },				\
-    .command_modes = LISPSYM_INITIALLY (Qnil),				\
     SUBR_NATIVECOMP_INITIALIZER						\
     }}
 
diff --git a/src/pdumper.c b/src/pdumper.c
index e83c7bcf9a1..64afbdd3065 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2992,16 +2992,15 @@ dump_subr (struct dump_context *ctx, const struct Lisp_Subr *subr)
                              COLD_OP_NATIVE_SUBR,
 			     make_lisp_ptr ((void *) subr, Lisp_Vectorlike));
       dump_field_lv (ctx, &out, subr, &subr->intspec.native, WEIGHT_NORMAL);
-      dump_field_lv (ctx, &out, subr, &subr->command_modes, WEIGHT_NORMAL);
     }
   else
     {
       dump_field_emacs_ptr (ctx, &out, subr, &subr->symbol_name);
       dump_field_emacs_ptr (ctx, &out, subr, &subr->intspec.string);
-      dump_field_emacs_ptr (ctx, &out, subr, &subr->command_modes);
     }
   DUMP_FIELD_COPY (&out, subr, doc);
 #ifdef HAVE_NATIVE_COMP
+  dump_field_lv (ctx, &out, subr, &subr->command_modes, WEIGHT_NORMAL);
   dump_field_lv (ctx, &out, subr, &subr->native_comp_u, WEIGHT_NORMAL);
   if (!NILP (subr->native_comp_u))
     dump_field_fixup_later (ctx, &out, subr, &subr->native_c_name);
-- 
2.47.1

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.