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

From: Pip Cet <[email-masked]>
Date: Thu, 23 Jan 2025 19:32:44 +0000

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

>> Date: Thu, 23 Jan 2025 09:24:12 -0800
>> Cc: [email-masked]
>> From: Paul Eggert <[email-masked]>
>>
>> On 2025-01-22 21:47, Eli Zaretskii wrote:
>> > Paul, any comments on the issue and/or the patch?
>>
>> Patch looks OK except for the "Ose" in a comment.
>
> Thanks.
>
> I think it would be good to have a test for this, if that is feasible.

While I don't think a test would be a bad thing per se (feel free to use
the code I posted, of course), I don't think it's the most sensible use
of resources here: the precise situation is unlikely to occur again
unless the patch is reverted, and similar bugs wouldn't be caught by
this test.

The most worrying aspect of this wasn't that there was a GC bug and we
used a free'd string, it was that we didn't crash at that point, but
silently treated the string as empty, but that's for bug#75768.

And while I do think we should revisit the safety of SAFE_ALLOCA
(scanning the extra memory as though it were on the stack seems easy,
but I haven't tried) and SDATA (pinning the sblocks in memory is doable,
I've got a patch, but there are subtle performance issue), those also
could benefit from broader discussion.  At the very least, a separate
bug report.

(In both of these cases, the feature/igc branch currently chooses the
safer approach; in the case of SDATA, it must.  In the case of
SAFE_ALLOCA, we could introduce an unsafe version for some situations in
which we are absolutely convinced no Lisp data can ever live in the
xmalloc'd block, but so far it's not been required for performance.

Many places use SDATA in situations where they expect no GC can happen,
and there are no comments or macros indicating this to us, so deciding
whether this is one of the cases is impossible.  I mention this because
this is likely to be the last such bug we find because it was buggy on
both branches but caused problems only on feature/igc.)

So, assuming no one volunteers to write a specific test, let's close
this bug.

Pip