Subject: Re: bug#75754: styled_format stack usage/GC protection
From: Pip Cet <[email-masked]>
To: Eli Zaretskii <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Mon, 03 Feb 2025 21:02:17 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

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

>> Date: Thu, 23 Jan 2025 23:58:41 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> 1. Reload format_start and format (and end, and format0) after every
>> call which might have GC'd.  If you think we should do this, please
>> tell me whether lisp_string_width can GC.
>
> It can, if called with the last argument 'true' (because
> find_automatic_composition calls into Lisp).  Currently, we call it
> with 'false', so it cannot.
>
>> More importantly, assuming it doesn't, document this in every
>> function in the call tree starting at lisp_string_width so we don't
>> accidentally change it.
>>
>> 2. memcpy the format string.  Two-liner, more likely to fix the bug for
>> good than (1), wastes more memory (since sa_avail has been negative
>> since we entered the function, this is xmalloc'd memory).
>>
>> 3. replace format by a ptrdiff_t and all instances of *format by
>> SREF (args[0], index).  Faster than 2, but many changes hurting
>> readability.
>
> I prefer (2), I think.  Assuming it indeed fixes the problem.

I had to modify it slightly to copy the final NUL character (no harm
done if it isn't used, but it does appear to be relied upon in a few
places).

Pushed now.  Please test, and I'm closing this bug with the following
notes for followup bugs:

1. styled_format still uses excessive amounts of stack, both because it
reserves enough space to print a long double using %f (eating up all of
our sa_avail space on this system), and because it reserves strlen
(format)/2 argument spec slots, rather than simply counting '%'
characters as we copy the string.

2. the crash tests need to be installed once we have a way of presenting
"Emacs crashed" messages nicely for ERT tets.  Alternatively, just wait
a few weeks; people who run new test suites on old Emacs versions can
expect the occasional crash.

3. If anyone wants to avoid copying the format string again (which isn't
really worth it because format strings aren't usually that long), please
ensure that modifying the string from Lisp callbacks in the middle of
the format won't cause crashes.

Thanks!

Pip