bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Pip Cet]

Recipe in emacs -Q:

In *scratch*, evaluate:

(byte-compile (lambda (x)
                (if x (f))
                (while t)))

Expected result:

A (fairly useless) bytecode object

Actual result:

A spinning hourglass.

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

On Wed, Mar 3, 2021 at 9:15 PM Pip Cet <pipcet@gmail.com> wrote:
>
> Recipe in emacs -Q:
>
> In *scratch*, evaluate:
>
> (byte-compile (lambda (x)
>                 (if x (f))
>                 (while t)))
>
> Expected result:
>
> A (fairly useless) bytecode object
>
> Actual result:
>
> A spinning hourglass.

Seems to be a fairly obvious bug in byte-opt.el, patch attached.

Okay to commit?

Pip

[-- Attachment #2: 0001-Fix-bytecompiler-infloop-compiling-infloops-bug-4690.patch --]
[-- Type: text/x-patch, Size: 1154 bytes --]

From 5852cbe0bf7b2e70636d252cbf5bf545b6a5cd39 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Wed, 3 Mar 2021 21:37:13 +0000
Subject: [PATCH] Fix bytecompiler infloop compiling infloops (bug#46906)

* lisp/emacs-lisp/byte-opt.el (byte-optimize-lapcode): Don't apply
optimization if we can't change anything.
---
 lisp/emacs-lisp/byte-opt.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index b51ba801552d6..aedfde6c0c6ab 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -2012,9 +2012,9 @@ byte-optimize-lapcode
 	 ((and (memq (car lap0) byte-goto-ops)
 	       (memq (car (setq tmp (nth 1 (memq (cdr lap0) lap))))
 		     '(byte-goto byte-return)))
-	  (cond ((and (not (eq tmp lap0))
-		      (or (eq (car lap0) 'byte-goto)
-			  (eq (car tmp) 'byte-goto)))
+	  (cond ((and (or (eq (car lap0) 'byte-goto)
+			  (eq (car tmp) 'byte-goto))
+                      (not (eq (cdr tmp) (cdr lap0))))
 		 (byte-compile-log-lap "  %s [%s]\t-->\t%s"
 				       (car lap0) tmp tmp)
 		 (if (eq (car tmp) 'byte-return)
-- 
2.30.1

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Andy Moreton]

On Wed 03 Mar 2021, Pip Cet wrote:

> On Wed, Mar 3, 2021 at 9:15 PM Pip Cet <pipcet@gmail.com> wrote:
>>
>> Recipe in emacs -Q:
>>
>> In *scratch*, evaluate:
>>
>> (byte-compile (lambda (x)
>>                 (if x (f))
>>                 (while t)))
>>
>> Expected result:
>>
>> A (fairly useless) bytecode object
>>
>> Actual result:
>>
>> A spinning hourglass.
>
> Seems to be a fairly obvious bug in byte-opt.el, patch attached.

Perhaps add your reproducer as a test case ?

    AndyM

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Wed, Mar 3, 2021 at 11:34 PM Andy Moreton <andrewjmoreton@gmail.com> wrote:
> On Wed 03 Mar 2021, Pip Cet wrote:
> > On Wed, Mar 3, 2021 at 9:15 PM Pip Cet <pipcet@gmail.com> wrote:
> >>
> >> Recipe in emacs -Q:
> >>
> >> In *scratch*, evaluate:
> >>
> >> (byte-compile (lambda (x)
> >>                 (if x (f))
> >>                 (while t)))
> >>
> >> Expected result:
> >>
> >> A (fairly useless) bytecode object
> >>
> >> Actual result:
> >>
> >> A spinning hourglass.
> >
> > Seems to be a fairly obvious bug in byte-opt.el, patch attached.
>
> Perhaps add your reproducer as a test case ?

Absolutely, thanks for the suggestion. See attached patch.

Should there, maybe, be a tag for tests that are known to loop forever
when they fail? It feels weird to write a test without a single should
in it...

Pip

[-- Attachment #2: 0001-Fix-bytecompiler-infloop-compiling-infloops-bug-4690.patch --]
[-- Type: text/x-patch, Size: 1924 bytes --]

From 2f430107f46b506dbb8dd79d9ce5a10cc2b3de9b Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Wed, 3 Mar 2021 21:37:13 +0000
Subject: [PATCH] Fix bytecompiler infloop compiling infloops (bug#46906)

* lisp/emacs-lisp/byte-opt.el (byte-optimize-lapcode): Don't apply
optimization if we can't change anything.
* test/lisp/emacs-lisp/bytecomp-tests.el (bytecomp-infloop): New test.
---
 lisp/emacs-lisp/byte-opt.el            | 6 +++---
 test/lisp/emacs-lisp/bytecomp-tests.el | 5 +++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index b51ba801552d6..aedfde6c0c6ab 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -2012,9 +2012,9 @@ byte-optimize-lapcode
 	 ((and (memq (car lap0) byte-goto-ops)
 	       (memq (car (setq tmp (nth 1 (memq (cdr lap0) lap))))
 		     '(byte-goto byte-return)))
-	  (cond ((and (not (eq tmp lap0))
-		      (or (eq (car lap0) 'byte-goto)
-			  (eq (car tmp) 'byte-goto)))
+	  (cond ((and (or (eq (car lap0) 'byte-goto)
+			  (eq (car tmp) 'byte-goto))
+                      (not (eq (cdr tmp) (cdr lap0))))
 		 (byte-compile-log-lap "  %s [%s]\t-->\t%s"
 				       (car lap0) tmp tmp)
 		 (if (eq (car tmp) 'byte-return)
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 03c267ccd0fef..eeb75c53ee2a9 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1222,6 +1222,11 @@ bytecomp-reify-function
         (byte-compile 'counter)
         (should (equal (counter) 1))))))
 
+(ert-deftest bytecomp-infloop ()
+  "Check that optimizing an infinite loop does not loop indefinitely."
+  ;; This form should not cause the optimizer to loop (bug#46906)
+  (byte-compile (lambda (x) (if (eq x x) (f)) (while t))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.30.1

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Juri Linkov]

>> Perhaps add your reproducer as a test case ?
>
> Absolutely, thanks for the suggestion. See attached patch.
>
> Should there, maybe, be a tag for tests that are known to loop forever
> when they fail? It feels weird to write a test without a single should
> in it...

Maybe 'with-timeout' could help not to loop forever in tests?

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Pip Cet]

On Thu, Mar 4, 2021 at 10:16 AM Juri Linkov <juri@linkov.net> wrote:
> Maybe 'with-timeout' could help not to loop forever in tests?

I don't think so, because it doesn't work for Lisp infloops.
(with-timeout (1 (error "foo")) (while t)) doesn't work here, at
least.

Pip

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Lars Ingebrigtsen]

Pip Cet <pipcet@gmail.com> writes:

> I don't think so, because it doesn't work for Lisp infloops.
> (with-timeout (1 (error "foo")) (while t)) doesn't work here, at
> least.

Yes, you need to put something that yields in there for `with-timeout'
to work.  For instance:

(with-timeout (1 (error "foo")) (while t (sleep-for 0.1)))

However, I haven't checked whether this form triggers the bug that we're
trying to reproduce.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Pip Cet]

On Thu, Mar 4, 2021 at 11:16 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Pip Cet <pipcet@gmail.com> writes:
> > I don't think so, because it doesn't work for Lisp infloops.
> > (with-timeout (1 (error "foo")) (while t)) doesn't work here, at
> > least.
>
> Yes, you need to put something that yields in there for `with-timeout'
> to work.  For instance:
>
> (with-timeout (1 (error "foo")) (while t (sleep-for 0.1)))
>
> However, I haven't checked whether this form triggers the bug that we're
> trying to reproduce.

Even if it would, that wouldn't solve our issue: it's the optimizer
which infloops trying to compile the form, and the optimizer certainly
does not yield anywhere that I'm aware of...

I think we're just going to have to put in a test which, on systems
with the bug, loops indefinitely. We could make it "expensive", though
it isn't, really.

(I'm not sure how many people are trying to run the Emacs 28 testsuite
against Emacs 27, or anything like that. It might be a non-issue.)

Pip

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Lars Ingebrigtsen]

Pip Cet <pipcet@gmail.com> writes:

> I think we're just going to have to put in a test which, on systems
> with the bug, loops indefinitely. We could make it "expensive", though
> it isn't, really.
>
> (I'm not sure how many people are trying to run the Emacs 28 testsuite
> against Emacs 27, or anything like that. It might be a non-issue.)

It's probably not an issue in practice, but it does seem rather
unsatisfactory that we don't have a mechanism for doing tests for
infloops in general.

I wonder whether we could cobble up something just for ert.  For
instance... doing a
(start-process ... "sleep 1; kill -s SIGUSR1 $EMACS_PID") or similar,
and then hook the SIGUSR1 handler to...  er...  do something useful
here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Juri Linkov]

> On Thu, Mar 4, 2021 at 10:16 AM Juri Linkov <juri@linkov.net> wrote:
>> Maybe 'with-timeout' could help not to loop forever in tests?
>
> I don't think so, because it doesn't work for Lisp infloops.
> (with-timeout (1 (error "foo")) (while t)) doesn't work here, at
> least.

Then maybe run a possibly infinite loop in a separate thread?

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Pip Cet]

On Thu, Mar 4, 2021 at 6:44 PM Juri Linkov <juri@linkov.net> wrote:
> > On Thu, Mar 4, 2021 at 10:16 AM Juri Linkov <juri@linkov.net> wrote:
> >> Maybe 'with-timeout' could help not to loop forever in tests?
> >
> > I don't think so, because it doesn't work for Lisp infloops.
> > (with-timeout (1 (error "foo")) (while t)) doesn't work here, at
> > least.
>
> Then maybe run a possibly infinite loop in a separate thread?

Or even as a separate process, maybe. That's what I'd like to do for
(apply nil)...

Pip

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> I don't think so, because it doesn't work for Lisp infloops.
>> (with-timeout (1 (error "foo")) (while t)) doesn't work here, at
>> least.
>
> Then maybe run a possibly infinite loop in a separate thread?

I don't think that would work -- our threading stuff is based on
"cooperative multithreading", I think?  Which means that the code we're
testing has to call `yield', which it normally doesn't.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46906: 28.0.50; byte compiler infloops trying to compile infloop

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I don't think that would work -- our threading stuff is based on
> "cooperative multithreading", I think?  Which means that the code we're
> testing has to call `yield', which it normally doesn't.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

We apparently forgot to apply Pip's fix because we started talking about
fixing the test harness.  So I've now applied the fix, but not the test,
because we don't really have a good way to test infinite loops in ert.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no