[Bug tree-optimization/60452] New: wrong code at -O1 and above on x86_64-linux-gnu (affecting trunk and 4.8.x)

[Bug tree-optimization/60452] New: wrong code at -O1 and above on x86_64-linux-gnu (affecting trunk and 4.8.x)

[su at cs dot ucdavis.edu]

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

            Bug ID: 60452
           Summary: wrong code at -O1 and above on x86_64-linux-gnu
                    (affecting trunk and 4.8.x)
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: su at cs dot ucdavis.edu

The current gcc trunk and 4.8.x mis-compile the following code on x86_64-linux
at -O1 in both 32-bit and 64-bit modes. 

This is a regression from 4.7.x. 


$ gcc-trunk -v
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-trunk/configure --prefix=/usr/local/gcc-trunk
--enable-languages=c,c++ --disable-werror --enable-multilib
Thread model: posix
gcc version 4.9.0 20140306 (experimental) [trunk revision 208366] (GCC) 
$ 
$ gcc-trunk -O0 small.c; a.out
$ gcc-trunk -O2 small.c; a.out
$ gcc-4.7.3 -O1 small.c; a.out
$ 
$ gcc-trunk -O1 small.c; a.out
Segmentation fault (core dumped)
$ gcc-4.8.2 -O1 small.c; a.out
Segmentation fault (core dumped)
$ 


-------------------------------


int a, c, d;
short b;

void
fn1 (int p)
{
}

int
fn2 ()
{
  return 0;
}

int
main ()
{
  int e[2] = { 0, 0 };
  for (; b < 1; b++)
    {
      for (; c;)
    if (e[0])
      break;
      fn1 (e[b]);
      for (; c; c++)
    e[0] = 0;
      d = fn2 ();
      if (b == (d | 2837))
    a = e[b];
    }
  return 0;
}

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-03-07
                 CC|                            |jakub at gcc dot gnu.org
   Target Milestone|---                         |4.8.3
            Summary|wrong code at -O1 on        |[4.8/4.9 Regression] wrong
                   |x86_64-linux-gnu (affecting |code at -O1 on
                   |trunk and 4.8.x)            |x86_64-linux-gnu (affecting
                   |                            |trunk and 4.8.x)
     Ever confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r192946.  Looking into it.

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P2

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Can be simplified into:
int a;

int
main ()
{
  int e[2] = { 0, 0 }, f = 0;
  if (a == 131072)
    f = e[a];
  return f;
}
which then starts to crash even starting from 4.3 or so (in between r125500 and
r126000).

The problem is that ifcvt.c doesn't consider e[131072], clearly out of bound
access to an automatic array, as possibly trapping/faulting.

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If I modify the shorter testcase such that int e[2]; is a static array, then we
don't generate cmove for it, because on:
(mem:SI (const:DI (plus:DI (symbol_ref:DI ("e")  <var_decl 0x7ffff19d6098 e>)
            (const_int 524288 [0x80000]))) [2 e+524288 S4 A32])
may_trap_or_fault_p returns true (correctly).  But in the case of automatic
out-of-bound access we get instead:
(mem/c:SI (plus:DI (reg/f:DI 20 frame)
        (const_int 524272 [0x7fff0])) [2  S4 A128])
and there may_trap_or_fault_p really can't (easily) know if it is valid or not,
there is no MEM_EXPR even.  This is because in the #c2 testcase as well as the
original one DECL_RTL of e variable is a register, not MEM.
But even if I try:
int a;
__attribute__((noinline, noclone)) void
foo (int *e)
{
  asm volatile ("" : : "r" (e) : "memory");
}

int
main ()
{
  int e[2] = { 0, 0 }, f = 0;
  if (a == 131072)
    f = e[a];
  foo (e);
  return f;
}
where we have:
(mem:SI (plus:DI (reg/f:DI 20 frame)
        (const_int 524272 [0x7fff0])) [2 e+524288 S4 A128])
instead and thus from MEM_EXPR we perhaps could find out that it is an out of
bound access, we still always treat all frame based accesses (whatever the
offset is) as non-trapping.
So perhaps we need to handle known out of bound MEMs specially when we find
that fact out (if we want to emit them into the RTL IL at all), one thing is
expansion, another thing if say initially non-constant offset is later
CSEd/forwprop etc. into constant out of bound offset.

Thoughts?

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Perhaps some new flag (MEM access will always fault?) or something similar.

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[mikpelinux at gmail dot com]

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

Mikael Pettersson <mikpelinux at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpelinux at gmail dot com

