[Bug c/103908] New: gcc 7.5.0 (and earlier) miscompile asm goto in O1 on x86-64

[Bug c/103908] New: gcc 7.5.0 (and earlier) miscompile asm goto in O1 on x86-64

[mathieu.desnoyers at efficios dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

            Bug ID: 103908
           Summary: gcc 7.5.0 (and earlier) miscompile asm goto in O1 on
                    x86-64
           Product: gcc
           Version: 7.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mathieu.desnoyers at efficios dot com
  Target Milestone: ---

Created attachment 52122
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52122&action=edit
Reproducer for asm goto miscompilation in O1 (x86-64) gcc-7.5.0 and prior

gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)
Target: x86_64-linux-gnu

Configured with: ../src/configure -v --with-pkgversion='Ubuntu
7.5.0-3ubuntu1~18.04' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr
--with-gcc-major-version-only --program-suffix=-7
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie
--with-system-zlib --with-target-system-zlib --enable-objc-gc=auto
--enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none --without-cuda-driver
--enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu
--target=x86_64-linux-gnu
Thread model: posix

How to reproduce:

% gcc-7 -O1 -c -o hoisting.o hoisting.i
% objdump -S hoisting.o


hoisting.o:     file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <foo>:
   0:   c7 05 00 00 00 00 01    movl   $0x1,0x0(%rip)        # a <foo+0xa>
   7:   00 00 00 
   a:   c3                      retq 

Expected behavior:

% gcc-8 -O1 -c -o hoisting.o hoisting.i
% objdump -S hoisting.o

hoisting.o:     file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <foo>:
   0:   83 3d 00 00 00 00 00    cmpl   $0x0,0x0(%rip)        # 7 <foo+0x7>
   7:   74 10                   je     19 <foo+0x19>
   9:   e8 00 00 00 00          callq  e <foo+0xe>
   e:   c7 05 00 00 00 00 01    movl   $0x1,0x0(%rip)        # 18 <foo+0x18>
  15:   00 00 00 
  18:   c3                      retq   
  19:   e8 00 00 00 00          callq  1e <foo+0x1e>
  1e:   eb ee                   jmp    e <foo+0xe>

This issue appears to be fixed in gcc 8.1.0, but I did not find any bugzilla
entry documenting this.

[Bug rtl-optimization/103908] gcc 7.5.0 (and earlier) miscompile asm goto in O1 on x86-64

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Seems to have been fixed by r258645

[Bug rtl-optimization/103908] gcc 7.5.0 (and earlier) miscompile asm goto in O1 on x86-64

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The attached testcase is:

int a, b;

void foo(void)
{
  if (a) {
    b = 1;
    asm goto ("call 0x0\n\t" : : : : next1);
next1:;
  } else {
    b = 1;
    asm goto ("call 0x1\n\t" : : : : next2);
next2:;
  }
}

[Bug rtl-optimization/103908] gcc 7.5.0 (and earlier) miscompile asm goto in O1 on x86-64

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Richi, did your commit really fix it, or just make it go latent?

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
            Summary|gcc 7.5.0 (and earlier)     |gcc miscompile asm goto for
                   |miscompile asm goto in O1   |O1
                   |on x86-64                   |
   Last reconfirmed|                            |2022-01-04
     Ever confirmed|0                           |1

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> Seems to have been fixed by r258645

that just makes the problem latent.
You can still reproduce the issue on the trunk with "-O1 -fdisable-tree-cselim
-fno-tree-sink" .

RTL ifcvt does:

starting the processing of deferred insns
ending the processing of deferred insns
df_analyze called
df_worklist_dataflow_doublequeue: n_basic_blocks 6 n_edges 6 count 6 (    1)

IF-THEN-ELSE-JOIN block found, pass 1, test 2, then 3, else 4, join 5
changing bb of uid 15
  from 4 to 2
