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

Previous Next

Full log

Message #44 received at 75520 <at> debbugs.gnu.org (full text, mbox):

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

> 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).

I guess the impact depends on the time to perform one iteration of
the loop.  If iterating over the 100 million elements also takes just
a few seconds, then I don't see a need to try and `maybe_quit`.

FWIW, I find accidental circular lists happen much more often than
accidental 100-million-entry lists.

[ In any case I think this is mostly irrelevant for the discussion at
  hand or at least can be decided separately/later.  ]

> 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.

+1

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

For (4) the question is not just "quit" but non-local exit in general
since `FOR_EACH_TAIL` can also signal an error.  We should likely use
`FOR_EACH_TAIL_SAFE` when non-local exits aren't known to be acceptable.

> 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?

Yeah, I just took a look and it's nasty.  I think we'll usually catch it
in `find_handler_clause` (because that one uses `Fmemq`) before we reach
that code, but there's probably some way to avoid those `Fmemq`.

> 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.

Agreed.  This specific code could likely use `Fmemq` even.

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

Sounds like a lot of work since many of those places can require a fair
bit of setup before we can trigger them.


        Stefan

Previous Next

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