[Bug c/94119] New: MIPS: Invalid use of branch delay slots leading to corrupt jump

[Bug c/94119] New: MIPS: Invalid use of branch delay slots leading to corrupt jump

[d.dorau at avm dot de]

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

            Bug ID: 94119
           Summary: MIPS: Invalid use of branch delay slots leading to
                    corrupt jump
           Product: gcc
           Version: 8.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: d.dorau at avm dot de
  Target Milestone: ---

Created attachment 48007
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48007&action=edit
Example code + Makefile to reproduce issue

We found an issue with gcc 8.3 issuing wrong instructions in MIPS branch
delay slots. The issue is that in a chain of bne instructions an instruction is
put repeatedly into the delay slots of the bne instructions that should be
issued only once. This leads to an invalid calculation of a subsequent jalr
address which then leads to a crash.

By using git bisect we found commit cd36a4518d14aae18ac89a6232e009ee2bcb6008 to
be the commit that introduced this behaviour (first appeared in gcc 7.1)
although that commit is not directly related to MIPS. So it maybe just
triggering a MIPS specific bug elsewhere.
The issue is still present in the gcc 10 branch. Reverting this commit "fixes"
the issue.

The commit mentioned fixes PR71785.

Steps to reproduce:

Compile the provided sample .c file with 

CFLAGS := -mno-abicalls \
          -mno-branch-likely \
          -ffreestanding \
          -O2 \
          -fno-reorder-blocks \
          -mlong-calls \
          -static \
          -g \
          -save-temps


The bad branch delay slots are in the expansion of the 

#define BUFFER_IS_FF(_a)              \
    ((_a)[0] == 0xff &&                         \
     (_a)[1] == 0xff &&                         \
     (_a)[2] == 0xff &&                         \
     (_a)[3] == 0xff &&                         \
     (_a)[4] == 0xff &&                         \
     (_a)[5] == 0xff)

macro:

The addiu s0,s0,0 instruction must only be issued once but instead is in
several
places. This leads to an invalid call at 9c.


    if((subtype == 8) && !BUFFER_IS_FF(data->addr1))
  30:   14c20016        bne     a2,v0,8c <foo+0x8c>
  34:   a7a00018        sh      zero,24(sp)
  38:   90a20000        lbu     v0,0(a1)
  3c:   240300ff        li      v1,255
  40:   14430010        bne     v0,v1,84 <foo+0x84>
  44:   3c100000        lui     s0,0x0
                        44: R_MIPS_HI16 memcmp_
  48:   90a30001        lbu     v1,1(a1)
  4c:   14620015        bne     v1,v0,a4 <foo+0xa4>
  50:   26100000        addiu   s0,s0,0
                        50: R_MIPS_LO16 memcmp_
  54:   90a20002        lbu     v0,2(a1)
  58:   14430012        bne     v0,v1,a4 <foo+0xa4>
  5c:   26100000        addiu   s0,s0,0
                        5c: R_MIPS_LO16 memcmp_
  60:   90a30003        lbu     v1,3(a1)
  64:   1462000f        bne     v1,v0,a4 <foo+0xa4>
  68:   26100000        addiu   s0,s0,0
                        68: R_MIPS_LO16 memcmp_
  6c:   90a20004        lbu     v0,4(a1)
  70:   14430004        bne     v0,v1,84 <foo+0x84>
  74:   00000000        nop
  78:   90a30005        lbu     v1,5(a1)
  7c:   10620003        beq     v1,v0,8c <foo+0x8c>
  80:   00000000        nop
  84:   10000007        b       a4 <foo+0xa4>
  88:   26100000        addiu   s0,s0,0
                        88: R_MIPS_LO16 memcmp_
    if(!memcmp_(data->addr2, nullbuf, BUF_LEN))
  8c:   3c100000        lui     s0,0x0
                        8c: R_MIPS_HI16 memcmp_
  90:   24060006        li      a2,6
  94:   27a50014        addiu   a1,sp,20
  98:   26100000        addiu   s0,s0,0
                        98: R_MIPS_LO16 memcmp_
  9c:   0200f809        jalr    s0


The provided example code + Makefile will reprocude the crash.

[Bug rtl-optimization/94119] MIPS: Invalid use of branch delay slots leading to corrupt jump

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2020-03-11
     Ever confirmed|0                           |1
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #1 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Please post the output of 'gcc -v' for the affected compiler.

[Bug rtl-optimization/94119] MIPS: Invalid use of branch delay slots leading to corrupt jump

[d.dorau at avm dot de]

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

