Subject: bug#78590: 31.0.50; print_object calls strout unsafely

From: Pip Cet <[email-masked]>
Date: Tue, 27 May 2025 18:05:00 +0000

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

>> Date: Mon, 26 May 2025 18:00:28 +0000
>> From:  Pip Cet via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>>
>> Pip Cet <[email-masked]> writes:
>>
>> > It's probably not worth it to come up with a complicated fix here: let's
>> > just use SAFE_ALLOCA_STRING, and add a comment explaining the reasons
>> > (GC, Lisp code modifying the string that is being printed).
>>
>> OK for master?
>
> I agree that the current code is buggy because it keeps referencing a
> Lisp string's data via a 'char *' pointer while calling a Lisp
> function PRINTCHARFUN.

How about calling Fmapc (printcharfun, string) at that point, from
print_string?  That avoids making any assumptions about our string, and
will be as resilient as mapc is against unreasonable self-modification
of strings (bug#75845).

> How about replacing iteration over a C 'char *' buffer with iteration
> over the original string (using SREF or string_char_and_length)?  That
> is, instead of
>
> 	      int len, ch = (string_char_and_length
> 			     ((const unsigned char *) ptr + i, &len));
> use
>
> 	      int len, ch = (string_char_and_length
> 			     (SDATA (string) + i, &len));
>
> where 'string' is the original Lisp string to print?

That's what print_string does (even though it should be more careful,
which is most easily achieved by delegating to Fmapc and fixing that).
It does, however, also transform the string by escaping some characters,
which seems incorrect to me in these circumstances (and would mean
changing existing behavior).

> If this requires adding an additional argument to strout, to pass the
> Lisp string, which will be used only in the last 'else' clause there,
> that's not difficult, right?

But we already have print_string, which does almost exactly that, right?
All it needs is an extra argument to avoid the byte8 escaping (unless
I'm wrong and we actually want that to happen to our string here).

Pip