GNU bug report logs - #78590
31.0.50; print_object calls strout unsafely

Previous Next

Package: emacs;

Reported by: Pip Cet <pipcet <at> protonmail.com>

Date: Mon, 26 May 2025 13:17:02 UTC

Severity: normal

Found in version 31.0.50

To reply to this bug, email your comments to 78590 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Mon, 26 May 2025 13:17:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Pip Cet <pipcet <at> protonmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 26 May 2025 13:17:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; print_object calls strout unsafely
Date: Mon, 26 May 2025 13:16:08 +0000
This is a bug of the "SDATA used after potential GC" type in print.c.
It requires using the print-number-table feature and a custom
printcharfun.

When print_object finds a string (not a fixnum or t) in the
print-number-table, it calls:

	  strout (SSDATA (num), SCHARS (num), SBYTES (num), printcharfun);

This assumes num (which is a string) isn't relocated in the middle of
printcharfun, but printcharfun is an arbitrary Lisp function, which can
trigger garbage collection and relocate the string.

It's a bit hard to reproduce this bug because recursive print
invocations are currently broken and crash Emacs, but this works:

;; -*- lexical-binding: t; -*-
(let ((print-circle t)
      (print-number-table (make-hash-table))
      (print-continuous-numbering t)
      (str "yy")
      (outstr ""))
  (garbage-collect)
  (make-string 100 ?a)
  (puthash str (make-string 3 ?x) print-number-table)
  (prin1 str
         (lambda (c)
           (setq outstr (concat outstr (string c)))
           (garbage-collect)
           (make-string 100 ?b)))
  (message "outstr %S" outstr))

The expected output is: outstr "xxx"
The actual output (on this machine) is: outstr "xbb"

After the first character is printed (appended to outstr), printcharfun
calls garbage-collect, which relocates the string we're printing.  The
string's data pointer now points to unallocated space which is reused by
(make-string 100 ?b) and filled with 'b' rather than 'x'.  We continue
to print the incorrect string.

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).

All other calls to strout look safe.

The other usages of SDATA/SSDATA in print.c look okay at first glance,
with this exception:

static void
print_unwind (Lisp_Object saved_text)
{
  memcpy (print_buffer.buffer, SDATA (saved_text), SCHARS (saved_text));
}

That should probably be SBYTES (saved_text), but recursive print
invocations don't work, and this particular call won't do anything
useful because print_buffer.pos and print_buffer.pos_byte aren't
restored, as far as I can see.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Mon, 26 May 2025 18:01:01 GMT) Full text and rfc822 format available.

Message #8 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: 78590 <at> debbugs.gnu.org
Subject: Re: 31.0.50; print_object calls strout unsafely
Date: Mon, 26 May 2025 18:00:28 +0000
Pip Cet <pipcet <at> protonmail.com> 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?

From a8f2d405f704bbff8014cf82c7c82f4d3f85fc40 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet <at> protonmail.com>
Subject: [PATCH] Fix unsafe SDATA usage in print.c (bug#78590)

* src/print.c (print_object): Avoid unsafe SDATA usage;
create a copy of the string instead.
* test/src/print-tests.el (test-print-number-realloc):
New test.
---
 src/print.c             |  9 ++++++++-
 test/src/print-tests.el | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/print.c b/src/print.c
index b17ec337f70..0c7a630702c 100644
--- a/src/print.c
+++ b/src/print.c
@@ -2282,7 +2282,14 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 	}
       else if (STRINGP (num))
 	{
-	  strout (SSDATA (num), SCHARS (num), SBYTES (num), printcharfun);
+	  /* Use a local copy of the string, to guard against GC
+	   * relocation and Lisp code modifying the string being
+	   * printed.  */
+	  char *ptr;
+	  USE_SAFE_ALLOCA;
+	  SAFE_ALLOCA_STRING (ptr, num);
+	  strout (ptr, SCHARS (num), SBYTES (num), printcharfun);
+	  SAFE_FREE ();
 	  goto next_obj;
 	}
     }
diff --git a/test/src/print-tests.el b/test/src/print-tests.el
index af57311135b..036248fd091 100644
--- a/test/src/print-tests.el
+++ b/test/src/print-tests.el
@@ -540,5 +540,23 @@ test-print-unreadable-function-buffer
       (should (eq callback-buffer buffer))
       (should (equal str "tata"))))
 