--- Comment #2 from d.dorau at avm dot de ---
(In reply to Eric Botcazou from comment #1)
> Please post the output of 'gcc -v' for the affected compiler.

We could reproduce this with several gcc builds of which I post the output
below:

1. buildroot cross compile toolchain (which we used for bisecting)

Using built-in specs.
COLLECT_GCC=/home/ddorau/gu/GU_PSQ19/tmp/sysroot/b831bdc30cd51d5f4ae435bcd40a3b66260fe956/bin/mips-buildroot-linux-musl-gcc.br_real
COLLECT_LTO_WRAPPER=/home/ddorau/gu/GU_PSQ19/tmp/sysroot/b831bdc30cd51d5f4ae435bcd40a3b66260fe956/bin/../libexec/gcc/mips-buildroot-linux-musl/8.3.0/lto-wrapper
Target: mips-buildroot-linux-musl
Configured with: ./configure --prefix=/GU/GCC_grx5_build/build/build/host
--sysconfdir=/GU/GCC_grx5_build/build/build/host/etc --enable-static
--target=mips-buildroot-linux-musl
--with-sysroot=/GU/GCC_grx5_build/build/build/host/mips-buildroot-linux-musl/sysroot
--enable-__cxa_atexit --with-gnu-ld --disable-libssp --disable-multilib
--disable-decimal-float --with-gmp=/GU/GCC_grx5_build/build/build/host
--with-mpc=/GU/GCC_grx5_build/build/build/host
--with-mpfr=/GU/GCC_grx5_build/build/build/host --with-pkgversion='Buildroot
2018.11.4-ga8fcc4740' --with-bugurl=http://bugs.buildroot.net/ --disable-libmpx
--disable-libquadmath --disable-libsanitizer --enable-tls --enable-plugins
--enable-lto --disable-libmudflap --enable-threads --without-isl
--without-cloog --with-float=soft --with-arch=interaptiv --with-abi=32
--with-nan=legacy --enable-languages=c,c++
--with-build-time-tools=/GU/GCC_grx5_build/build/build/host/mips-buildroot-linux-musl/bin
--enable-shared --enable-libgomp
Thread model: posix
gcc version 8.3.0 (Buildroot 2018.11.4-ga8fcc4740)

2. Ubuntu mips cross compiler, gcc 7.5 (for verification)

# mips-linux-gnu-gcc-7 -v
Using built-in specs.
COLLECT_GCC=mips-linux-gnu-gcc-7
COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/mips-linux-gnu/7/lto-wrapper
Target: mips-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,d,fortran,objc,obj-c++ --prefix=/usr
--with-gcc-major-version-only --program-suffix=-7 --enable-shared
--enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/
--enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libitm
--disable-libsanitizer --disable-libquadmath --disable-libquadmath-support
--enable-plugin --with-system-zlib --enable-multiarch --disable-werror
--enable-multilib --with-arch-32=mips32r2 --with-fp-32=xx --with-lxc1-sxc1=no
--enable-targets=all --with-arch-64=mips64r2 --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=mips-linux-gnu
--program-prefix=mips-linux-gnu- --includedir=/usr/mips-linux-gnu/include
Thread model: posix
gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04) 

3. Ubuntu mips cross compiler, gcc 8.3 (for verification)

# mips-linux-gnu-gcc-8 -v
Using built-in specs.
COLLECT_GCC=mips-linux-gnu-gcc-8
COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/mips-linux-gnu/8/lto-wrapper
Target: mips-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
8.3.0-6ubuntu1~18.04.1' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++ --prefix=/usr
--with-gcc-major-version-only --program-suffix=-8 --enable-shared
--enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/
--enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libitm
--disable-libsanitizer --disable-libquadmath --disable-libquadmath-support
--enable-plugin --with-system-zlib --disable-libphobos --enable-multiarch
--disable-werror --enable-multilib --with-arch-32=mips32r2 --with-fp-32=xx
--with-lxc1-sxc1=no --enable-targets=all --with-arch-64=mips64r2
--enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu
--target=mips-linux-gnu --program-prefix=mips-linux-gnu-
--includedir=/usr/mips-linux-gnu/include
Thread model: posix
gcc version 8.3.0 (Ubuntu 8.3.0-6ubuntu1~18.04.1)

[Bug rtl-optimization/94119] MIPS: Invalid use of branch delay slots leading to corrupt jump

[ebotcazou at gcc dot gnu.org]

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

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> The addiu s0,s0,0 instruction must only be issued once but instead is in
> several places. This leads to an invalid call at 9c.

Duplicating the instruction is not a problem per se if it is executed only once
on any execution path.  Are you sure that the problem is for the call at 9c?

