bug#75322: SAFE_ALLOCA assumed to root Lisp_Objects/SSDATA(string)

["Gerd Möllmann" <gerd.moellmann@gmail.com>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: pipcet@protonmail.com,  75322@debbugs.gnu.org
>> Date: Sat, 04 Jan 2025 09:47:43 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Let's discuss this on a case by case basis.  Not all uses of alloca
>> > are the same or have the same requirements and restrictions.
>> 
>> Okay. For the SDATA case, I find Pip's commit does the right thing. It
>> uses xstrdup for the strings and makes sure these are freed later by
>> registering them in one of the special specpdl entries that exist for
>> that purpose (record_unwind_protect_ptr). Works with both GCs, is always
>> safe, I don't see disadvantages, and we don't have to check if GC runs
>> or not and compact strings (old) or moves string data around (igc).
>
> The disadvantage is to xstrdup strings when that is not needed.  It
> increases memory pressure and also costs some CPU time.  Not very
> significant disadvantage, admittedly, but still.  If this is needed,
> it's a small price to pay, but if it isn't needed, it's a waste.

That's true.

>
> So let's see if we agree that it is indeed needed.  The way to do that
> is to explain how GC could happen while the code which assembles the
> environment in make_environment_block and the code in emacs_spawn
> afterwards run.  Note that we block SIGCHLD and block input while
> emacs_spawn runs.

I'm sorry, but I'm afraid that's not something for me. The possible gains
are so small that I don't want to spend time checking this.

>> For the Lisp_Object cases, I strongly suspect that these cases are an
>> oversight and were left over when SAFE_ALLOCA_LISP was introduced. The
>> reason for introducing SAFE_ALLOCA_LISP is, IMO, what Pip said (old GC):
>> to make sure that the Lisp_Objects are marked, via specpdl stack entries
>> again, when the specpdl stack(s) are marked. Not doing that looks
>> definitely wrong. Replacing the other ALLOCA macros that are currently
>> used for holding Lisp_Objects with SAFE_ALLOCA_LISP would solve things
>> for both GCs.
>
> Can we first identify those cases, so that we could discuss the
> specific codes in question?

Okay. Pip talked about callproc.c and process.c. I believe process.c is
the one with the SDATA.

In callproc.c I found two: call_process and create_temp_file both use
SAFE_NALLOCA to store Lisp_Objects. I think these should be replaces
with SAVE_ALLOCA_LISP.