[Bug target/44575] New: __builtin_va_arg overwrites into adjacent stack location

[Bug target/44575] New: __builtin_va_arg overwrites into adjacent stack location

[eraman at google dot com]

$ cat vararg.c
#include <stdlib.h>
#include <stdarg.h>
#include <string.h>

int fails = 0;
struct S116 { float a[3]; } ;
struct S116 a116[5];

void check116va (int z, ...)
{ struct S116 arg, *p;
  va_list ap;
  int j=0,k=0;
  int i;
  __builtin_va_start(ap,z);
  for (i = 2; i < 4; ++i) {
    p = NULL;
    j++;
    k+=2;
    switch ((z << 4) | i) {
      case 0x12: case 0x13: p = &a116[2]; arg = __builtin_va_arg(ap,struct
S116); break;
      default: ++fails; break;
    }
    if (p && p->a[2] != arg.a[2]) {
      ++fails;
    }
    if (fails)
      break;
  }
  __builtin_va_end(ap);
}
int main()
{
  memset (a116, '\0', sizeof (a116));
  a116[2].a[2] = -49026.625000;
  check116va (1, a116[2], a116[2]);
  if (fails)
    abort();
}

$ ./trunk-gcc -O0  vararg.c && ./a.out
Aborted

./trunk-gcc is gcc 4.6.0  configured with --target=x86_64-unknown-linux-gnu
--disable-nls --enable-threads=posix --enable-symvers=gnu --enable-__cxa_atexit
--enable-c99 --enable-long-long --with-gnu-as --with-gnu-ld
--build=x86_64-unknown-linux-gnu --host=x86_64-unknown-linux-gnu
--enable-checking=release --enable-multilib --enable-targets=all
--with-arch-32=pentium3 --with-tune-32=pentium4 
--enable-shared=libgcc,libmudflap,libssp,libstdc++,libgfortran
--with-pic=libgfortran --enable-languages=c,c++,fortran 
--with-native-system-header-dir=/include  --enable-linker-build-id 
--with-host-libstdcxx=-lstdc++ FCFLAGS='-g -O2 ' 

The test cases passes with gcc 4.2.4 and 4.4.3.  

The gimple for __builtin_va_arg (from vararg.c.004t.gimple ) contains

  addr.1 = &va_arg_tmp.4;
  addr.5 = (long unsigned int * {ref-all}) addr.1;
  sse_addr.6 = (long unsigned int *) sse_addr.3;
  D.3520 = *sse_addr.6;
  *addr.5 = D.3520;          ---> (1)      
  addr.7 = (long unsigned int * {ref-all}) addr.1;
  D.3522 = addr.7 + 8;
  sse_addr.8 = (long unsigned int *) sse_addr.3;
  D.3524 = sse_addr.8 + 16;
  D.3525 = *D.3524;
  *D.3522 = D.3525;         ---> (2)

The assignments  (1) and (2) above are 8 byte moves, one at offset 0 and
another at offset 8, into va_arg_tmp.4. But the size of va_arg_tmp.4 is 12
bytes (sizeof (struct S116)) resulting in overwriting of adjacent stack
location ( variable i in this case) leading to the failure.


-- 
           Summary: __builtin_va_arg overwrites into adjacent stack location
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: eraman at google dot com
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


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

[Bug target/44575] __builtin_va_arg overwrites into adjacent stack location

[jakub at gcc dot gnu dot org]

------- Comment #1 from jakub at gcc dot gnu dot org  2010-06-18 07:04 -------
Regressed with r146817 (SSA expand).


-- 


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

[Bug target/44575] [4.5/4.6 Regression] __builtin_va_arg overwrites into adjacent stack location

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
            Summary|__builtin_va_arg overwrites |[4.5/4.6 Regression]
                   |into adjacent stack location|__builtin_va_arg overwrites
                   |                            |into adjacent stack location
   Target Milestone|---                         |4.5.1


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

[Bug target/44575] [4.5/4.6 Regression] __builtin_va_arg overwrites into adjacent stack location

[matz at gcc dot gnu dot org]

------- Comment #2 from matz at gcc dot gnu dot org  2010-06-18 15:58 -------
It's not SSA expand (might be exposed by it, don't know), but the
bug is pre-existing already in 4.3:

  long unsigned int D.2219;
  struct S116 va_arg_tmp.3;
...
  addr.0 = &va_arg_tmp.3;
  addr.4 = (long unsigned int *) addr.0;
  sse_addr.5 = (long unsigned int *) sse_addr.2;
  D.2214 = *sse_addr.5;
  *addr.4 = D.2214;
  addr.6 = (long unsigned int *) addr.0;
  D.2216 = addr.6 + 8;
  sse_addr.7 = (long unsigned int *) sse_addr.2;
  D.2218 = sse_addr.7 + 16;
  D.2219 = *D.2218;
  *D.2216 = D.2219;

Here the second store is also a 8-byte store at offset 8 of a struct only
12 bytes long.  The problem is in ix86_gimplify_va_arg (and friends).
For the type in question (struct S116), we build this classes[] array:

