[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Slightly extending commit 16b0520a9

From: Paul Eggert
Subject: Re: Slightly extending commit 16b0520a9
Date: Sun, 6 Aug 2017 17:10:01 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

Stefan Monnier wrote:
it seems that XCDR and XCAR here are
safe because before calling those functions, eval_sub happens to call
Flength on the args, and that triggers an error if the form is not
a proper list, so `XCDR (args)` will indeed be a cons once we get
to Fif.

That's true only if the S-expression does not mutate as it is being evaluated. If the S-expression modifies itself after Flength and before XCDR, Emacs can crash. So Alex's first patch is the opposite of what it should be: instead of changing the Fcar to an XCAR, we should change an XCDR to an Fcdr in the next line. I looked for nearby instances of this problem, and fixed them by installing the attached patch. This should address both problems that Alex mentioned.

Arguably Fif could be called from elsewhere than eval_sub, and arguably
eval_sub's implementation could be changed in such a way that it doesn't
catch this error, so the safety of using XCDR is debatable.

Even before the attached patch was installed, several UNEVALLED functions assumed that their arguments were proper lists. So, when writing the abovementioned patch, I found it simpler to make this assumption everywhere. The argument lists might become improper if they are mutated, so UNEVALLED functions that can invoke arbitrary lisp code still must check the lists as they go, though.

Fif should not be performance sensitive

Yes, in this part of eval.c using XCDR instead of Fcdr is helpful mostly as a hint to the reader that the object is a cons; it's not a significant performance improvement.

Attachment: 0001-Fix-some-crashes-on-self-modifying-Elisp-code.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]