+(ert-deftest test-print-number-realloc ()
+  ;; Test for bug#78590.  Note that this may in rare cases crash unfixed
+  ;; Emacs versions.
+  (let ((print-circle t)
+        (print-number-table (make-hash-table))
+        (print-continuous-numbering t)
+        (str "yy")
+        (outstr ""))
+    (garbage-collect)
+    (ignore (make-string 100 ?a))
+    (puthash str (make-string 3 ?x) print-number-table)
+    (prin1 str
+           (lambda (c)
+             (setq outstr (concat outstr (string c)))
+             (garbage-collect)
+             (ignore (make-string 100 ?b))))
+    (should (equal outstr "xxx"))))
+
 (provide 'print-tests)
 ;;; print-tests.el ends here
-- 
2.48.1





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Mon, 26 May 2025 18:49:01 GMT) Full text and rfc822 format available.

Message #11 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Mon, 26 May 2025 21:48:18 +0300
> 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" <bug-gnu-emacs <at> gnu.org>
> 
> Pip Cet <pipcet <at> protonmail.com> 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.  But I wonder whether the best solution is to
copy the string to a C buffer.  E.g., what if the string is very long?
we'd just have two copies of it during the print, which increases
memory pressure.

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?  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?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Tue, 27 May 2025 18:06:02 GMT) Full text and rfc822 format available.

Message #14 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Tue, 27 May 2025 18:05:00 +0000
"Eli Zaretskii" <eliz <at> gnu.org> 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" <bug-gnu-emacs <at> gnu.org>
>>
>> Pip Cet <pipcet <at> protonmail.com> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Tue, 27 May 2025 18:29:02 GMT) Full text and rfc822 format available.

Message #17 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Tue, 27 May 2025 21:27:43 +0300
> Date: Tue, 27 May 2025 18:05:00 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 78590 <at> debbugs.gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > 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?

If that works, fine.  But didn't you say below that print_string does
something we don't want to do in this case?  Or am I misunderstanding
what you mean?

> > 	      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).

Yes.

> 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).

So calling print_string as it is will not do what we want, right?

> > 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).

Sure, that works as well, I think (though I took only a cursory look
at print_string, so maybe I missed something).

The important part, I think, is to avoid duplicating the string if we
can avoid that.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Tue, 27 May 2025 18:47:02 GMT) Full text and rfc822 format available.

Message #20 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Tue, 27 May 2025 18:46:24 +0000
"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Tue, 27 May 2025 18:05:00 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 78590 <at> debbugs.gnu.org
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > 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?
>
> If that works, fine.  But didn't you say below that print_string does
> something we don't want to do in this case?  Or am I misunderstanding
> what you mean?

Sorry, that wasn't quite clear.  I also forgot to include the proposed
patch:

From 43a27ab64584d98cf10a3f1616701ceae0cdb510 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet <at> protonmail.com>
Subject: [PATCH 1/2] Fix unsafe SDATA usage in print.c (bug#78590)

* src/print.c (print_string_1): Renamed from 'print_string', with an
extra argument to disable nonascii escaping.  Call 'Fmapc' when passed
a Lisp function.
(print_string): New function.
(print_object): Use 'print_string_1', not 'strout'.
---
 src/print.c             | 33 ++++++++++++---------------------
 test/src/print-tests.el | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/src/print.c b/src/print.c
index b17ec337f70..cd8316a4048 100644
--- a/src/print.c
+++ b/src/print.c
@@ -469,18 +469,18 @@ strout (const char *ptr, ptrdiff_t size, ptrdiff_t size_byte,
    because printing one char can relocate.  */
 
 static void
-print_string (Lisp_Object string, Lisp_Object printcharfun)
+print_string_1 (Lisp_Object string, Lisp_Object printcharfun, bool escape_nonascii)
 {
   if (EQ (printcharfun, Qt) || NILP (printcharfun))
     {
       ptrdiff_t chars;
 
-      if (print_escape_nonascii)
+      if (escape_nonascii)
 	string = string_escape_byte8 (string);
 
       if (STRING_MULTIBYTE (string))
 	chars = SCHARS (string);
-      else if (! print_escape_nonascii
+      else if (! escape_nonascii
 	       && (EQ (printcharfun, Qt)
 		   ? ! NILP (BVAR (&buffer_defaults, enable_multibyte_characters))
 		   : ! NILP (BVAR (current_buffer, enable_multibyte_characters))))
@@ -524,25 +524,16 @@ print_string (Lisp_Object string, Lisp_Object printcharfun)
     }
   else
     {
-      /* Otherwise, string may be relocated by printing one char.
-	 So re-fetch the string address for each character.  */
-      ptrdiff_t i;
-      ptrdiff_t size = SCHARS (string);
-      ptrdiff_t size_byte = SBYTES (string);
-      if (size == size_byte)
-	for (i = 0; i < size; i++)
-	  printchar (SREF (string, i), printcharfun);
-      else
-	for (i = 0; i < size_byte; )
-	  {
-	    /* Here, we must convert each multi-byte form to the
-	       corresponding character code before handing it to PRINTCHAR.  */
-	    int len, ch = string_char_and_length (SDATA (string) + i, &len);
-	    printchar (ch, printcharfun);
-	    i += len;
-	  }
+      /* printcharfun is a Lisp function.  */
+      Fmapc (printcharfun, string);
     }
 }
+
+static void
+print_string (Lisp_Object string, Lisp_Object printcharfun)
+{
+  print_string_1 (string, printcharfun, print_escape_nonascii);
+}
 
 DEFUN ("write-char", Fwrite_char, Swrite_char, 1, 2, 0,
        doc: /* Output character CHARACTER to stream PRINTCHARFUN.
@@ -2282,7 +2273,7 @@ print_object (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag)
 	}
       else if (STRINGP (num))
 	{
-	  strout (SSDATA (num), SCHARS (num), SBYTES (num), printcharfun);
+	  print_string_1 (num, printcharfun, false);
 	  goto next_obj;
 	}
     }
diff --git a/test/src/print-tests.el b/test/src/print-tests.el
index af57311135b..036248fd091 100644
--- a/test/src/print-tests.el
+++ b/test/src/print-tests.el
@@ -540,5 +540,23 @@ test-print-unreadable-function-buffer
       (should (eq callback-buffer buffer))
       (should (equal str "tata"))))
 
+(ert-deftest test-print-number-realloc ()
+  ;; Test for bug#78590.  Note that this may in rare cases crash unfixed
+  ;; Emacs versions.
+  (let ((print-circle t)
+        (print-number-table (make-hash-table))
+        (print-continuous-numbering t)
+        (str "yy")
+        (outstr ""))
+    (garbage-collect)
+    (ignore (make-string 100 ?a))
+    (puthash str (make-string 3 ?x) print-number-table)
+    (prin1 str
+           (lambda (c)
+             (setq outstr (concat outstr (string c)))
+             (garbage-collect)
+             (ignore (make-string 100 ?b))))
+    (should (equal outstr "xxx"))))
+
 (provide 'print-tests)
 ;;; print-tests.el ends here
-- 
2.48.1

>> 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).
>
> So calling print_string as it is will not do what we want, right?

The new function print_string_1 has an extra flag parameter; all
existing callers set it to print_escape_nonascii by calling print_string
as before, the new caller directly calls print_string_1 and forcibly
disables byte8 escaping (and enables multibyte conversion where
necessary).

>> > 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).
>
> Sure, that works as well, I think (though I took only a cursory look
> at print_string, so maybe I missed something).
>
> The important part, I think, is to avoid duplicating the string if we
> can avoid that.

I believe you're right.  (It's probably okay to copy strings when
printcharfun is Qt, redisplay will be much slower than creating the
copy, but when output isn't for the echo area there's no reason to
believe the replacement strings are short).

Pip





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Wed, 28 May 2025 11:25:03 GMT) Full text and rfc822 format available.

Message #23 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Wed, 28 May 2025 14:24:38 +0300
> Date: Tue, 27 May 2025 18:46:24 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 78590 <at> debbugs.gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> How about calling Fmapc (printcharfun, string) at that point, from
> >> print_string?
> >
> > If that works, fine.  But didn't you say below that print_string does
> > something we don't want to do in this case?  Or am I misunderstanding
> > what you mean?
> 
> Sorry, that wasn't quite clear.  I also forgot to include the proposed
> patch:

Ah, okay, there was indeed an misunderstanding, see below.

>    else
>      {
> -      /* Otherwise, string may be relocated by printing one char.
> -	 So re-fetch the string address for each character.  */
> -      ptrdiff_t i;
> -      ptrdiff_t size = SCHARS (string);
> -      ptrdiff_t size_byte = SBYTES (string);
> -      if (size == size_byte)
> -	for (i = 0; i < size; i++)
> -	  printchar (SREF (string, i), printcharfun);
> -      else
> -	for (i = 0; i < size_byte; )
> -	  {
> -	    /* Here, we must convert each multi-byte form to the
> -	       corresponding character code before handing it to PRINTCHAR.  */
> -	    int len, ch = string_char_and_length (SDATA (string) + i, &len);
> -	    printchar (ch, printcharfun);
> -	    i += len;
> -	  }
> +      /* printcharfun is a Lisp function.  */
> +      Fmapc (printcharfun, string);

What is the rationale for calling Fmapc here?  Doesn't it do in this
case what the original code did, but at a price of two additional
levels of function calls?  IOW, why did you want to get rid of the
original code above?

I have no other issues with your suggested patch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Wed, 28 May 2025 13:45:08 GMT) Full text and rfc822 format available.

Message #26 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Wed, 28 May 2025 13:44:32 +0000
"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Tue, 27 May 2025 18:46:24 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: 78590 <at> debbugs.gnu.org
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>>    else
>>      {
>> -      /* Otherwise, string may be relocated by printing one char.
>> -	 So re-fetch the string address for each character.  */
>> -      ptrdiff_t i;
>> -      ptrdiff_t size = SCHARS (string);
>> -      ptrdiff_t size_byte = SBYTES (string);
>> -      if (size == size_byte)
>> -	for (i = 0; i < size; i++)
>> -	  printchar (SREF (string, i), printcharfun);
>> -      else
>> -	for (i = 0; i < size_byte; )
>> -	  {
>> -	    /* Here, we must convert each multi-byte form to the
>> -	       corresponding character code before handing it to PRINTCHAR.  */
>> -	    int len, ch = string_char_and_length (SDATA (string) + i, &len);
>> -	    printchar (ch, printcharfun);
>> -	    i += len;
>> -	  }
>> +      /* printcharfun is a Lisp function.  */
>> +      Fmapc (printcharfun, string);
>
> What is the rationale for calling Fmapc here?  Doesn't it do in this
> case what the original code did, but at a price of two additional
> levels of function calls?  IOW, why did you want to get rid of the
> original code above?

Let's leave out that hunk for now?  We can replace it by Fmapc when
Fmapc is fixed for bug#75845.  I see calling mapc on a record crashes
now, so I'll try to come up with an updated patch for bug#75845 that
fixes both that issue and the self-modification thing.

Pip





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#78590; Package emacs. (Wed, 28 May 2025 13:55:08 GMT) Full text and rfc822 format available.

Message #29 received at 78590 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78590 <at> debbugs.gnu.org
Subject: Re: bug#78590: 31.0.50; print_object calls strout unsafely
Date: Wed, 28 May 2025 16:54:07 +0300
> Date: Wed, 28 May 2025 13:44:32 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 78590 <at> debbugs.gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> Date: Tue, 27 May 2025 18:46:24 +0000
> >> From: Pip Cet <pipcet <at> protonmail.com>
> >> Cc: 78590 <at> debbugs.gnu.org
> >>
> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> >>
> >>    else
> >>      {
> >> -      /* Otherwise, string may be relocated by printing one char.
> >> -	 So re-fetch the string address for each character.  */
> >> -      ptrdiff_t i;
> >> -      ptrdiff_t size = SCHARS (string);
> >> -      ptrdiff_t size_byte = SBYTES (string);
> >> -      if (size == size_byte)
> >> -	for (i = 0; i < size; i++)
> >> -	  printchar (SREF (string, i), printcharfun);
> >> -      else
> >> -	for (i = 0; i < size_byte; )
> >> -	  {
> >> -	    /* Here, we must convert each multi-byte form to the
> >> -	       corresponding character code before handing it to PRINTCHAR.  */
> >> -	    int len, ch = string_char_and_length (SDATA (string) + i, &len);
> >> -	    printchar (ch, printcharfun);
> >> -	    i += len;
> >> -	  }
> >> +      /* printcharfun is a Lisp function.  */
> >> +      Fmapc (printcharfun, string);
> >
> > What is the rationale for calling Fmapc here?  Doesn't it do in this
> > case what the original code did, but at a price of two additional
> > levels of function calls?  IOW, why did you want to get rid of the
> > original code above?
> 
> Let's leave out that hunk for now?  We can replace it by Fmapc when
> Fmapc is fixed for bug#75845.  I see calling mapc on a record crashes
> now, so I'll try to come up with an updated patch for bug#75845 that
> fixes both that issue and the self-modification thing.

Fine by me, thanks.

So feel free to install on the master branch.




This bug report was last modified 18 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.