>   8c:	3c100000 	lui	s0,0x0
> 			8c: R_MIPS_HI16	memcmp_
>   90:	24060006 	li	a2,6
>   94:	27a50014 	addiu	a1,sp,20
>   98:	26100000 	addiu	s0,s0,0
> 			98: R_MIPS_LO16	memcmp_
>   9c:	0200f809 	jalr	s0

AFAICS there is only one "addiu s0,s0,0" executed after the "lui s0,0x0" in
this basic block.  Doesn't the problem occur for the call in the following
basic block instead?

[Bug rtl-optimization/94119] MIPS: Invalid use of branch delay slots leading to corrupt jump

[d.dorau at avm dot de]

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

--- Comment #4 from d.dorau at avm dot de ---
(In reply to Eric Botcazou from comment #3)

> AFAICS there is only one "addiu s0,s0,0" executed after the "lui s0,0x0" in
> this basic block.  Doesn't the problem occur for the call in the following
> basic block instead?

You are right. I truncated the disassembly too early.
Please let me correct myself.

    if((subtype == 8) && !BUFFER_IS_FF(data->addr1))
  30:   14c20016        bne     a2,v0,8c <foo+0x8c>
  34:   a7a00018        sh      zero,24(sp)
  38:   90a20000        lbu     v0,0(a1)
  3c:   240300ff        li      v1,255
  40:   14430010        bne     v0,v1,84 <foo+0x84>
  44:   3c100000        lui     s0,0x0
                        44: R_MIPS_HI16 memcmp_
  48:   90a30001        lbu     v1,1(a1)
  4c:   14620015        bne     v1,v0,a4 <foo+0xa4>
  50:   26100000        addiu   s0,s0,0
                        50: R_MIPS_LO16 memcmp_
  54:   90a20002        lbu     v0,2(a1)
  58:   14430012        bne     v0,v1,a4 <foo+0xa4>
  5c:   26100000        addiu   s0,s0,0
                        5c: R_MIPS_LO16 memcmp_
  60:   90a30003        lbu     v1,3(a1)
  64:   1462000f        bne     v1,v0,a4 <foo+0xa4>
  68:   26100000        addiu   s0,s0,0
                        68: R_MIPS_LO16 memcmp_
  6c:   90a20004        lbu     v0,4(a1)
  70:   14430004        bne     v0,v1,84 <foo+0x84>
  74:   00000000        nop
  78:   90a30005        lbu     v1,5(a1)
  7c:   10620003        beq     v1,v0,8c <foo+0x8c>
  80:   00000000        nop
  84:   10000007        b       a4 <foo+0xa4>
  88:   26100000        addiu   s0,s0,0
                        88: R_MIPS_LO16 memcmp_
    if(!memcmp_(data->addr2, nullbuf, BUF_LEN))
  8c:   3c100000        lui     s0,0x0
                        8c: R_MIPS_HI16 memcmp_
  90:   24060006        li      a2,6
  94:   27a50014        addiu   a1,sp,20
  98:   26100000        addiu   s0,s0,0
                        98: R_MIPS_LO16 memcmp_
  9c:   0200f809        jalr    s0
  a0:   24840006        addiu   a0,a0,6
    bar(dev, data, subtype);
    if(!memcmp_(&dev->b, &dev->a, 1))
  a4:   24060001        li      a2,1
  a8:   02202825        move    a1,s1
  ac:   0200f809        jalr    s0
  b0:   26240001        addiu   a0,s1,1


The two flows:

1) Buffer is all-FF:
   The flow reaches 7c, where it branches to 8c. Regarding s0 we get the
following
   instructions:

   44:  3c100000        lui     s0,0x0
   50:  26100000        addiu   s0,s0,0
   5c:  26100000        addiu   s0,s0,0
   68:  26100000        addiu   s0,s0,0
   8c:  3c100000        lui     s0,0x0
   98:  26100000        addiu   s0,s0,0
   9c:  0200f809        jalr    s0

   where each addui adds "R_MIPS_LO16 memcmp_" to s0. The lui instruction
   at 8c resets s0 so that the call at 9c is successful.

2) Last byte of buffer is non-FF
   The flow reaches 7c, but it does not branch here. Instead we branch to
   a4 at 84. We get

   44:  3c100000        lui     s0,0x0
   50:  26100000        addiu   s0,s0,0
   5c:  26100000        addiu   s0,s0,0
   68:  26100000        addiu   s0,s0,0
   88:  26100000        addiu   s0,s0,0
   ac:  0200f809        jalr    s0

   In this case we get 4 times "R_MIPS_LO16 memcmp_" to s0 which leads to
   a crash at ac.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|Mips                        |mips*-*-*
   Target Milestone|---                         |8.5
             Status|WAITING                     |NEW
            Summary|MIPS: Invalid use of branch |[8/9/10 regression] invalid
                   |delay slots leading to      |filling of branch delay
                   |corrupt jump                |slots leads to corrupt jump

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> You are right. I truncated the disassembly too early.

