bug#36447: 27.0.50; New "Unknown keyword" errors

[Stefan Monnier]

> A naïve question: wouldn't the problem go away if we modified purecopy
> not to do the above, i.e. not to merge the next vector of a hash table
> with that of another?

It might do the trick, yes (the patch below does that, for example, tho
I think Pip's patch is a better approach).
Note that it would mean we still modify data in the purespace, which
ideally shouldn't happen.

The problem fundamentally is that `purecopy` assumes that the argument
passed to it will never again be modified (it doesn't have to be
immutable before, but it should be afterwards) but if we rehash
afterwards then we break this promise.

>> The (disappointingly trivial) fix:
>> call copy-sequence on h->next before rehashing the table.
> Rehashing is not only done during dumping, right?

The problem is not rehashing in general, but rehashing a purecopied
hash-table.  This should normally never be needed, since a purecopied
hash-table should never be modified (no puthash/remhash should be
applied to it), but sadly pdumper may need to recompute the hashes
because objects's addresses may have changed between the dump and
the reload.

> So the fix you propose also makes rehashing slightly less efficient.

In my patch I added a comment to explain that hash_table_rehash is not
used in "normal" rehashing, but only in the rare case of rehashing on
the first access to a preloaded hash-table, so the efficiency
His patch looks good (probably better than mine).

His patch can/should be made slightly more efficient by only doing the
Fcopy_sequence on those hash-tables that are in purespace.


        Stefan


diff --git a/src/fns.c b/src/fns.c
index 2fc000a7f4..cc4fd7f2d7 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4218,6 +4218,11 @@ maybe_resize_hash_table (struct Lisp_Hash_Table *h)
     }
 }
 
+/* Recompute the hashes (and hence also the "next" pointers).
+   Normally there's never a need to recompute hashes
+   This is done only on first-access to a hash-table loaded from
+   the "pdump", because the object's addresses may have changed, thus
+   affecting their hash.  */
 void
 hash_table_rehash (struct Lisp_Hash_Table *h)
 {
diff --git a/src/alloc.c b/src/alloc.c
index 64aaa8acdf..b0114a9459 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -5348,9 +5348,24 @@ purecopy_hash_table (struct Lisp_Hash_Table *table)
 
   pure->header = table->header;
   pure->weak = purecopy (Qnil);
+  /* After reloading the pdumped data, objects may ehave changed location
+     in memory, so their hash may have changed.  So the `hash`, `next`, and
+     `index` vectors are not immutable and can't safely be hash-cons'ed.  */
+  if (HASH_TABLE_P (Vpurify_flag))
+    {
+      Fremhash (table->hash, Vpurify_flag);
+      Fremhash (table->next, Vpurify_flag);
+      Fremhash (table->index, Vpurify_flag);
+    }
   pure->hash = purecopy (table->hash);
   pure->next = purecopy (table->next);
   pure->index = purecopy (table->index);
+  if (HASH_TABLE_P (Vpurify_flag))
+    {
+      Fremhash (table->hash, Vpurify_flag);
+      Fremhash (table->next, Vpurify_flag);
+      Fremhash (table->index, Vpurify_flag);
+    }
   pure->count = table->count;
   pure->next_free = table->next_free;
   pure->pure = table->pure;