--- Comment #5 from Mikael Pettersson <mikpelinux at gmail dot com> ---
(In reply to Jakub Jelinek from comment #2)
> The problem is that ifcvt.c doesn't consider e[131072], clearly out of bound
> access to an automatic array, as possibly trapping/faulting.

Thus this one looks related to PR50588.

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[ebotcazou at gcc dot gnu.org]

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

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> But even if I try:
> int a;
> __attribute__((noinline, noclone)) void
> foo (int *e)
> {
>   asm volatile ("" : : "r" (e) : "memory");
> }
> 
> int
> main ()
> {
>   int e[2] = { 0, 0 }, f = 0;
>   if (a == 131072)
>     f = e[a];
>   foo (e);
>   return f;
> }
> where we have:
> (mem:SI (plus:DI (reg/f:DI 20 frame)
>         (const_int 524272 [0x7fff0])) [2 e+524288 S4 A128])
> instead and thus from MEM_EXPR we perhaps could find out that it is an out
> of bound access, we still always treat all frame based accesses (whatever
> the offset is) as non-trapping.
> So perhaps we need to handle known out of bound MEMs specially when we find
> that fact out (if we want to emit them into the RTL IL at all), one thing is
> expansion, another thing if say initially non-constant offset is later
> CSEd/forwprop etc. into constant out of bound offset.
> 
> Thoughts?

Again quite an artificial testcase...  What about adding a comparison of the
offset with the result of get_frame_size if the base register is SFP/HFP/SP?

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #6)
> > But even if I try:
> > int a;
> > __attribute__((noinline, noclone)) void
> > foo (int *e)
> > {
> >   asm volatile ("" : : "r" (e) : "memory");
> > }
> > 
> > int
> > main ()
> > {
> >   int e[2] = { 0, 0 }, f = 0;
> >   if (a == 131072)
> >     f = e[a];
> >   foo (e);
> >   return f;
> > }
> > where we have:
> > (mem:SI (plus:DI (reg/f:DI 20 frame)
> >         (const_int 524272 [0x7fff0])) [2 e+524288 S4 A128])
> > instead and thus from MEM_EXPR we perhaps could find out that it is an out
> > of bound access, we still always treat all frame based accesses (whatever
> > the offset is) as non-trapping.
> > So perhaps we need to handle known out of bound MEMs specially when we find
> > that fact out (if we want to emit them into the RTL IL at all), one thing is
> > expansion, another thing if say initially non-constant offset is later
> > CSEd/forwprop etc. into constant out of bound offset.
> > 
> > Thoughts?
> 
> Again quite an artificial testcase...  What about adding a comparison of the
> offset with the result of get_frame_size if the base register is SFP/HFP/SP?

But what would be safe positive/negative offsets from frame_pointer?
I mean, e.g. size of arguments is not included in the frame size, so size of
arguments would need to be taken into account too, plus does the middle-end
really know all the biases etc.?

[Bug tree-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[ebotcazou at gcc dot gnu.org]

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

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> But what would be safe positive/negative offsets from frame_pointer?
> I mean, e.g. size of arguments is not included in the frame size, so size of
> arguments would need to be taken into account too, plus does the middle-end
> really know all the biases etc.?

No, that would be either conservative or not bullet-proof, at least if used
alone.  Maybe compare MEM_OFFSET and get_frame_size and return true if the
former is larger than the latter.  Why do we drop the MEM_EXPR if the DECL_RTL
is a reg?

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|tree-optimization           |rtl-optimization

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
expand_expr_real_1 for the ARRAY_REF in question does:
        /* If we have either an offset, a BLKmode result, or a reference
           outside the underlying object, we must force it to memory.  
           Such a case can occur in Ada if we have unchecked conversion
           of an expression from a scalar type to an aggregate type or 
           for an ARRAY_RANGE_REF whose type is BLKmode, or if we were 
           passed a partially uninitialized object or a view-conversion
           to a larger size.  */
        must_force_mem = (offset
                          || mode1 == BLKmode
                          || bitpos + bitsize > GET_MODE_BITSIZE (mode2));
...
        /* Otherwise, if this is a constant or the object is not in memory
           and need be, put it there.  */
        else if (CONSTANT_P (op0) || (!MEM_P (op0) && must_force_mem))
          {
            tree nt = build_qualified_type (TREE_TYPE (tem),
                                            (TYPE_QUALS (TREE_TYPE (tem))
                                             | TYPE_QUAL_CONST));
            memloc = assign_temp (nt, 1, 1);
            emit_move_insn (memloc, op0);
            op0 = memloc;   
            mem_attrs_from_type = true;
          }

op0 is DECL_RTL of the array, (reg/v:DI 85 [ e ]), bitpos is 0x400000, bitsize
is 32, mode2 is DImode.

Not sure if it is safe to set MEM_EXPR (etc. on assign_temp result, if it is,
we could do that.  Note that MEM_NOTRAP_P is set on it (I assume it is fine
too, because we should consider it only when not out of bound access).
In any case, as the #c3 testcase shows, even when we do have MEM_EXPR and could
see that it is out of bound, we don't use that info at all.

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[ebotcazou at gcc dot gnu.org]

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

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

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

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
I'll try to find something plausible.

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> If I modify the shorter testcase such that int e[2]; is a static array, then
> we don't generate cmove for it, because on:
> (mem:SI (const:DI (plus:DI (symbol_ref:DI ("e")  <var_decl 0x7ffff19d6098 e>)
>             (const_int 524288 [0x80000]))) [2 e+524288 S4 A32])
> may_trap_or_fault_p returns true (correctly).  But in the case of automatic
> out-of-bound access we get instead:
> (mem/c:SI (plus:DI (reg/f:DI 20 frame)
>         (const_int 524272 [0x7fff0])) [2  S4 A128])
> and there may_trap_or_fault_p really can't (easily) know if it is valid or
> not, there is no MEM_EXPR even.  This is because in the #c2 testcase as well
> as the original one DECL_RTL of e variable is a register, not MEM.
> But even if I try:
> int a;
> __attribute__((noinline, noclone)) void
> foo (int *e)
> {
>   asm volatile ("" : : "r" (e) : "memory");
> }
> 
> int
> main ()
> {
>   int e[2] = { 0, 0 }, f = 0;
>   if (a == 131072)
>     f = e[a];
>   foo (e);
>   return f;
> }
> where we have:
> (mem:SI (plus:DI (reg/f:DI 20 frame)
>         (const_int 524272 [0x7fff0])) [2 e+524288 S4 A128])
> instead and thus from MEM_EXPR we perhaps could find out that it is an out
> of bound access, we still always treat all frame based accesses (whatever
> the offset is) as non-trapping.
> So perhaps we need to handle known out of bound MEMs specially when we find
> that fact out (if we want to emit them into the RTL IL at all), one thing is
> expansion, another thing if say initially non-constant offset is later
> CSEd/forwprop etc. into constant out of bound offset.
> 
> Thoughts?

Well, may_trap_or_fault_p has to be conservative.  See how tree_could_trap_p
handles bound violations.  If you can't prove it isn't out-of-bounds
then you have to assume it may be.

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 on x86_64-linux-gnu (affecting trunk and 4.8.x)

[ebotcazou at gcc dot gnu.org]

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

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
This also happens for regular objects on stack, i.e. with a MEM_EXPR:

int a;

int
main (void)
{
  char e[3] = { 0, 0, 0 }, f = 0;
  if (a == 131072)
    f = e[a];
  return f;
}

But I think that using MEM_OFFSET would be too conservative as well, for
example if the MEM_EXPR is a structure with a flexible array member.  So I
think that we should go for something based on the real offset from SFP/HPF/SP
and the value of get_frame_size plus some bias, even if there might be a gap
with the actual end of the frame, that doesn't really matter.

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 with large offsets in the frame

[ebotcazou at gcc dot gnu.org]

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

--- Comment #13 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Author: ebotcazou
Date: Wed Mar 26 07:38:30 2014
New Revision: 208837

URL: http://gcc.gnu.org/viewcvs?rev=208837&root=gcc&view=rev
Log:
    PR rtl-optimization/60452
    * rtlanal.c (rtx_addr_can_trap_p_1): Fix head comment.
    <case REG>: Return 1 for invalid offsets from the frame pointer.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/20140326-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/rtlanal.c
    trunk/gcc/testsuite/ChangeLog

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 with large offsets in the frame

[ebotcazou at gcc dot gnu.org]

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

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Author: ebotcazou
Date: Wed Mar 26 07:42:30 2014
New Revision: 208838

URL: http://gcc.gnu.org/viewcvs?rev=208838&root=gcc&view=rev
Log:
    PR rtl-optimization/60452
    * rtlanal.c (rtx_addr_can_trap_p_1): Fix head comment.
    <case REG>: Return 1 for invalid offsets from the frame pointer.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/20140326-1.c
      - copied unchanged from r208837,
trunk/gcc/testsuite/gcc.c-torture/execute/20140326-1.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/rtlanal.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 with large offsets in the frame

[ebotcazou at gcc dot gnu.org]

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

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

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

--- Comment #15 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Thanks for reporting the problem.

[Bug rtl-optimization/60452] [4.8/4.9 Regression] wrong code at -O1 with large offsets in the frame

[ebotcazou at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |newsgrp at duradsl dot dyndns.org

--- Comment #16 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
*** Bug 50588 has been marked as a duplicate of this bug. ***