OK, thanks, I can reproduce.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Fixing.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
commit r9-8372-g593e47a6134085e9b856c62f98f72acd4446ba7c
Author: Eric Botcazou <ebotcazou@gcc.gnu.org>
Date:   Fri Mar 13 09:58:44 2020 +0100

    Fix incorrect filling of delay slots in branchy code at -O2

    The issue is that relax_delay_slots can streamline the CFG in some cases,
    in particular remove BARRIERs, but removing BARRIERs changes the way the
    instructions are associated with (basic) blocks by the liveness analysis
    code in resource.c (find_basic_block) and thus can cause entries in the
    cache maintained by resource.c to become outdated, thus producing wrong
    answers downstream.

    The fix is to invalidate the cache entries affected by the removal of
    BARRIERs in relax_delay_slots, i.e. for the instructions down to the
    next BARRIER.

            PR rtl-optimization/94119
            * resource.h (clear_hashed_info_until_next_barrier): Declare.
            * resource.c (clear_hashed_info_until_next_barrier): New function.
            * reorg.c (add_to_delay_list): Fix formatting.
            (relax_delay_slots): Call clear_hashed_info_until_next_barrier on
            the next instruction after removing a BARRIER.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[marxin at gcc dot gnu.org]

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
commit r10-7152-g3e6ab5cefa81165e90fb62abf50e515f85a17e9a
Author: Eric Botcazou <ebotcazou@gcc.gnu.org>
Date:   Fri Mar 13 09:58:44 2020 +0100

    Fix incorrect filling of delay slots in branchy code at -O2

    The issue is that relax_delay_slots can streamline the CFG in some cases,
    in particular remove BARRIERs, but removing BARRIERs changes the way the
    instructions are associated with (basic) blocks by the liveness analysis
    code in resource.c (find_basic_block) and thus can cause entries in the
    cache maintained by resource.c to become outdated, thus producing wrong
    answers downstream.

    The fix is to invalidate the cache entries affected by the removal of
    BARRIERs in relax_delay_slots, i.e. for the instructions down to the
    next BARRIER.

            PR rtl-optimization/94119
            * resource.h (clear_hashed_info_until_next_barrier): Declare.
            * resource.c (clear_hashed_info_until_next_barrier): New function.
            * reorg.c (add_to_delay_list): Fix formatting.
            (relax_delay_slots): Call clear_hashed_info_until_next_barrier on
            the next instruction after removing a BARRIER.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[marxin at gcc dot gnu.org]

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
commit r8-10121-g92902501c2ad030d56fd3347defd00e79187c51f
Author: Eric Botcazou <ebotcazou@gcc.gnu.org>
Date:   Fri Mar 13 09:58:44 2020 +0100

    Fix incorrect filling of delay slots in branchy code at -O2

    The issue is that relax_delay_slots can streamline the CFG in some cases,
    in particular remove BARRIERs, but removing BARRIERs changes the way the
    instructions are associated with (basic) blocks by the liveness analysis
    code in resource.c (find_basic_block) and thus can cause entries in the
    cache maintained by resource.c to become outdated, thus producing wrong
    answers downstream.

    The fix is to invalidate the cache entries affected by the removal of
    BARRIERs in relax_delay_slots, i.e. for the instructions down to the
    next BARRIER.

            PR rtl-optimization/94119
            * resource.h (clear_hashed_info_until_next_barrier): Declare.
            * resource.c (clear_hashed_info_until_next_barrier): New function.
            * reorg.c (add_to_delay_list): Fix formatting.
            (relax_delay_slots): Call clear_hashed_info_until_next_barrier on
            the next instruction after removing a BARRIER.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Fixed on all active branches.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[d.dorau at avm dot de]

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

--- Comment #11 from d.dorau at avm dot de ---
(In reply to Eric Botcazou from comment #10)
> Fixed on all active branches.

Thanky you very much for the quick fix! To verify I cherry-picked your commit
onto our 8.3 build and the result looks fine so far.

I noticed that you accidentially put the wrong year into the ChangeLog.

[Bug rtl-optimization/94119] [8/9/10 regression] invalid filling of branch delay slots leads to corrupt jump

[ebotcazou at gcc dot gnu.org]

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

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I noticed that you accidentially put the wrong year into the ChangeLog.

Thanks, fixed.