GNU bug report logs - #48368
eq? problem at -O2 since Guile 3.0.5

Previous Next

Package: guile;

Reported by: Marius Bakke <marius <at> gnu.org>

Date: Tue, 11 May 2021 20:50:01 UTC

Severity: normal

Done: Andy Wingo <wingo <at> igalia.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48368 in the body.
You can then email your comments to 48368 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-guile <at> gnu.org:
bug#48368; Package guile. (Tue, 11 May 2021 20:50:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Marius Bakke <marius <at> gnu.org>:
New bug report received and forwarded. Copy sent to bug-guile <at> gnu.org. (Tue, 11 May 2021 20:50:02 GMT) Full text and rfc822 format available.

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

From: Marius Bakke <marius <at> gnu.org>
To: bug-guile <at> gnu.org
Subject: eq? problem at -O2 since Guile 3.0.5
Date: Tue, 11 May 2021 22:49:19 +0200
[Message part 1 (text/plain, inline)]
Hi!

I haven't been able to make a reproducer for this, but for illustrative
purposes, here is a code snippet that fails with -O2 on Guile 3.0.5 and
later (excerpt from GNU Shepherd):

  (define (run-command socket-file action service args)
    "Perform ACTION with ARGS on SERVICE, and display the result.  Connect to
  the daemon via SOCKET-FILE."
    (with-system-error-handling
     (let ((sock    (open-connection socket-file))
           (action* (if (and (eq? action 'detailed-status)
                             (memq service '(root shepherd)))
                        'status
                        action)))
       ;; Send the command.
       (write-command (shepherd-command action* service #:arguments args)
                      sock)

https://git.savannah.gnu.org/cgit/shepherd.git/tree/modules/shepherd/scripts/herd.scm#n124

At -O2, (eq? action 'detailed-status) evaluates to true no matter what
symbol ACTION holds.

Interestingly, adding (pk action*) between LET and WRITE-COMMAND gives
the expected result and mitigates the problem(!).

There are other issues that can be observed by running the Shepherd test
suite (i.e. make -j4 check).  With -O1 all pass.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to bug-guile <at> gnu.org:
bug#48368; Package guile. (Sun, 23 May 2021 15:24:02 GMT) Full text and rfc822 format available.

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

From: Marius Bakke <marius <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>, 47172 <at> debbugs.gnu.org
Cc: 48368 <at> debbugs.gnu.org
Subject: Re: bug#47172: Shepherd 0.8.1 tests fail on core-updates
Date: Sun, 23 May 2021 17:23:48 +0200
[Message part 1 (text/plain, inline)]
Ludovic Courtès <ludo <at> gnu.org> skriver:

> Ludovic Courtès <ludo <at> gnu.org> skribis:
>
>> This turns out to be due to a… miscompilation bug.
>>
>> In (shepherd scripts herd), ‘run-command’ has this code:
>>
>>   (let ((sock    (open-connection socket-file))
>>         (action* (if (and (eq? action 'detailed-status)
>>                           (memq service '(root shepherd)))
>>                      'status
>>                      action)))
>>     …)
>>
>> Problem is that everything works as if (eq? action 'detailed-status)
>> was omitted, such that ‘herd stop root’ is interpreted as ‘herd status
>> root’.
>
> A workaround that works with 3.0.7 is swapping the two ‘and’
> sub-expressions:
>
> diff --git a/modules/shepherd/scripts/herd.scm b/modules/shepherd/scripts/herd.scm
> index 106de1e..39d2e34 100644
> --- a/modules/shepherd/scripts/herd.scm
> +++ b/modules/shepherd/scripts/herd.scm
> @@ -126,8 +126,8 @@ of pairs."
>  the daemon via SOCKET-FILE."
>    (with-system-error-handling
>     (let ((sock    (open-connection socket-file))
> -         (action* (if (and (eq? action 'detailed-status)
> -                           (memq service '(root shepherd)))
> +         (action* (if (and (memq service '(root shepherd))
> +                           (eq? action 'detailed-status))
>                        'status
>                        action)))
>       ;; Send the command.

Cc'ing the relevant Guile bug:

  https://bugs.gnu.org/48368

See also commit 79be6a985799adc6d663890250f4fb7c12f015b4 on
'core-updates' that builds with -O1 as a less satisfactory workaround.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to bug-guile <at> gnu.org:
bug#48368; Package guile. (Sun, 23 May 2021 21:44:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Marius Bakke <marius <at> gnu.org>
Cc: 47172 <at> debbugs.gnu.org, 48368 <at> debbugs.gnu.org
Subject: Re: bug#47172: Shepherd 0.8.1 tests fail on core-updates
Date: Sun, 23 May 2021 23:43:09 +0200
[Message part 1 (text/plain, inline)]
Hello,

Marius Bakke <marius <at> gnu.org> skribis:

> Ludovic Courtès <ludo <at> gnu.org> skriver:

[...]

>> A workaround that works with 3.0.7 is swapping the two ‘and’
>> sub-expressions:
>>
>> diff --git a/modules/shepherd/scripts/herd.scm b/modules/shepherd/scripts/herd.scm
>> index 106de1e..39d2e34 100644
>> --- a/modules/shepherd/scripts/herd.scm
>> +++ b/modules/shepherd/scripts/herd.scm
>> @@ -126,8 +126,8 @@ of pairs."
>>  the daemon via SOCKET-FILE."
>>    (with-system-error-handling
>>     (let ((sock    (open-connection socket-file))
>> -         (action* (if (and (eq? action 'detailed-status)
>> -                           (memq service '(root shepherd)))
>> +         (action* (if (and (memq service '(root shepherd))
>> +                           (eq? action 'detailed-status))
>>                        'status
>>                        action)))
>>       ;; Send the command.
>
> Cc'ing the relevant Guile bug:
>
>   https://bugs.gnu.org/48368

Oh nice!  (It would have saved me a bit of time to catch up on email
beforehand.  :-))

> See also commit 79be6a985799adc6d663890250f4fb7c12f015b4 on
> 'core-updates' that builds with -O1 as a less satisfactory workaround.

I found that ‘-O2 -Ono-resolve-primitives’ also does the trick.

If we manually replace ‘memq’ by two ‘eq?’ tests (which is what the
compiler does), the same problem is exhibited:

[Message part 2 (text/x-patch, inline)]
diff --git a/modules/shepherd/scripts/herd.scm b/modules/shepherd/scripts/herd.scm
index 106de1e..513508f 100644
--- a/modules/shepherd/scripts/herd.scm
+++ b/modules/shepherd/scripts/herd.scm
@@ -127,7 +127,8 @@ the daemon via SOCKET-FILE."
   (with-system-error-handling
    (let ((sock    (open-connection socket-file))
          (action* (if (and (eq? action 'detailed-status)
-                           (memq service '(root shepherd)))
+                           (or (eq? service 'root)
+                               (eq? service 'shepherd)))
                       'status
                       action)))
      ;; Send the command.
[Message part 3 (text/plain, inline)]
‘-Ono-resolve-primitives’ also helps in this case.

‘-Ono-optimize-branch-chains’ has no effect.

So, not much progress, but at least we have a workaround.

Thanks,
Ludo’.

Information forwarded to bug-guile <at> gnu.org:
bug#48368; Package guile. (Mon, 24 May 2021 08:16:01 GMT) Full text and rfc822 format available.

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

From: Andy Wingo <wingo <at> igalia.com>
To: 48368 <at> debbugs.gnu.org
Subject: reduced test case
Date: Mon, 24 May 2021 10:14:54 +0200
Test case:

  (define (f a b)
    (let ((c (if (and (eq? a 'foo)
                      (eq? b 'bar))
                 'ERROR
                 a)))
      (pk c)))

If you run as (f 'not-foo 'bar), you get 'ERROR.  Yeeps!




Reply sent to Andy Wingo <wingo <at> igalia.com>:
You have taken responsibility. (Mon, 24 May 2021 12:20:02 GMT) Full text and rfc822 format available.

Notification sent to Marius Bakke <marius <at> gnu.org>:
bug acknowledged by developer. (Mon, 24 May 2021 12:20:02 GMT) Full text and rfc822 format available.

Message #19 received at 48368-done <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> igalia.com>
To: 48368-done <at> debbugs.gnu.org
Subject: thanks
Date: Mon, 24 May 2021 14:19:22 +0200
Fixed in 17aab66e75136cf23c7f0d4942b61d6947f98f9b.  Thanks for the
report :)




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 22 Jun 2021 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 280 days ago.

Previous Next


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