[Bug c/101469] New: wrong code with "-O2 -fPIE" for SH

[Bug c/101469] New: wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

            Bug ID: 101469
           Summary: wrong code with "-O2 -fPIE" for SH
           Product: gcc
           Version: 10.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rin at NetBSD dot org
                CC: rin at NetBSD dot org
  Target Milestone: ---
            Target: shle--netbsdelf

This Bug is for GCC 10.3 for shle:

----
$ shle--netbsdelf-gcc -v
Using built-in specs.
COLLECT_GCC=/build/gcc10/tools/bin/shle--netbsdelf-gcc
COLLECT_LTO_WRAPPER=/build/gcc10/tools/libexec/gcc/shle--netbsdelf/10.3.0/lto-wrapper
Target: shle--netbsdelf
Configured with: /usr/src/tools/gcc/../../external/gpl3/gcc/dist/configure
--target=shle--netbsdelf --enable-long-long --enable-threads
--with-bugurl=http://www.NetBSD.org/support/send-pr.html
--with-pkgversion='NetBSD nb1 20210411' --with-system-zlib --without-isl
--enable-__cxa_atexit --enable-libstdcxx-time=rt --enable-libstdcxx-threads
--with-diagnostics-color=auto-if-env --with-default-libstdcxx-abi=new
--with-sysroot=/build/gcc10/dest/landisk --with-mpc=/build/gcc10/tools
--with-mpfr=/build/gcc10/tools --with-gmp=/build/gcc10/tools --disable-nls
--disable-multilib --program-transform-name='s,^,shle--netbsdelf-,'
--enable-languages='c c++ objc' --prefix=/build/gcc10/tools
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.3.0 (NetBSD nb1 20210411)
----

GCC miscompile this code with "-O2 -fPIE":

----
typedef struct {
        int pad[16];
        int i;
        int *p;
} struct_t;

struct_t *sp;

void *ptr(void);

void func(void) {
        sp = ptr();
        sp->p = &sp->i;
}
----

The following is objdump with comments:

----
00000000 <func>:
   0:   mov.l   r12,@-r15
   2:   mova    24 <func+0x24>,r0
   4:   mov.l   24 <func+0x24>,r12
   6:   sts.l   pr,@-r15
   8:   add     r0,r12                  ! r12 = .got
   a:   mov.l   28 <func+0x28>,r1
   c:   bsrf    r1                      ! r0 = ptr()
   e:   nop
  10:   mov.l   2c <func+0x2c>,r1
  12:   mov     r0,r2                   ! r2 = r0
  14:   mov     r12,r0
  16:   mov.l   r2,@(r0,r1)             ! @(.got, 2c) = sp = r2
  18:   add     #64,r2                  ! r2 = &sp->i
  1a:   mov.l   r2,@(4,r12)             ! XXX
  1c:   lds.l   @r15+,pr
  1e:   rts
  20:   mov.l   @r15+,r12
  22:   nop
  24:   .word 0x0000
  26:   .word 0x0000
  28:   sett
  2a:   .word 0x0000
  2c:   .word 0x0000
----

The problem is marked by XXX in comment; if this line were

----
  1a:   mov.l   r2,@(4,r2)
----

it would make sense, i.e.,

----
  @(4, &sp->i) = sp->p = r2 = &sp->i
----

However, unfortunately, GCC somehow mistakes r12 (= .got) with r2.
As a result, sp->p is not correctly set, and .got gets corrupted.

Note that generated code is almost same for "-Os -fPIE". And the
problem occurs also for GCC 9.3.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #1 from Rin Okuyama <rin at NetBSD dot org> ---
I've confirmed that the problem also occurs for sh3-none-elf:

----
$ sh3-none-elf-gcc -v
Using built-in specs.
COLLECT_GCC=/usr/pkg/cross-sh3-none-elf/bin/sh3-none-elf-gcc
COLLECT_LTO_WRAPPER=/usr/pkg/cross-sh3-none-elf/libexec/gcc/sh3-none-elf/10.3.0/lto-wrapper
Target: sh3-none-elf
Configured with:
/build/pkgsrc/cross/sh3-none-elf-gcc/work.x86_64/gcc-10.3.0/configure
--target=sh3-none-elf --enable-languages=c,c++ --with-newlib --disable-nls
--disable-libstdcxx-pch --prefix=/usr/pkg/cross-sh3-none-elf
--build=x86_64--netbsd --host=x86_64--netbsd
--infodir=/usr/pkg/cross-sh3-none-elf/info
--mandir=/usr/pkg/cross-sh3-none-elf/man
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 10.3.0 (GCC)
----

