Subject: bug#75754: styled_format stack usage/GC protection

From: Pip Cet <[email-masked]>
Date: Fri, 24 Jan 2025 13:21:24 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Thu, 23 Jan 2025 22:37:43 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> > What did I miss?
>>
>> Can we make this a new bug?
>
> We could, or we could keep discussing that in this bug (since this is
> still about styled_format).

Let's do the latter, too much discussion happened here already and the
subject does kind of cover this bug, too (as well as the excessive stack
usage which remains unfixed; I cannot easily fix it without a lot of
self-study or Paul Eggert's help).

>> This one is SDATA, not SAFE_ALLOCA.
>>
>> diff --git a/src/editfns.c b/src/editfns.c
>> index 4ba356d627c..23a5f9aeac6 100644
>> --- a/src/editfns.c
>> +++ b/src/editfns.c
>> @@ -3491,7 +3491,7 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
>>    /* If we start out planning a unibyte result,
>>       then discover it has to be multibyte, we jump back to retry.  */
>>   retry:
>> -
>> +  format_start = SSDATA (args[0]);
>>    p = buf;
>>    nchars = 0;
>>
>>
>> should fix it.
>
> Didn't try it yet, since we are still discussing what to do.  But I
> would like to install the test, for now as expected to fail.  WDYT?

Please don't.  The test without the bugfix has absolutely undefined
behavior and will most likely crash Emacs in some circumstances, and
might cause other tests to produce incorrect results as well.

Let's wait with adding such tests until we have ERT infrastructure to
deal with them as gracefully as we can (crash tests need to be run in
timeout, with a ulimit, so this involves Makefile work, too).

(Stefan Kangas: I do recall you asked me to sketch how ERT changes could
improve the output of "make check" in cases where Emacs crashed, in
bug#75648.  As Michael rejected the initial approach I had taken, I
decided to delay this until I could make a new proposal that hadn't been
rejected already.  This bug now looks like it may result in one).

Pip