(gdb) p regclass
$47 = {X86_64_SSE_CLASS, X86_64_SSE_CLASS, ...

That's okay, for such structs we really need two words, and both are passed
in registers (if available).  But the construct_container constructs this
container:

(gdb) p debug_rtx(container)
(parallel:BLK [
        (expr_list:REG_DEP_TRUE (reg:DI 21 xmm0)
            (const_int 0 [0]))
        (expr_list:REG_DEP_TRUE (reg:DI 22 xmm1)
            (const_int 8 [0x8]))
    ])

So, we try to move the content at offset 0 in DImode (the register itself
will be irrelevant here, as we're needing a temporary), which is still fine.
But the register of slot 1 also has DImode, for moving the values at offset
8.  This mode will be used to determine the type of the move instruction
generated, and is the one where things become wrong.  See the loop in 
ix86_gimplify_va_arg, starting here:

          for (i = 0; i < XVECLEN (container, 0); i++)
            {
            ...


-- 

matz at gcc dot gnu dot org changed:

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


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

[Bug target/44575] [4.5/4.6 Regression] __builtin_va_arg overwrites into adjacent stack location

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2010-06-21 12:49 -------
Created an attachment (id=20960)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20960&action=view)
gcc46-pr44575.patch

Untested fix.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jakub at gcc dot gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED


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

[Bug target/44575] [4.5/4.6 Regression] __builtin_va_arg overwrites into adjacent stack location

[jakub at gcc dot gnu dot org]

------- Comment #4 from jakub at gcc dot gnu dot org  2010-06-21 16:34 -------
Subject: Bug 44575

Author: jakub
Date: Mon Jun 21 16:33:49 2010
New Revision: 161097

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161097
Log:
        PR target/44575
        * config/i386/i386.c (ix86_gimplify_va_arg): When copying
        va_arg from a set of register save slots into a temporary,
        if the container is bigger than type size, do the copying
        using smaller mode or using memcpy.

        * gcc.c-torture/execute/pr44575.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr44575.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/44575] [4.5 Regression] __builtin_va_arg overwrites into adjacent stack location

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.0
           Priority|P3                          |P2


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

[Bug target/44575] [4.5 Regression] __builtin_va_arg overwrites into adjacent stack location

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2010-07-01 11:02 -------
Subject: Bug 44575

Author: jakub
Date: Thu Jul  1 11:01:58 2010
New Revision: 161660

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=161660
Log:
        Backport from mainline
        2010-06-21  Jakub Jelinek  <jakub@redhat.com>

        PR target/44575
        * config/i386/i386.c (ix86_gimplify_va_arg): When copying
        va_arg from a set of register save slots into a temporary,
        if the container is bigger than type size, do the copying
        using smaller mode or using memcpy.

        * gcc.c-torture/execute/pr44575.c: New test.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.c-torture/execute/pr44575.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/i386/i386.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug target/44575] [4.5 Regression] __builtin_va_arg overwrites into adjacent stack location

[jakub at gcc dot gnu dot org]

------- Comment #6 from jakub at gcc dot gnu dot org  2010-07-01 11:06 -------
Fixed.


-- 

jakub at gcc dot gnu dot org changed:

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


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

[Bug target/44575] [4.5 Regression] __builtin_va_arg overwrites into adjacent stack location

[eraman at google dot com]

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

--- Comment #7 from Easwaran Raman <eraman at google dot com> 2010-09-30 00:21:17 UTC ---
This is a variation of the same problem where __builtin_va_arg overwrites into
adjacent stack location [Not sure if I should reopen this bug or file a new
one]:

$ cat vararg.cc

#include <stdarg.h>
#include <stdlib.h>
struct S933 { struct{struct{}b[6];union{}c[7];}a;char d;char e; };

struct S933 arg;
void check933va (int z, ...) {
  char c;
  va_list ap;
  __builtin_va_start(ap,z);
  c = 'a';
  arg = __builtin_va_arg(ap,struct S933);
  if (c != 'a')
    abort();

}
int main() {
  struct S933 s933;
  check933va (1, s933);
}

$ ./trunk-g++  -O0  vararg.cc && ./a.out
Aborted

./trunk-g++ is GNU C++  version 4.6.0 20100924 (experimental)
(x86_64-unknown-linux-gnu)

The relevant portion of the gimple is below:
  D.2773_4 = ap.reg_save_area;
  D.2774_5 = ap.gp_offset;
  D.2775_6 = (long unsigned int) D.2774_5;
  int_addr.1_7 = D.2773_4 + D.2775_6;
  addr.0_8 = &va_arg_tmp.3;
  D.2777_9 = addr.0_8 + 8;
  D.2778_10 = MEM[(long unsigned int *)int_addr.1_7];
  *D.2777_9 = D.2778_10;    <--- Bad move

The move to address D.2777_9 is the problem

For this struct type, construct_container returns the following:

(parallel:BLK [
        (expr_list:REG_DEP_TRUE (reg:DI 0 ax)
            (const_int 8 [0x8]))
    ])

The destination of the move is at offset 8 (INTVAL (XEXP (slot, 1))) of the
temporary created. The size of the temp (sizeof(S933)) is 15 bytes and the move
is in DI mode. I think the problem is the check  if (prev_size + cur_size >
size) doesn't really check if the destination is overwritten.

[Bug target/44575] [4.5 Regression] __builtin_va_arg overwrites into adjacent stack location

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-10-01 13:13:36 UTC ---
Author: jakub
Date: Fri Oct  1 13:13:31 2010
New Revision: 164884

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164884
Log:
    Backport from mainline
    2010-09-30  Jakub Jelinek  <jakub@redhat.com>

    PR target/45843
    * config/i386/i386.c (ix86_gimplify_va_arg): Use
    INTVAL (XEXP (slot, 1)) as prev_size.

    2010-06-21  Jakub Jelinek  <jakub@redhat.com>

    PR target/44575
    * config/i386/i386.c (ix86_gimplify_va_arg): When copying
    va_arg from a set of register save slots into a temporary,
    if the container is bigger than type size, do the copying
    using smaller mode or using memcpy.

    Backport from mainline
    2010-09-30  Jakub Jelinek  <jakub@redhat.com>

    * g++.dg/torture/pr45843.C: New test.

    2010-06-21  Jakub Jelinek  <jakub@redhat.com>

    PR target/44575
    * gcc.c-torture/execute/pr44575.c: New test.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr45843.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr44575.c
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/config/i386/i386.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog