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

From: Pip Cet <[email-masked]>
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.