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

From: Pip Cet <[email-masked]>
Date: Sat, 18 Jan 2025 16:40:21 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Sat, 18 Jan 2025 14:20:39 +0000
>> From:  Pip Cet via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>>
>> "Pip Cet via \"Bug reports for GNU Emacs, the Swiss army knife of text editors\"" <[email-masked]> writes:
>>
>> > 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.
>>
>> Patch 1 adds :crash tests to ert.el.
>
> Hmm... maybe this should be discussed separately, as it's a much
> broader issue.  Can you provide the motivation, both for the new macro
> (which is basically a one-liner), and for the new tag?
>
> I've added some people to the discussion, because of this particular
> part (maybe we should take this part to a separate thread).

Responding separately to these two issues.  If we need a new bug number,
please let me know.  This is the ERT response.

Summary: Some bugs cause Emacs to crash.  We'd like to introduce
regression tests so the crashes, once fixed, won't reappear.  However,
if they do, ERT currently makes it hard to find out which test failed.
Also, these cases are usually such strange things for Elisp to do that
that it's perfectly okay for behavior to be undefined or change: we
don't want to test a return value using (should) or (should-not), and we
want to permit errors but not require them.

>> +        (cl-destructuring-bind (stats test ) event-args
>> +          (and (not ert-quiet)
>> +               (memq :crash (ert-test-tags test))
>> +               (let* ((max (prin1-to-string (length (ert--stats-tests stats))))
>> +                      (format-string (concat "%9s  %"
>> +                                             (prin1-to-string (length max))
>> +                                             "s/" max "  %S")))
>> +                 (message format-string
>> +                          "started"
>> +                          (1+ (ert--stats-test-pos stats test))
>> +                          (ert-test-name test))))))
>
> Is it guaranteed that this text will be output before Emacs dies?

It reaches errputc, which has this code:

  fputc_unlocked (c, errstream ());

#ifdef WINDOWSNT
  /* Flush stderr after outputting a newline since stderr is fully
     buffered when redirected to a pipe, contrary to POSIX.  */
  if (c == '\n')
    fflush_unlocked (stderr);
#endif

So I think it does (but it's different from vmessage, which calls fflush
in other cases).

> Maybe we should introduce flush-standard-error, or change the code to
> print to stdout and use flush-standard-output?

I don't think that's necessary.

Pip