Subject: Missing assertions to detect access to GC-freed strings

From: Pip Cet <[email-masked]>
Date: Wed, 22 Jan 2025 20:11:58 +0000

This is a spin-off from bug#75754.  I produced an example program to
demonstrate the correctness bug described therein:

(setq print-unreadable-function
      (lambda (&rest args)
        (garbage-collect)
        (make-string 100 ?θ)))

(message "%S" (format "%S %S" [1] (symbol-function '+)))

This program sometimes completes without throwing an error or causing a
segfault on the master branch, but produces incorrect output: the
correct output starts with "[1] "; the incorrect output omits the "[1]".

This is because the freed string looks like this:

(gdb) p info[0].argument
$7 = XIL(0x555555fdcea4)
(gdb) pp $
#<INVALID_LISP_OBJECT 0x555555fdcea4>
(gdb) p XSTRING($7)
$8 = (struct Lisp_String *) 0x555555fdcea0
(gdb) p *$
$9 = {
  u = {
    s = {
      size = 0,
      size_byte = -1,
      intervals = 0x0,
      data = 0x0
    },
    next = 0x0,
    gcaligned = 0 '\000'
  }
}

This is obviously invalid; as Emacs strings include a trailing NUL byte,
their data pointer must never be NULL.  However, as the size field is 0
(this is invalid for all but the pair of empty strings), we never
attempt to dereference the NULL pointer, and that means we do not abort
or crash soon enough.

This needs to be fixed.  GC bugs usually lead to segmentation faults and
crashes; producing incorrect output and silently corrupting user data is
much, much worse.

I'm proposing to add checks, at least temporarily, to the
--enable-checking builds: these checks would ensure that a string
contains a non-NULL data field if it is accessed in any way.

Even with this patch, we would fail to recognize impossible strings
automatically if even a single bit in the data pointer is set.  I don't
have a better solution yet, though, but the patch (to follow as soon as
this has a bug number) would have detected this particular problem a
little sooner.