So, this is not NetBSD-specific.

Also, for shle--netbsdelf, GCC 7.5 and 5.5 has the same problem; this is
not a recent regression.

Thanks,
rin

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[uwe at netbsd dot org]

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

--- Comment #2 from Valeriy E. Ushakov <uwe at netbsd dot org> ---
I don't have the latest gcc handy, but I see this bug already in an old netbsd
tree from about an year ago with gcc 8.4.0

As hgutch@n.o pointed out, this seems to be a problem with this peephole2:

;;      mov     r12,r0
;;      add     #-48,r0     ->  add     #-48,r12
;;      mov.l   r0,@(4,r10)     mov.l   r12,@(4,r10)
;;      (r12 dead)
(define_peephole2
  [(set (match_operand:SI 0 "arith_reg_dest")
        (match_operand:SI 1 "arith_reg_dest"))
   (set (match_dup 0) (plus:SI (match_dup 0)
                               (match_operand:SI 2 "const_int_operand")))
   (set (match_operand:SI 3 "general_movdst_operand") (match_dup 0))]


as far as I can tell, it optimizes

  r0 = r2
  r0 += 64
  *(r0+4) = r0

to

  r2 += 64
  *(r0+4) = r2

failing to notice that the the destination uses r0 too.

Then in the cprop-registers step that r0 is replaced with r12 b/c of r0 = r12
done a bit earlier.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #3 from Rin Okuyama <rin at NetBSD dot org> ---
Thank you very much for your analysis!

If that peephole is removed, GCC 10.3 generates working codes.

NetBSD/shle built by this compiler works fine as far as I can see.
I'm carrying out full regression tests of NetBSD on this system.
(It takes about a day to finish.)

Is there a better fix than mechanically removing that peephole?

Thanks,
rin

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[olegendo at gcc dot gnu.org]

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

--- Comment #4 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Rin Okuyama from comment #3)
> Thank you very much for your analysis!
> 
> If that peephole is removed, GCC 10.3 generates working codes.
> 
> NetBSD/shle built by this compiler works fine as far as I can see.
> I'm carrying out full regression tests of NetBSD on this system.
> (It takes about a day to finish.)
> 
> Is there a better fix than mechanically removing that peephole?

Thanks for reporting and analyzing this issue.

I was suspecting that to be a broken peephole pattern.  Of course it's better
to fix the broken pattern instead of deleting it completely.

I'll try to come up with a patch during this week.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #5 from Rin Okuyama <rin at NetBSD dot org> ---
Thank you for your kind attention! I look forward to hearing from you.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #6 from Rin Okuyama <rin at NetBSD dot org> ---
(In reply to Rin Okuyama from comment #3)
> If that peephole is removed, GCC 10.3 generates working codes.
> 
> NetBSD/shle built by this compiler works fine as far as I can see.
> I'm carrying out full regression tests of NetBSD on this system.
> (It takes about a day to finish.)

It took more time due to minor troubles for my side, but it finished.
Fallout from this bug is fixed, whereas no new regressions are observed!

Thanks,
rin

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-07-07
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 55498
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55498&action=edit
also change address register when applying the peephole

The attached patch should fix the issue without removing the peephole
optimization.  If the eliminated register is in the memory address operand,
then the memory operand will be updated respectively.

Does anybody still has access to the system where this bug happened? I'd
appreciate any testing support.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #8 from Rin Okuyama <rin at NetBSD dot org> ---
(In reply to Oleg Endo from comment #7)

Hi Oleg. Thank you very much for your great work!

I've tested your patch with GCC 10.4.0 in this weekend, and it perfectly
worked:
- testcase attached to comment #0 compiled correctly
- full regression tests on NetBSD/shle-current built by patched GCC
successfully completed without any regression (compared with system built by
GCC with that peephole optimization removed)
- some 3rd party softwares (including perl 5.36.0, ruby 3.10.10, zsh 5.9, ...)
self-built on shle host by GCC of same code base

Let me thank you again, Oleg!

rin

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[olegendo at gcc dot gnu.org]

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

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Rin Okuyama from comment #8)
> (In reply to Oleg Endo from comment #7)
> 
> Hi Oleg. Thank you very much for your great work!
> 
> I've tested your patch with GCC 10.4.0 in this weekend, and it perfectly
> worked:
> - testcase attached to comment #0 compiled correctly
> - full regression tests on NetBSD/shle-current built by patched GCC
> successfully completed without any regression (compared with system built by
> GCC with that peephole optimization removed)
> - some 3rd party softwares (including perl 5.36.0, ruby 3.10.10, zsh 5.9,
> ...) self-built on shle host by GCC of same code base
> 
> Let me thank you again, Oleg!
> 
> rin

Thanks you so much for getting back!  Sorry for being late for 2 years with
this.  I will commit the patch to all open GCC versions.  Unfortunately GCC
10.5 has just been released recently and that was the last version 10.  So the
patch will be included from GCC version 11.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #10 from Rin Okuyama <rin at NetBSD dot org> ---
(In reply to Oleg Endo from comment #9)
> Sorry for being late for 2 years with this.
...
> Unfortunately GCC 10.5 has just been released recently and that was the last version 10.
> So the patch will be included from GCC version 11.

No problem at all! We will probably ship GCC 10.5 with your fix for NetBSD 10.
And we will switch to GCC 12 for NetBSD-current soon.

> I will commit the patch to all open GCC versions.

I'm looking forward to it :)

I'm really excited to know that we have active GCC developer working on SH!

Thanks,
rin

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Oleg Endo <olegendo@gcc.gnu.org>:

https://gcc.gnu.org/g:4dbb3af1efe55174a714d15c2994cf2842ef8c28

commit r14-2511-g4dbb3af1efe55174a714d15c2994cf2842ef8c28
Author: Oleg Endo <olegendo@gcc.gnu.org>
Date:   Fri Jul 14 09:54:20 2023 +0900

    SH: Fix PR101496 peephole bug

    gcc/ChangeLog:

            PR target/101469
            * config/sh/sh.md (peephole2): Handle case where eliminated reg is
also
            used by the address of the following memory operand.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Oleg Endo
<olegendo@gcc.gnu.org>:

https://gcc.gnu.org/g:75b8353e4b61566f7e8ac627204e2bcd6bfe60a6

commit r11-10909-g75b8353e4b61566f7e8ac627204e2bcd6bfe60a6
Author: Oleg Endo <olegendo@gcc.gnu.org>
Date:   Fri Jul 14 09:54:20 2023 +0900

    SH: Fix PR101469 peephole bug

    gcc/ChangeLog:

            PR target/101469
            * config/sh/sh.md (peephole2): Handle case where eliminated reg
            is also used by the address of the following memory operand.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Oleg Endo
<olegendo@gcc.gnu.org>:

https://gcc.gnu.org/g:5f20f736c1144dd9f2ae2f99099b7f7b21a3ab4e

commit r12-9772-g5f20f736c1144dd9f2ae2f99099b7f7b21a3ab4e
Author: Oleg Endo <olegendo@gcc.gnu.org>
Date:   Fri Jul 14 09:54:20 2023 +0900

    SH: Fix PR101469 peephole bug

    gcc/ChangeLog:

            PR target/101469
            * config/sh/sh.md (peephole2): Handle case where eliminated reg
            is also used by the address of the following memory operand.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[cvs-commit at gcc dot gnu.org]

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

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Oleg Endo
<olegendo@gcc.gnu.org>:

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

commit r13-7563-gef4b6d29d9801c970bee1b3e8675b19ef5f61d2a
Author: Oleg Endo <olegendo@gcc.gnu.org>
Date:   Fri Jul 14 09:54:20 2023 +0900

    SH: Fix PR101469 peephole bug

    gcc/ChangeLog:

            PR target/101469
            * config/sh/sh.md (peephole2): Handle case where eliminated reg
            is also used by the address of the following memory operand.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

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

--- Comment #15 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Fixed on master, GCC 13, GCC 12, GCC 11.

Should also be applied to older release branches that are maintained elsewhere.

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[rin at NetBSD dot org]

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

--- Comment #16 from Rin Okuyama <rin at NetBSD dot org> ---
The fix has been cherry-picked for NetBSD:
https://mail-index.netbsd.org/source-changes/2023/07/18/msg146078.html

Let me thank you again, Oleg!

[Bug target/101469] wrong code with "-O2 -fPIE" for SH

[olegendo at gcc dot gnu.org]

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

--- Comment #17 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Rin Okuyama from comment #16)
> The fix has been cherry-picked for NetBSD:
> https://mail-index.netbsd.org/source-changes/2023/07/18/msg146078.html
> 
> Let me thank you again, Oleg!

Sure, you're welcome.  Let me know if there's anything else.  When you open an
SH related PR here in GCC's bugzilla, please add me to the CC list, or else I
might not see it timely.