eq? problem at -O2 since Guile 3.0.5

DoneSubmitted by Marius Bakke.
Details
3 participants
  • Ludovic Courtès
  • Marius Bakke
  • Andy Wingo
Owner
unassigned
Severity
normal
M
M
Marius Bakke wrote on 11 May 22:49 +0200
Download
(address . bug-guile@gnu.org)
87pmxxhu7k.fsf@gnu.org
Download
Hi!
I haven't been able to make a reproducer for this, but for illustrativepurposes, here is a code snippet that fails with -O2 on Guile 3.0.5 andlater (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 whatsymbol ACTION holds.
Interestingly, adding (pk action*) between LET and WRITE-COMMAND givesthe expected result and mitigates the problem(!).
There are other issues that can be observed by running the Shepherd testsuite (i.e. make -j4 check). With -O1 all pass.
Download
-----BEGIN PGP SIGNATURE-----
iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYJrtzw8cbWFyaXVzQGdudS5vcmcACgkQ6HGLpZEUEHe4hQEAknNS0PVUkTkz0Wy3x/S3Qu50FowvKcMDxmKxnsISOG8BALKLfG7ywwYVOnuC73czgpEaLT3EMzjBdGHnh7Tu6wEO=4y51-----END PGP SIGNATURE-----
M
M
Marius Bakke wrote on 23 May 17:23 +0200
Download
Re: bug#47172: Shepherd 0.8.1 tests fail on core-updates
(address . 48368@debbugs.gnu.org)
875yz9qxrv.fsf@gnu.org
Download
Ludovic Courtès <ludo@gnu.org> skriver:
Toggle quote (36 lines)> Ludovic Courtès <ludo@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.
Download
-----BEGIN PGP SIGNATURE-----
iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYKpzhA8cbWFyaXVzQGdudS5vcmcACgkQ6HGLpZEUEHd0CgD9FsWiNMu2PxB/773BI2hOmYPKZqyX+KbAy05RC7+xubIBAPcyjBy9TtmqfG0aCSUu1r6a8dmFKkJm4r4eb5fLEwEK=o7Sr-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote on 23 May 23:43 +0200
Download
(name . Marius Bakke)(address . marius@gnu.org)
87lf853z4i.fsf@gnu.org
Download
Hello,
Marius Bakke <marius@gnu.org> skribis:
Toggle quote (2 lines)> Ludovic Courtès <ludo@gnu.org> skriver:
[...]
Toggle quote (23 lines)>> 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 emailbeforehand. :-))
Toggle quote (3 lines)> 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 thecompiler does), the same problem is exhibited:
Download
Toggle diff (14 lines)diff --git a/modules/shepherd/scripts/herd.scm b/modules/shepherd/scripts/herd.scmindex 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.
Download
‘-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’.
A
A
Andy Wingo wrote on 24 May 10:14 +0200
Download
reduced test case
(address . 48368@debbugs.gnu.org)
87lf841rb5.fsf@igalia.com
Download
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!
A
A
Andy Wingo wrote on 24 May 14:19 +0200
Download
thanks
(address . 48368-done@debbugs.gnu.org)
87cztg1fzp.fsf@igalia.com
Download
Fixed in 17aab66e75136cf23c7f0d4942b61d6947f98f9b. Thanks for thereport :)
Closed
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send email to 48368@debbugs.gnu.org