bug#56623: memory leak introduced by new nonrecursive lisp reader

["Mattias Engdegård" <mattiase@acm.org>]

It just struck me that you may just be unlucky.

The GC scans the C stack conservatively. The reader rewrite reduced the number of activation records on the stack; instead there is a much more compact marking stack which is scanned precisely, which is both much faster and, well, more precise.

However, in the absence of recursion the size of an on-stack data buffer in the reader was increased substantially (from 64 to 1024) because the danger from stack overflow was gone, and it made for a measurable increase in reading performance.

This means that when the GC now scans the C stack, it has to scan 1024 bytes of typically uninitialised data left over from previous stack activations which could very well include pointers to otherwise dead objects that will now be retained.

A counter-argument is that these undead objects will only be kept artificially alive as long as GC only takes place in the reader. Anyway, the hunch is easily tested: try either or both of these changes:

1. reduce stackbufsize:

--- a/src/lread.c
+++ b/src/lread.c
@@ -2919,7 +2919,7 @@ digit_to_number (int character, int base)
 /* Size of the fixed-size buffer used during reading.
    It should be at least big enough for `invalid_radix_integer' but
    can usefully be much bigger than that.  */
-enum { stackbufsize = 1024 };
+enum { stackbufsize = 64 };
 
 static void
 invalid_radix_integer (EMACS_INT radix, char stackbuf[VLA_ELEMS (stackbufsize)],


2. zero the buffer on entry:

--- a/src/lread.c
+++ b/src/lread.c
@@ -3678,6 +3678,7 @@ read_stack_push (struct read_stack_entry e)
 read0 (Lisp_Object readcharfun, bool locate_syms)
 {
   char stackbuf[stackbufsize];
+  memset (stackbuf, 0, stackbufsize);
   char *read_buffer = stackbuf;
   ptrdiff_t read_buffer_size = sizeof stackbuf;
   char *heapbuf = NULL;


If either makes things better, maybe we can come up with a solution that doesn't hurt performance.