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

From: Pip Cet <[email-masked]>
Date: Fri, 03 Jan 2025 17:20:40 +0000

process.c and callproc.c use ALLOCA to store arrays of Lisp_Object
values, or arrays of pointers to string data.

With the current GC, that is correct code only if it's provable that
these objects are marked through some other reference to them (or we
know no GC can happen), because if the last reference is hiding in a
SAFE_ALLOCA'd buffer AND that buffer is not on the C stack, it will be
collected.

With MPS GC, it's even more important to do so, because the object might
otherwise be moved, invalidating the pointer.  This is a possible
explanation for bug#75292.

In either case, I don't immediately see that the current code would be
safe.

SAFE_ALLOCA doesn't call xmalloc very often: it usually uses alloca(),
which results in data on the C stack, where it is visible to both
garbage collectors.  An alternative to fixing this bug in callproc.c and
process.c (and reviewing every other use of SAFE_ALLOCA) would be to
ensure that in the rare case that SAFE_ALLOCA memory is not on the
stack, we'd still conservatively scan it for references.

If we decide to retain the current SAFE_ALLOCA behavior, it is very
important to test builds where SAFE_ALLOCA always uses xmalloc, so we
have a chance to detect such bugs.  Unfortunately, currently bidi.c and
some other places assume that MAX_ALLOCA is "large enough" so this isn't
a simple matter of defining that constant to be 0.

A large stack footprint comes at a cost: it needs to be scanned during
GC, but more importantly there is the risk of latent bugs because stack
pages might not be accessed in the "right" order and the OS might assume
that an excessively-offset access is a program bug rather than an
attempt to allocate large amounts of stack; GCC on GNU/Linux currently
tries to work around this issue by touching each 4096-byte page of the
stack at least once, in the right order.

Note that call_process already allocates more than 64 KB of stack space
unconditionally (in an inner scope, but GCC hoists such allocations to
the function frame).  It may be a good idea not to use SAFE_ALLOCA at
all in this function, unless the 64 KB allocation can be removed.