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

[Andrea Corallo]

Pip Cet <pipcet@protonmail.com> writes:

> "Andrea Corallo" <acorallo@gnu.org> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> Date: Sat, 04 Jan 2025 11:12:48 +0000
>>>> From: Pip Cet <pipcet@protonmail.com>
>>>> Cc: emacs-devel@gnu.org, stefankangas@gmail.com, acorallo@gnu.org, 
>>>> gerd.moellmann@gmail.com, monnier@iro.umontreal.ca
>>>>
>>>> "Eli Zaretskii" <eliz@gnu.org> writes:
>>>>
>>>> > Is this about bug#74966?  If so, let's not discuss the same issue
>>>>
>>>> No.  This is about the --enable-checking code treating --lambda-fixup as
>>>> a forbidden symbol and crashing if it is used in the obvious fashion by
>>>> any code that happens to be native-compiled, which is the current status
>>>> on scratch/no-purespace.
>>>
>>> Then I'd like to hear Andrea's opinion on this.
>>
>> My opinion is that, to fix the scratch/no-purespace bug, I'm okay with
>> the #$ solution if it works.  Honestly I'd be fine also using a comp
>> specific symbol like comp--something which is documented not to be
>> compilable, maybe less nice but probably less convoluted.  Anyway if Pip
>> prefers #$ and is not too complex I'm okay with that.
>
> Here's the patch.  Okay to push?
>
> (As with all the DEFVARs in comp.c, space for the new symbol is reserved
> in globals, and the documentation ends up in etc/DOC, even in
> non-nativecomp builds.  The same is true for DEFSYM: make-docfile.c
> doesn't care about #ifdef.  If someone fixes this so non-nativecomp
> builds don't pay a price for nativecomp, this code will need fixing,
> too.)
>
> commit a0abd3033414a011125e815d30890ed22b80a84e (HEAD)
> Author: Pip Cet <pipcet@protonmail.com>
> Date:   Tue Jan 7 12:13:40 2025 +0000
>
>     Use #$ for lambda fixups in native compilation data vectors
>     
>     * lisp/emacs-lisp/comp.el (comp--#$): New defvar.
>     (comp--finalize-container): Use it.
>     * src/comp.c (ABI_VERSION): Bump.
>     (emit_static_object): Ensure 'comp--#$' prints as "#$".
>     (load_static_obj): Ensure '#$' reads as Vcomp__hashdollar.
>     (check_comp_unit_relocs): Adjust assertion.
>     (syms_of_comp): Define 'comp--#$'.
>     * src/pdumper.c (dump_do_dump_relocation): Adjust assertion.

The code LGTM, I'd only expand the commit message with a quick
explaination of the mechanism and a link to this discussion.

> +  specpdl_ref count = SPECPDL_INDEX ();
>    static_obj_t *blob =
>      dynlib_sym (comp_u->handle, format_string ("%s_blob", name));
> +  /* Special value so we can recognize #$.  */

Probably this comment should be a little more generous.

Otherwise okay for me for scratch/no-purespace.

Thanks!

  Andrea