GNU bug report logs - #75520
Circular code or data can hang Emacs unquittably

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acorallo <at> gnu.org, mattiase <at> acm.org, Paul Eggert <eggert <at> cs.ucla.edu>, 75520 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: bug#75520: Circular code or data can hang Emacs unquittably
Date: Tue, 14 Jan 2025 18:45:51 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sun, 12 Jan 2025 21:57:55 -0800
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Mattias Engdegård
>>  <mattiase <at> acm.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
>>  Andrea Corallo <acorallo <at> gnu.org>, 75520 <at> debbugs.gnu.org
>> From: Paul Eggert <eggert <at> cs.ucla.edu>
>>
>> On 2025-01-12 13:46, Pip Cet wrote:
>> > This seems to be a common problem.  I did a quick git grep
>> > 'while.*CONSP':
>>
>> Yes, I did something similar in 2017; see, for example, commit
>> 14dd9101ec4838f75addf25bf6b06ef33f8a7e97, which fixes 'length',
>> 'member', 'memq', etc. I don't remember how carefully I audited the code
>> for the problem way back then. Either I missed several potential issues,
>> or the code has changed since then, or both.
>
> Seems like we have a consensus here that this should be fixed.

I agree, but it's a significant task: My git grep caught ~8 bugs, but
covered only 22% of cases (at best), so I'd naively expect at least 20
bugs, and fixing them is usually simple but occasionally tedious
(because one might forget to include an additional check, for example,
as I did when I accidentally removed a STRINGP (XCAR (tem)) on
scratch/igc).

(I don't know what happened to coccinelle; it used to be good at this
kind of task, but I'm not sure it's still being developed or extended in
the appropriate ways).

My preference would be to use FOR_EACH_TAIL where we can, and to retain
that macro's quitting behavior: while the comment claims there is no
reason to quit in FOR_EACH_TAIL because Emacs doesn't support "very
long" lists, I believe that statement to be inaccurate for the MPS
branch.  I can easily create a 100-million-entry list on the master
branch, too; it takes a few seconds to create (and a few more to
collect).

That would mean:

1. find likely infloops
2. prove that the infloop actually is reachable
3. check each loop for extra conditions
4. ensure that the loop is a good place to quit
5. use FOR_EACH_TAIL
6. write a test for each case

I'm volunteering to do 1, 3, 4, 5.  I think (2) is a waste of time, to
be honest: if it looks like a bug and there isn't a clear comment
indicating that it's not one, it should be fixed.  (6) is very hard, if
we actually want the test to fail (i.e. infloop) reliably for unfixed
Emacs versions.  (I also think a blind belief that more tests are better
is unwarranted, but that's a project decision; since we have decided to
test exhaustively, if not as exhaustively as possible, I'll do that).

If (4) isn't possible, we'll have to put an Flength call somewhere
further up the call chain.

As for (2), consider, for example, the Qerror_conditions property of a
symbol.  skip_debugger contains an unprotected loop iterating through
this property (this is the inner loop; the outer loop, iterating over
Vdebug_ignored_errors, definitely can cause an infloop); if we ever
reach that code, we'll infloop.  I tried setting the Qerror_conditions
property to a circular list, but it didn't infloop.  Do I now go and try
to prove conclusively that every code path that reaches skip_debugger is
actually safe?  If so, do I add comments to every code paths to ensure
that it will remain safe?  I think the effort to do so is
disproportionate: let's just use FOR_EACH_TAIL_SAFE there, even if there
is a possibility that it might not be required for correctness.

> But can we please have at least one test for every place where we fix
> this kind of bug?

Assuming you mean a test case that reliably infloops unfixed Emacs:
probably not.  If the requirement is simply to make some variable
circular and call code which might refer to it, on the off chance that
that code would infloop once in a while, maybe.

And what if we can't find such a test?  I'm pretty sure
find_font_encoding isn't unused, and it will infloop if
Vfont_encoding_alist is circular and we don't want to return one of its
elements, but how do I trigger it?  Even if I managed to trigger it on
my system (no luck so far), would that do anything on Windows, with its
totally different font handling code?  Wouldn't it depend on which fonts
are installed?

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.