[Bug target/44199] New: ppc64 glibc miscompilation

[Bug target/44199] New: ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

vfscanf is miscompiled by gcc 4.4, here is a shorter testcase that is
miscompiled also by 4.6:

extern void *alloca (__SIZE_TYPE__);
extern void bar (long *, char *);

long
foo (long x)
{
  long buf[400];
  char *p = alloca (x);
  bar (buf, p);
  return buf[0];
}

at -O2 -m64 results in:
...
        bl .bar
        nop
        addi 1,31,3328
        ld 3,112(31)
        ld 0,16(1)
        ld 31,-8(1)
        mtlr 0
        blr
Note that sched2 swapped incorrectly the stack frame release and ld 3,112(31),
so the ld insn reads from memory location 3216 below the stack (which is much
lower than 288 bytes of ppc64 red zone).
If a signal comes in in between addi and ld and something clobbers the stack,
this function (or, with 4.4 vfscanf) will return incorrect value.
There is nothing in the epilogue stack release insn that would make it a memory
barrier.


-- 
           Summary: ppc64 glibc miscompilation
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
GCC target triplet: powerpc64-linux


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2010-05-19 18:30 -------
Looks related to PR 30282.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2010-05-19 19:09 -------
*** Bug 44201 has been marked as a duplicate of this bug. ***


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #2 from jakub at gcc dot gnu dot org  2010-05-19 19:09 -------
*** Bug 44200 has been marked as a duplicate of this bug. ***


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #4 from jakub at gcc dot gnu dot org  2010-05-19 19:23 -------
Yes, it is related, but solving it in the scheduler in generic way isn't going
to be trivial.

E.g. x86_64 emits memory_blockage early in ix86_expand_epilogue:
  /* See the comment about red zone and frame
     pointer usage in ix86_expand_prologue.  */
  if (frame_pointer_needed && frame.red_zone_size)
    emit_insn (gen_memory_blockage ());

Another testcase:

extern void *alloca (__SIZE_TYPE__);
__attribute__((noinline, noclone))
long *bar (long *p)
{
  asm volatile ("" : : "r" (p) : "memory");
  return p;
}

long
foo (long x)
{
  long *p = (long *) alloca (x * sizeof (long));
  long *q = bar (p);
  return q[0];
}

which shows that some blockage or preventing scheduling over the stack release
is needed even when the frame size is smaller than the red zone size and that
the memory address doesn't need to be obviously based on stack pointer (it
would be just safe to allow moving over stack accesses that are provably in the
red zone or above.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2010-05-19 19:27 -------
rs6000.c already has rs6000_emit_stack_tie and calls it in several places in
the epilogue generation, I guess we just should add another call to this.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #6 from jakub at gcc dot gnu dot org  2010-05-19 20:31 -------
Created an attachment (id=20705)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20705&action=view)
gcc46-pr44199.patch

Untested patch.  Unfortunately, rs6000_emit_stack_tie isn't good enough.
1) it uses frame alias set, but the stack block can have arbitrary alias sets
in
   it and we need to avoid moving all of them over
2) for the frame_pointer_needed case using sp based BLK mem isn't sufficient,
   as then fp based mem can still be scheduled over it.  Using stack_tie insn
   just with hard frame pointer based BLK mem doesn't work either, then the sp
   restore insn can be moved over it.

Could anyone please test this on SPEC to see if it causes meassurable
differences?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #7 from bergner at gcc dot gnu dot org  2010-05-19 21:52 -------
Pat is going to SPEC test the patch and will report back here with his results.


-- 

bergner at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pthaugen at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[amodra at gmail dot com]

------- Comment #8 from amodra at gmail dot com  2010-05-20 04:31 -------
FWIW, Jakub's patch looks a reasonable fix to me.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[pthaugen at gcc dot gnu dot org]

------- Comment #9 from pthaugen at gcc dot gnu dot org  2010-05-20 16:23 -------
Spec testing went fine, differences in the noise range.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #10 from jakub at gcc dot gnu dot org  2010-05-20 16:31 -------
Have you also bootstrapped/regtested the patch?  I can do so (easily for 4.4
branch, with more effort for the trunk), but haven't done that yet.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[pthaugen at gcc dot gnu dot org]

------- Comment #11 from pthaugen at gcc dot gnu dot org  2010-05-20 17:59 -------
No I didn't bootstrap/regtest. I can take care of trunk if you want to do 4.4.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #12 from jakub at gcc dot gnu dot org  2010-05-20 18:23 -------
If you could, it would be very much appreciated.  Starting 4.4
bootstrap/regtest now.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[pthaugen at gcc dot gnu dot org]

------- Comment #13 from pthaugen at gcc dot gnu dot org  2010-05-21 02:32 -------
Bootstrap of trunk went fine. Regression test (contrib/test_summary) showed the
following difference between base/patched versions, but didn't have any
testcases show up in the diff, so not sure what to make of that. This is for
32-bit gcc testsuite.

