Subject: bug#75648: Minor safety improvements to fns.c/eval.c

From: Pip Cet <[email-masked]>
Date: Sat, 18 Jan 2025 12:18:27 +0000

This is a spin-off from bug#75584, originally reported in

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=75584#27

fns.c contains some minor bugs which can cause crashes for buggy Elisp.
While it's conceivable that one of them could happen by accident, it
doesn't affect the pdumper build, so it is still very unlikely.

They are:

1. plist-get and plist-put assume that the cdr of a cons cell is also a
cons cell.  They check this, then call out to Lisp, then rely on the
fact.  However, the Lisp code can call setcdr and turn the cdr into a
non-cons cell, which causes a crash.

2. Fntake, Fsort, and Fwidget_put modify user-supplied cons cells with
XSETCAR/XSETCDR.  They do not check that the cons cell isn't "pure", in
which case writing to it may cause crashes.  In the case of Fsort, this
is likely to happen when a user accidentally attempts to sort a
(partially) pure list in-place.

3. Fsetq does the same thing to the lexical environment, which may
include a pure cons cell establishing a binding.

As (2) and (3) will become non-bugs once purespace is removed, I would
like to propose not fixing them for now.

(1), however, needs a fix.  Also, just as importantly, it needs tests.

Unfortunately, ert is not really set up very well for tests that may
crash.

My proposal will be to:

1. Give such tests a :crash tag
2. Introduce a should-not-crash macro which succeeds if the form it
evaluates returns in any way, whether by error or not.
3. Modify ert.el to print when a :crash test is about to start running.
This allows us to identify the crashing test.
4. Deviate from the current logical test order and put crash tests last.
It's conceivable that if a once-fixed crash reoccurs, the reason is a
simple bug that may show up in regular tests, too.  If two tests fail
and one of them crashes the test run, it's better for the first failure
to have been reported first.

I'll send a patch once this has a bug number.