POSIX:FILE-STAT: store into a cons that a GC inside check_chmod_mode_to_list() may have moved

In modules/syscalls/calls.c, file_stat_to_STACK() builds the mode slot like this (master db78dbd2, line 1896; the same code is in 2.49.92):

      pushSTACK(allocate_cons());
      Car(STACK_0) = mknod_type_check_reverse(fmt);
      Cdr(STACK_0) = check_chmod_mode_to_list(ps->st_mode & ~S_IFMT);

The order in which the two sides of the last assignment are evaluated is unspecified, and GCC 14.2 (Debian trixie, x86_64) computes the address of Cdr(STACK_0) first: it loads STACK_0 into a register, calls check_chmod_mode_to_list(), which conses through map_c_to_list(), and then stores through the register it loaded before the call:

   f9f13:  mov    -0x8(%rax),%rbx              ; STACK_0, the cons
   f9f27:  call   1ef048 <map_c_to_list>
   f9f2c:  mov    %rax,-0x3(%rbx)              ; Cdr(old cons) = list

When a GC runs inside that call and moves the cons, the list is stored at the cons' old address.

We first saw it as a SIGSEGV in ASDF's upgrade test in CI (UIOP's probe-file* calls posix:file-stat on CLISP), always at the same point of the run. The kernel's report gave the faulting instruction as offset 0xf9f2c in base/lisp.run, which is the store above. It does not need ASDF:

(defun file-stat-gc (&optional (n 3000000))
  (let* ((f "/tmp/file-stat-gc.tmp")
         (good (progn (close (open f :direction :output :if-exists :supersede))
                      (posix:file-stat-mode (posix:file-stat f))))
         (bad 0) (junk nil))
    (dotimes (i n)
      (dotimes (k 40) (push k junk))
      (when (zerop (mod i 50000)) (setq junk nil))
      (unless (equal (posix:file-stat-mode (posix:file-stat f)) good)
        (incf bad)))
    (format t "~&~D FILE-STATs, ~D with a wrong mode~%" n bad)))
(compile 'file-stat-gc)
(time (file-stat-gc))

Results, clisp -norc -q file-stat-gc.lisp, x86_64, Debian trixie, GCC 14.2:

  • master db78dbd2, configured --with-threads=POSIX_THREADS --with-unicode --with-module=rawsock --with-module=bindings/glibc --with-module=zlib --with-module=asdf: Segmentation fault within about a second, in each of three runs.
  • 2.49.92, the same options plus --enable-portability: no crash, but some results come back without their mode list (59 of 3,000,000 in one run).
  • master with the patch below: 3,000,000 calls, 505 GCs, no crash and no wrong mode, in each of three runs; 2.49.92 with the patch likewise.
--- a/modules/syscalls/calls.c
+++ b/modules/syscalls/calls.c
@@ -1893,7 +1893,11 @@
     if (fmt) {
       pushSTACK(allocate_cons());
       Car(STACK_0) = mknod_type_check_reverse(fmt);
-      Cdr(STACK_0) = check_chmod_mode_to_list(ps->st_mode & ~S_IFMT);
+      { /* check_chmod_mode_to_list() can GC and move the cons, so call it
+           before the cons' address is taken */
+        object mode = check_chmod_mode_to_list(ps->st_mode & ~S_IFMT);
+        Cdr(STACK_0) = mode;
+      }
     } else pushSTACK(check_chmod_mode_to_list(ps->st_mode));
   }
 #else

The same shape -- a heap place on the left, a call that may allocate on the right -- also appears at these places in master. We have not seen them fail and have not checked how GCC compiles them:

  • src/pathname.d:8969 Car(STACK_0) = check_string(Car(STACK_0));
  • modules/pcre/cpcre.c:250 Car(STACK_0) = asciz_to_string(...);
  • modules/rawsock/rawsock.c:531-532 uint_to_I(...), asciz_to_string(...)
  • modules/dirkey/dirkey.c:175 n_char_to_string(...)
  • modules/berkeley-db/bdb.c:1753 Cdr(STACK_1) = copy_list(STACK_0);

(Found by searching for Car/Cdr(STACK_n) = f(...) only; other place forms were not searched.)

The Common Lisp Foundation's CLISP images (clfoundation/clisp on Docker Hub) carry the patch above for now, and will drop it when a fix is upstream.