< # of expected passes          59078
---
> # of expected passes          59075
98c98
< # of unsupported tests                872
---
> # of unsupported tests                875


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #14 from jakub at gcc dot gnu dot org  2010-05-21 08:08 -------
For me it bootstrapped/regtested on 4.4 branch without any testsuite changes
(both 32-bit and 64-bit).
To see the unsupported tests difference, you can
grep ^UNSUPPORTED gcc/testsuite/gcc/gcc.log | sort
between the unpatched/patched builds and see what the differences are I guess.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #15 from bergner at gcc dot gnu dot org  2010-05-21 19:14 -------
I also did a powerpc64-linux bootstrap and regtest (both 32-bit and 64-bit) and
I didn't see any new failures and I also did not see any extra UNSUPPORTED
tests.  The only time UNSUPPORTED showed up in the test_summary output
(UNRESOLVED: one_time_plugin.c compilation,...) was due to different source
directory paths.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[iains at gcc dot gnu dot org]

------- Comment #16 from iains at gcc dot gnu dot org  2010-05-21 19:24 -------
is : http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44229
potentially a similar problem?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[dominiq at lps dot ens dot fr]

------- Comment #17 from dominiq at lps dot ens dot fr  2010-05-22 13:31 -------
> is : http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44229
> potentially a similar problem?

It does not look like: the patch in comment #6 does not fix pr44229.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #18 from jakub at gcc dot gnu dot org  2010-05-26 06:01 -------
Subject: Bug 44199

Author: jakub
Date: Wed May 26 06:00:44 2010
New Revision: 159853

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159853
Log:
        PR target/44199
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): If cfun->calls_alloca
        or total_size is larger than red zone size for non-V4 ABI, emit a
        stack_tie resp. frame_tie insn before stack pointer restore.
        * config/rs6000/rs6000.md (frame_tie): New insn.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/config/rs6000/rs6000.md


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #19 from jakub at gcc dot gnu dot org  2010-05-26 06:02 -------
Subject: Bug 44199

Author: jakub
Date: Wed May 26 06:02:30 2010
New Revision: 159854

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159854
Log:
        PR target/44199
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): If cfun->calls_alloca
        or total_size is larger than red zone size for non-V4 ABI, emit a
        stack_tie resp. frame_tie insn before stack pointer restore.
        * config/rs6000/rs6000.md (frame_tie): New insn.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-4_5-branch/gcc/config/rs6000/rs6000.md


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #20 from jakub at gcc dot gnu dot org  2010-05-26 06:05 -------
Subject: Bug 44199

Author: jakub
Date: Wed May 26 06:05:29 2010
New Revision: 159855

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159855
Log:
        PR target/44199
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): If cfun->calls_alloca
        or total_size is larger than red zone size for non-V4 ABI, emit a
        stack_tie resp. frame_tie insn before stack pointer restore.
        * config/rs6000/rs6000.md (frame_tie): New insn.

Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-4_4-branch/gcc/config/rs6000/rs6000.md


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #21 from jakub at gcc dot gnu dot org  2010-05-26 06:07 -------
Should be fixed now.


-- 

jakub at gcc dot gnu dot org changed:

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


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[pthaugen at gcc dot gnu dot org]

------- Comment #22 from pthaugen at gcc dot gnu dot org  2010-05-26 15:51 -------
The 4.4 patch isn't complete.

/home/gccbuild/gcc_4.4_anonsvn/gcc/gcc/config/rs6000/rs6000.c:17166: undefined
reference to `offset_below_red_zone_p'
/home/gccbuild/gcc_4.4_anonsvn/gcc/gcc/config/rs6000/rs6000.c:17188: undefined
reference to `offset_below_red_zone_p'


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #23 from jakub at gcc dot gnu dot org  2010-05-26 16:09 -------
Subject: Bug 44199

Author: jakub
Date: Wed May 26 16:09:25 2010
New Revision: 159878

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159878
Log:
        PR target/44199
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): Fix up a backport
        glitch.

Modified:
    branches/gcc-4_4-branch/gcc/config/rs6000/rs6000.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #24 from jakub at gcc dot gnu dot org  2010-05-26 16:10 -------
Oops sorry, forgot redhat/gcc-4_4-branch has this function backported.
Fixed now.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #25 from bergner at gcc dot gnu dot org  2010-05-27 16:31 -------
Subject: Bug 44199

Author: bergner
Date: Thu May 27 16:31:05 2010
New Revision: 159930

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159930
Log:
        Backport from mainline:

        2010-05-26  Jakub Jelinek  <jakub@redhat.com>

        PR target/44199
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): If cfun->calls_alloca
        or total_size is larger than red zone size for non-V4 ABI, emit a
        stack_tie resp. frame_tie insn before stack pointer restore.
        * config/rs6000/rs6000.md (frame_tie): New insn.

Modified:
    branches/ibm/gcc-4_4-branch/gcc/ChangeLog.ibm
    branches/ibm/gcc-4_4-branch/gcc/config/rs6000/rs6000.c
    branches/ibm/gcc-4_4-branch/gcc/config/rs6000/rs6000.md


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199

[Bug target/44199] ppc64 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #26 from bergner at gcc dot gnu dot org  2010-06-02 15:40 -------
Subject: Bug 44199

Author: bergner
Date: Wed Jun  2 15:40:09 2010
New Revision: 160160

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=160160
Log:
        Backport from GCC 4.4:

        2010-05-26  Jakub Jelinek  <jakub@redhat.com>

        PR target/44199
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): Fix up a backport
        glitch.

Modified:
    branches/ibm/gcc-4_4-branch/gcc/ChangeLog.ibm
    branches/ibm/gcc-4_4-branch/gcc/config/rs6000/rs6000.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44199