GNU bug report logs - #75322
SAFE_ALLOCA assumed to root Lisp_Objects/SSDATA(string)

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>, 75322 <at> debbugs.gnu.org
Subject: bug#75322: SAFE_ALLOCA assumed to root Lisp_Objects/SSDATA(string)
Date: Sat, 04 Jan 2025 11:41:20 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
>> Cc: pipcet <at> protonmail.com,  75322 <at> debbugs.gnu.org
>> Date: Sat, 04 Jan 2025 09:47:43 +0100
>>
>> Eli Zaretskii <eliz <at> 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.

It's easier to xstrdup rather than figure out which no-GC assumptions
are in the subprocess code, which of them are valid (probably not all of
them, considering Fexpand_file_name), and how to document them in a way
that doesn't turn this area of code into more of a minefield than it is
already :-)

> 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 not going to explain "how GC could happen" in the MPS case: GC can
strike at any time.

For traditional GC, if we want to change the code as little as possible,
we could indeed try to establish no-GC assumptions, but we don't have
to: we can simply start with correct but slow code and maybe drop a note
to the usual suspects that there is optimization potential here.

>> 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?

TBH, if there is a single bug caused by an incorrect SAFE_ALLOCA which
should be a SAFE_ALLOCA_LISP, I think that's reason enough to modify
SAFE_ALLOCA to conservatively scan any xmalloc'd memory it uses.  No
performance impact as this case is nonexistent in practice.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.