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

From: Pip Cet <[email-masked]>
Date: Sat, 25 Jan 2025 18:20:40 +0000

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

>> Date: Sat, 25 Jan 2025 09:45:19 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:

>> If there's modification involved, it's a bug which will cause apparently
>> random behavior but no crashes.  If there's no modification, it's a bug
>> only because the code might be used with alloc.c GC.
>>
>> >> >   Lisp_Object my_string;
>> >> >   char *p = SDATA (my_string);
>> >> >   /* trigger GC */

This line is not important.  GC is transparent to client code like this,
which doesn't violate the rules by hiding a pointer somewhere.

>> >> >   Faset (my_string, make_fixnum (0), make_fixnum (97));
          ^^^^^

This is the important part.  Faset can and does reallocate string data
unpredictably.

>> >> >   /* code which expects p[0] to be 'a' */
>> >>
>> >> If that is all the code, this is safe with feature/igc.
>> >
>> > You say "safe", but it sounds like we have different definitions of
>> > "safe".  Because later you say:
>>
>> I meant "won't crash, but most likely useless".
>>
>> >> MPS GC will never make pointers that live on the stack invalid.  It's
>> >> transparent to correct client programs.  So, yes, if you're asking about
>> >> MPS GC, the sequence is safe and doesn't crash, but may surprise you by
>> >> reproducing the old string data.
>> >
>> > Which seems to mean your "safe" means the code will not crash, whereas
>> > what I meant is the correctness of the code.  Code which refers to the
>> > old string data is incorrect, even if it won't crash.
>>
>> I believe that's true.
>
> The above confirms what I thought about this, thanks.

The "sequence" I mentioned above included a string modification, which
invalidates the SDATA pointer (sometimes).  GC never does, so its
inclusion in the sequence was irrelevant.

>> > So my conclusion is that even with MPS, referring in code after GC to
>> > an SDATA pointer taken before GC leads to incorrect code.  IOW,
>> > pointers to SDATA of a string cannot be relied upon to be valid
>> > (i.e. pointing to the correct string) across GC, so the code must
>> > re-initialize such pointer after any fragment that could GC.  This was
>> > the rule with the old GC, and it should continue to be the rule with
>> > MPS.
>>
>> No.
>
> And this seems to contradict it.

Here, you were talking ONLY about GC, the irrelevant part.  GC never
invalidates an SDATA pointer.  Only string modification does.

>> GC doesn't invalidate SDATA pointers.
>
> I never said anything about invalidating the SDATA pointers.  So if

Then they're valid.  Period.

Pip