deleting insn with uid = 16.
deleting block 4
Removing jump 6.
deleting insn with uid = 6.
deleting insn with uid = 9.
deleting insn with uid = 8.
deleting block 3
Merging block 5 into block 2...
changing bb of uid 22
Merged blocks 2 and 5.
Conversion succeeded on pass 1.

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52125
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52125&action=edit
gcc12-pr103908.patch

Untested fix.

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:80ad67e2af0620d58d57d0406dc22693cf5b8ca9

commit r12-6278-g80ad67e2af0620d58d57d0406dc22693cf5b8ca9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 6 09:29:34 2022 +0100

    ifcvt: Check for asm goto at the end of then_bb/else_bb in ifcvt [PR103908]

    On the following testcase, RTL ifcvt sees then_bb
    (note 7 6 8 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
    (insn 8 7 9 3 (set (mem/c:SI (symbol_ref:DI ("b") [flags 0x2]  <var_decl
0x7fdccf5b0cf0 b>) [1 b+0 S4 A32])
            (const_int 1 [0x1])) "pr103908.c":6:7 81 {*movsi_internal}
         (nil))
    (jump_insn 9 8 13 3 (parallel [
                (asm_operands/v ("# insn 1") ("") 0 []
                     []
                     [
                        (label_ref:DI 21)
                    ] pr103908.c:7)
                (clobber (reg:CC 17 flags))
            ]) "pr103908.c":7:5 -1
         (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))
     -> 21)
    and similarly else_bb (just with a different asm_operands template).
    It checks that those basic blocks have a single successor and
    uses last_active_insn which intentionally skips over JUMP_INSNs, sees
    both basic blocks contain the same set and merges them (or if the
    sets are different, attempts some other noce optimization).
    But we can't assume that the jump, even when it has only a single
successor,
    has no side-effects.

    The following patch fixes it by punting if test_bb ends with a JUMP_INSN
    that isn't onlyjump_p.

    2022-01-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/103908
            * ifcvt.c (bb_valid_for_noce_process_p): Punt on bbs ending with
            asm goto.

            * gcc.target/i386/pr103908.c: New test.

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0dd533da49cf15aa9afd35634ba4bd80e9c96565

commit r11-9498-g0dd533da49cf15aa9afd35634ba4bd80e9c96565
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 6 09:29:34 2022 +0100

    ifcvt: Check for asm goto at the end of then_bb/else_bb in ifcvt [PR103908]

    On the following testcase, RTL ifcvt sees then_bb
    (note 7 6 8 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
    (insn 8 7 9 3 (set (mem/c:SI (symbol_ref:DI ("b") [flags 0x2]  <var_decl
0x7fdccf5b0cf0 b>) [1 b+0 S4 A32])
            (const_int 1 [0x1])) "pr103908.c":6:7 81 {*movsi_internal}
         (nil))
    (jump_insn 9 8 13 3 (parallel [
                (asm_operands/v ("# insn 1") ("") 0 []
                     []
                     [
                        (label_ref:DI 21)
                    ] pr103908.c:7)
                (clobber (reg:CC 17 flags))
            ]) "pr103908.c":7:5 -1
         (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))
     -> 21)
    and similarly else_bb (just with a different asm_operands template).
    It checks that those basic blocks have a single successor and
    uses last_active_insn which intentionally skips over JUMP_INSNs, sees
    both basic blocks contain the same set and merges them (or if the
    sets are different, attempts some other noce optimization).
    But we can't assume that the jump, even when it has only a single
successor,
    has no side-effects.

    The following patch fixes it by punting if test_bb ends with a JUMP_INSN
    that isn't onlyjump_p.

    2022-01-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/103908
            * ifcvt.c (bb_valid_for_noce_process_p): Punt on bbs ending with
            asm goto.

            * gcc.target/i386/pr103908.c: New test.

    (cherry picked from commit 80ad67e2af0620d58d57d0406dc22693cf5b8ca9)

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.3 too.

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:8df8eb6dd5b02d2a5b950a00c09590da423db8cc

