master 1d754c79603 4/4: Change HASH_UNUSED_ENTRY_KEY from Qunbound to NULL float

[Mattias Engdegård]

branch: master
commit 1d754c79603f1b6e4574c7e64c1bf5fb8c6c190d
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>

    Change HASH_UNUSED_ENTRY_KEY from Qunbound to NULL float
    
    This removes hacks from code that had to be careful not to use
    Qunbound as a hash table key, at the cost of a minor hack in
    the GC marker.
    
    * src/lisp.h (INVALID_LISP_VALUE, HASH_UNUSED_ENTRY_KEY):
    Define as a null-pointer float.
    * src/alloc.c (process_mark_stack): Add hack to ignore that value.
    * src/pdumper.c (dump_object_needs_dumping_p)
    (pdumper_init_symbol_unbound, pdumper_load):
    * src/print.c (PRINT_CIRCLE_CANDIDATE_P): Remove hacks for Qunbound.
---
 src/alloc.c   | 24 ++++++++++++++++--------
 src/lisp.h    |  7 ++++++-
 src/pdumper.c | 19 +------------------
 src/print.c   |  8 ++++++--
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index b78445f65df..2a1690d2cff 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -7293,6 +7293,9 @@ process_mark_stack (ptrdiff_t base_sp)
                  struct Lisp_Hash_Table *h = (struct Lisp_Hash_Table *)ptr;
                  set_vector_marked (ptr);
                  if (h->weakness == Weak_None)
+                   /* The values pushed here may include
+                      HASH_UNUSED_ENTRY_KEY, which this function must
+                      cope with.  */
                    mark_stack_push_values (h->key_and_value,
                                            2 * h->table_size);
                  else
@@ -7437,14 +7440,19 @@ process_mark_stack (ptrdiff_t base_sp)
          }
 
        case Lisp_Float:
-         CHECK_ALLOCATED_AND_LIVE (live_float_p, MEM_TYPE_FLOAT);
-         /* Do not mark floats stored in a dump image: these floats are
-            "cold" and do not have mark bits.  */
-         if (pdumper_object_p (XFLOAT (obj)))
-           eassert (pdumper_cold_object_p (XFLOAT (obj)));
-         else if (!XFLOAT_MARKED_P (XFLOAT (obj)))
-           XFLOAT_MARK (XFLOAT (obj));
-         break;
+         {
+           struct Lisp_Float *f = XFLOAT (obj);
+           if (!f)
+             break;            /* for HASH_UNUSED_ENTRY_KEY */
+           CHECK_ALLOCATED_AND_LIVE (live_float_p, MEM_TYPE_FLOAT);
+           /* Do not mark floats stored in a dump image: these floats are
+              "cold" and do not have mark bits.  */
+           if (pdumper_object_p (f))
+             eassert (pdumper_cold_object_p (f));
+           else if (!XFLOAT_MARKED_P (f))
+             XFLOAT_MARK (f);
+           break;
+         }
 
        case_Lisp_Int:
          break;
diff --git a/src/lisp.h b/src/lisp.h
index edea7cc23bb..ae78947805e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -2515,8 +2515,13 @@ struct Lisp_Hash_Table
   struct Lisp_Hash_Table *next_weak;
 } GCALIGNED_STRUCT;
 
+/* A specific Lisp_Object that is not a valid Lisp value.
+   We need to be careful not to leak this value into machinery
+   where it may be treated as one; we'd get a segfault if lucky.  */
+#define INVALID_LISP_VALUE make_lisp_ptr (NULL, Lisp_Float)
+
 /* Key value that marks an unused hash table entry.  */
-#define HASH_UNUSED_ENTRY_KEY Qunbound
+#define HASH_UNUSED_ENTRY_KEY INVALID_LISP_VALUE
 
 /* KEY is a key of an unused hash table entry.  */
 INLINE bool
diff --git a/src/pdumper.c b/src/pdumper.c
index 4602931b63a..8d030585c83 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -1337,9 +1337,7 @@ dump_object_needs_dumping_p (Lisp_Object object)
      included in the dump despite all references to them being
      bitwise-invariant.  */
   return (!dump_object_self_representing_p (object)
-         || (dump_object_emacs_ptr (object)
-             /* Don't dump Qunbound -- it's not a legal hash table key.  */
-             && !BASE_EQ (object, Qunbound)));
+         || dump_object_emacs_ptr (object));
 }
 
 static void
@@ -2553,19 +2551,6 @@ dump_symbol (struct dump_context *ctx,
   return offset;
 }
 
-/* Give Qunbound its name.
-   All other symbols are dumped and loaded but not Qunbound because it
-   cannot be used as a key in a hash table.
-   FIXME: A better solution would be to use a value other than Qunbound
-   as a marker for unused entries in hash tables.  */
-static void
-pdumper_init_symbol_unbound (void)
-{
-  eassert (NILP (SYMBOL_NAME (Qunbound)));
-  const char *name = "unbound";
-  init_symbol (Qunbound, make_pure_c_string (name, strlen (name)));
-}
-
 static dump_off
 dump_vectorlike_generic (struct dump_context *ctx,
                         const union vectorlike_header *header)
@@ -5767,8 +5752,6 @@ pdumper_load (const char *dump_filename, char *argv0)
   for (int i = 0; i < nr_dump_hooks; ++i)
     dump_hooks[i] ();
 
-  pdumper_init_symbol_unbound ();
-
 #ifdef HAVE_NATIVE_COMP
   pdumper_set_emacs_execdir (argv0);
 #else
diff --git a/src/print.c b/src/print.c
index c61fb3cd574..c99d8d5fe3a 100644
--- a/src/print.c
+++ b/src/print.c
@@ -1305,8 +1305,7 @@ print (Lisp_Object obj, Lisp_Object printcharfun, bool 
escapeflag)
           || RECORDP (obj)))                              \
    || (! NILP (Vprint_gensym)                             \
        && SYMBOLP (obj)                                           \
-       && !SYMBOL_INTERNED_P (obj)                        \
-       && !hash_unused_entry_key_p (obj)))
+       && !SYMBOL_INTERNED_P (obj)))
 
 /* The print preprocess stack, used to traverse data structures.  */
 
@@ -1392,6 +1391,9 @@ static void
 print_preprocess (Lisp_Object obj)
 {
   eassert (!NILP (Vprint_circle));
+  /* The ppstack may contain HASH_UNUSED_ENTRY_KEY which is an invalid
+     Lisp value.  Make sure that our filter stops us from traversing it.  */
+  eassert (!PRINT_CIRCLE_CANDIDATE_P (HASH_UNUSED_ENTRY_KEY));
   ptrdiff_t base_sp = ppstack.sp;
 
   for (;;)
@@ -1450,6 +1452,8 @@ print_preprocess (Lisp_Object obj)
                    if (HASH_TABLE_P (obj))
                      {
                        struct Lisp_Hash_Table *h = XHASH_TABLE (obj);
+                       /* The values pushed here may include
+                          HASH_UNUSED_ENTRY_KEY; see top of this function.  */
                        pp_stack_push_values (h->key_and_value,
                                              2 * h->table_size);
                      }