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) = listWhen 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));
}
#elseThe 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.