commit r10-10664-g8df8eb6dd5b02d2a5b950a00c09590da423db8cc
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 6 09:29:34 2022 +0100

    ifcvt: Check for asm goto at the end of then_bb/else_bb in ifcvt [PR103908]

    On the following testcase, RTL ifcvt sees then_bb
    (note 7 6 8 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
    (insn 8 7 9 3 (set (mem/c:SI (symbol_ref:DI ("b") [flags 0x2]  <var_decl
0x7fdccf5b0cf0 b>) [1 b+0 S4 A32])
            (const_int 1 [0x1])) "pr103908.c":6:7 81 {*movsi_internal}
         (nil))
    (jump_insn 9 8 13 3 (parallel [
                (asm_operands/v ("# insn 1") ("") 0 []
                     []
                     [
                        (label_ref:DI 21)
                    ] pr103908.c:7)
                (clobber (reg:CC 17 flags))
            ]) "pr103908.c":7:5 -1
         (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))
     -> 21)
    and similarly else_bb (just with a different asm_operands template).
    It checks that those basic blocks have a single successor and
    uses last_active_insn which intentionally skips over JUMP_INSNs, sees
    both basic blocks contain the same set and merges them (or if the
    sets are different, attempts some other noce optimization).
    But we can't assume that the jump, even when it has only a single
successor,
    has no side-effects.

    The following patch fixes it by punting if test_bb ends with a JUMP_INSN
    that isn't onlyjump_p.

    2022-01-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/103908
            * ifcvt.c (bb_valid_for_noce_process_p): Punt on bbs ending with
            asm goto.

            * gcc.target/i386/pr103908.c: New test.

    (cherry picked from commit 80ad67e2af0620d58d57d0406dc22693cf5b8ca9)

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:e875dc9f975ee0a1f9468fda9ee29533cca77181

commit r9-10117-ge875dc9f975ee0a1f9468fda9ee29533cca77181
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 6 09:29:34 2022 +0100

    ifcvt: Check for asm goto at the end of then_bb/else_bb in ifcvt [PR103908]

    On the following testcase, RTL ifcvt sees then_bb
    (note 7 6 8 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
    (insn 8 7 9 3 (set (mem/c:SI (symbol_ref:DI ("b") [flags 0x2]  <var_decl
0x7fdccf5b0cf0 b>) [1 b+0 S4 A32])
            (const_int 1 [0x1])) "pr103908.c":6:7 81 {*movsi_internal}
         (nil))
    (jump_insn 9 8 13 3 (parallel [
                (asm_operands/v ("# insn 1") ("") 0 []
                     []
                     [
                        (label_ref:DI 21)
                    ] pr103908.c:7)
                (clobber (reg:CC 17 flags))
            ]) "pr103908.c":7:5 -1
         (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))
     -> 21)
    and similarly else_bb (just with a different asm_operands template).
    It checks that those basic blocks have a single successor and
    uses last_active_insn which intentionally skips over JUMP_INSNs, sees
    both basic blocks contain the same set and merges them (or if the
    sets are different, attempts some other noce optimization).
    But we can't assume that the jump, even when it has only a single
successor,
    has no side-effects.

    The following patch fixes it by punting if test_bb ends with a JUMP_INSN
    that isn't onlyjump_p.

    2022-01-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/103908
            * ifcvt.c (bb_valid_for_noce_process_p): Punt on bbs ending with
            asm goto.

            * gcc.target/i386/pr103908.c: New test.

    (cherry picked from commit 80ad67e2af0620d58d57d0406dc22693cf5b8ca9)

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug rtl-optimization/103908] gcc miscompile asm goto for O1

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103908

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|12.0                        |10.3.0, 11.2.0, 9.4.0
      Known to work|                            |10.4.0, 11.3.0, 9.5.0
   Target Milestone|---                         |9.5