Re: Merging scratch/no-purespace to remove unexec and purespace

[Pip Cet]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Andrea Corallo <acorallo@gnu.org> writes:
>
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>
>>>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>>>> Cc: emacs-devel@gnu.org,  Pip Cet <pipcet@protonmail.com>,  Stefan Monnier
>>>>>  <monnier@iro.umontreal.ca>
>>>>> Date: Tue, 17 Dec 2024 18:56:05 +0100
>>>>>
>>>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>>>>
>>>>> > Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>>>> >
>>>>> >> I think check_comp_unit_relocs should be removed in the branch. What's
>>>>> >> left of it the branch, checks in master if everything has been
>>>>> >> put in purespace which should be there. IIUC correctly, of course.
>>>>> >
>>>>> > Patch attached.
>>>>>
>>>>> Pushed. Complaints to me please.
>>>>
>>>> I'd like at least Andrea to take a look and confirm.
>>>
>>> It's 81fc23b5d6a60ca4f3d269ab2c88eb9a850bac4c
>>
>> Hi Gerd,
>>
>> looking at the commit now, why do you think 'check_comp_unit_relocs'
>> should be removed?
>>
>> Even if now the situation is simpler 'check_comp_unit_relocs' is still
>> performing some sanity checks like if lambdas are all been fixed-up and
>> present in 'comp_u->lambda_gc_guard_h'.
>>
>>   Andrea
>
> Hi Andrea.
>
> The check you mention checks something that I don't see how it could
> happen. (With the usual disclaimers, because it's been some time since I
> was in that code for igc.)

I'm confused, I thought you did hit the assertion?

AFAICT, the problem is simply that comp.el uses the symbol lambda-fixup.

So a relocation for that symbol is emitted.  But no fixup is, because
this isn't an actual lambda, it's merely the symbol.  The debug code
then sees "lambda-fixup", assumes it's a failed fixup, and asserts.

IOW, the old code happened not to run into this problem because
lambda-fixup was pure, and we never applied the sanity checks to the
pure section.

This "fix" appears to work:

diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index dbd14b2740d..5d364e77e66 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -3254,7 +3254,7 @@ comp--finalize-container
                  ;; from the corresponding m-var.
                  collect (if (gethash obj
                                       (comp-ctxt-byte-func-to-func-h 
comp-ctxt))
-                             'lambda-fixup
+                             (intern (concat "lambda" (make-string 1 ?-) 
"fixup"))
                            obj))))
 
 (defun comp--finalize-relocs ()

My suggestion is to fix the "sanity check" on the master branch, change
it to apply to pure relocs there, and restore the fixed check on
scratch/no-purespace afterwards.

Pip