[Bug c/36043] New: gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[Bug c/36043] New: gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[bartoschek at gmx dot de]

For the following programm gcc produces wrong code. To pass the struct colour
by value gcc reads 8 bytes although the struct has a size of 6. This causes
reads after allocated memory. In the example program the memory is mmap'ed and
the last element passed. The 8 byte read crosses page boundaries and causes a
segmentation violation. The code in question is:

        movq    -8(%rbp), %rax
        addq    $12282, %rax
        movq    (%rax), %rdi
        call    print_colour

The error also occurs if you malloc the memory in question but it is harder to
provoke a segmentation violation.

The error occurs only in 64bit mode. It does not depend on the optimization
level. However if you want to reproduce it with higher optimization, you have
to move print_colour to a different compilation unit.

It could be reproduced on different gcc versions up to 4.3.0

#include <sys/mman.h>
#include <stdio.h>

struct colour
{
  unsigned short red;
  unsigned short green;
  unsigned short blue;
};

void print_colour(struct colour col)
{
  printf( "colour is %d %d %d\n",
          col.red, col.green, col.blue);
}

int main(void)
{
  struct colour * c = mmap(NULL, sizeof(struct colour) * 2048,
                           PROT_READ|PROT_WRITE,
                           MAP_PRIVATE|MAP_ANONYMOUS, 0, 0);
  if (c != MAP_FAILED) {
    c[2047].red = 115;
    c[2047].green = 122;
    c[2047].blue = 98;
    print_colour(c[2047]);
    munmap(c, sizeof(struct colour) * 2048);
  }
  return 0;
}

Here is the whole command line:

-bash-3.00$ LANG=C /lfs/user/bartosch/software/gcc/bin/gcc -v -save-temps -Wall
-W -pedantic  gls.c
Using built-in specs.
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-4.3.0/configure
--prefix=/lfs/user/bartosch/software/gcc --enable-languages=c,c++
--with-mpfr=/lfs/user/bartosch/software/gcc
Thread model: posix
gcc version 4.3.0 (GCC)
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-W' '-pedantic'
'-mtune=generic'
 /lfs/user/bartosch/software/gcc/libexec/gcc/x86_64-unknown-linux-gnu/4.3.0/cc1
-E -quiet -v gls.c -mtune=generic -Wall -W -pedantic -fpch-preprocess -o gls.i
ignoring nonexistent directory
"/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/../../../../x86_64-unknown-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/local/include
 /lfs/user/bartosch/software/gcc/include
 /lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/include

/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/include-fixed
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-W' '-pedantic'
'-mtune=generic'
 /lfs/user/bartosch/software/gcc/libexec/gcc/x86_64-unknown-linux-gnu/4.3.0/cc1
-fpreprocessed gls.i -quiet -dumpbase gls.c -mtune=generic -auxbase gls -Wall
-W -pedantic -version -o gls.s
GNU C (GCC) version 4.3.0 (x86_64-unknown-linux-gnu)
        compiled by GNU C version 4.3.0, GMP version 4.2.1, MPFR version 2.3.0.
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 261e5a68fb24a56eb3beaa6eb43384e4
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-W' '-pedantic'
'-mtune=generic'
 as -V -Qy -o gls.o gls.s
GNU assembler version 2.15.92.0.2 (x86_64-redhat-linux) using BFD version
2.15.92.0.2 20040927
COMPILER_PATH=/lfs/user/bartosch/software/gcc/libexec/gcc/x86_64-unknown-linux-gnu/4.3.0/:/lfs/user/bartosch/software/gcc/libexec/gcc/x86_64-unknown-linux-gnu/4.3.0/:/lfs/user/bartosch/software/gcc/libexec/gcc/x86_64-unknown-linux-gnu/:/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/:/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/
LIBRARY_PATH=/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/:/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-W' '-pedantic'
'-mtune=generic'

/lfs/user/bartosch/software/gcc/libexec/gcc/x86_64-unknown-linux-gnu/4.3.0/collect2
--eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2
/usr/lib/../lib64/crt1.o /usr/lib/../lib64/crti.o
/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/crtbegin.o
-L/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0
-L/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/../../../../lib64
-L/lib/../lib64 -L/usr/lib/../lib64
-L/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/../../..
gls.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s
--no-as-needed
/lfs/user/bartosch/software/gcc/lib/gcc/x86_64-unknown-linux-gnu/4.3.0/crtend.o
/usr/lib/../lib64/crtn.o
-bash-3.00$ ./a.out
Speicherzugriffsfehler
-bash-3.00$


-- 
           Summary: gcc reads 8 bytes for a struct of size 6 which leads to
                    sigsegv
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: bartoschek at gmx dot de
 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=36043

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2008-04-25 14:55 -------
Hmm, didn't we fix this? ...

        movw    $115, (%rax)
        movw    $122, 2(%rax)
        movw    $98, 4(%rax)
        movq    (%rax), %rdi
        call    print_colour


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org, matz at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2008-04-25 14:55:39
               date|                            |


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2008-04-25 14:59 -------
Ahm, not exactly a dup of PR31309.

Shorter (non-runtime) testcase:

struct colour
{
  unsigned short red;
  unsigned short green;
  unsigned short blue;
};

void print_colour(struct colour col);

void foo(struct colour *c)
{
  print_colour(*c);
}


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2008-04-25 15:08 -------
The problem is that struct colour is laid out like

    arg 0 <parm_decl 0x2b4f8506e2d0 c
        type <pointer_type 0x2b4f85160780 type <record_type 0x2b4f85160540
colour>
            unsigned DI
            size <integer_cst 0x2b4f85068b70 constant invariant 64>
            unit size <integer_cst 0x2b4f85068ba0 constant invariant 8>
            align 64 symtab 0 alias set -1 canonical type 0x2b4f85160780>
        used unsigned DI file t.c line 10 col 25 size <integer_cst
0x2b4f85068b70 64> unit size <integer_cst 0x2b4f85068ba0 8>
        align 64 context <function_decl 0x2b4f8515fea0 foo> initial
<pointer_type 0x2b4f85160780>
        (reg/v/f:DI 58 [ c ]) arg-type <pointer_type 0x2b4f85160780>
        incoming-rtl (reg:DI 5 di [ c ])>>

which doesn't match its natural alignment nor size.


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[matz at gcc dot gnu dot org]

------- Comment #4 from matz at gcc dot gnu dot org  2008-04-25 15:12 -------
That's the layout of 'c', a pointer variable.  We don't see the layout
of the record_type here.


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2008-04-25 15:17 -------
Forget that.  load_register_parameters converts

(mem/s:BLK (reg/v/f:DI 58 [ c ]) [0 S6 A16])

to

(mem/s:BLK (reg/v/f:DI 58 [ c ]) [0 S6 A16])

here:

          else if (partial == 0 || args[i].pass_on_stack)
            {
              rtx mem = validize_mem (args[i].value);


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2008-04-25 15:29 -------
Errm.  change_address_1 simply builds a DImode mem (with alignment
info properly retained)

#0  0x00000000005e80c2 in adjust_address_1 (memref=0x2b0751d81520, 
    mode=DImode, offset=0, validate=0, adjust=1)
    at /space/rguenther/src/svn/gcc-4_3-branch/gcc/emit-rtl.c:1910
#1  0x00000000005e396c in operand_subword (op=0x2b0751d81520, offset=0, 
    validate_address=1, mode=BLKmode)
    at /space/rguenther/src/svn/gcc-4_3-branch/gcc/emit-rtl.c:1346
#2  0x00000000005e3a28 in operand_subword_force (op=0x2b0751d81520, offset=0, 
    mode=BLKmode)
    at /space/rguenther/src/svn/gcc-4_3-branch/gcc/emit-rtl.c:1374
#3  0x000000000060fd27 in move_block_to_reg (regno=5, x=0x2b0751d81520, 
    nregs=1, mode=BLKmode)
    at /space/rguenther/src/svn/gcc-4_3-branch/gcc/expr.c:1560
#4  0x000000000054f7b9 in load_register_parameters (args=0x7fff59711810, 
    num_actuals=1, call_fusage=0x7fff597119d0, flags=0, is_sibcall=0, 
    sibcall_failure=0x7fff597119ac)
    at /space/rguenther/src/svn/gcc-4_3-branch/gcc/calls.c:1681
#5  0x00000000005526b4 in expand_call (exp=0x2b075142d460, target=0x0, 
    ignore=1) at /space/rguenther/src/svn/gcc-4_3-branch/gcc/calls.c:2763

no idea if that is supposed to work (and thus *movdi_1_rex64 shouldn't match)
or if not ...


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2008-04-25 15:43 -------
So, the problem is in move_block_to_reg that copies the argument piecewise
to regs like

  for (i = 0; i < nregs; i++)
    emit_move_insn (gen_rtx_REG (word_mode, regno + i),
                    operand_subword_force (x, i, mode));

not accounting for the fact that the last reg may be only partially filled
from the memory.  But the necessary information to deal with this is also
not available in this function currently.


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[matz at gcc dot gnu dot org]

------- Comment #8 from matz at gcc dot gnu dot org  2008-04-25 16:15 -------
FWIW, I think the error is in the caller of move_block_to_reg. 
move_block_to_reg can make use of a load_multiple instruction, which really
loads full regs.  I.e. it would be unreasonable to require changes in
move_block_to_reg to handle non-power-of-2 sizes.  Hence the caller
(load_register_parameters) needs to handle this.  I'm not sure if the
n_aligned_regs thingy could be misused for this, or if one simply should
opencode the special case of the last register being partial.


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2008-04-25 16:20 -------
Index: calls.c
===================================================================
--- calls.c     (revision 134659)
+++ calls.c     (working copy)
@@ -2708,7 +2708,7 @@ expand_call (tree exp, rtx target, int i
         and whose alignment does not permit a direct copy into registers,
         make a group of pseudos that correspond to each register that we
         will later fill.  */
-      if (STRICT_ALIGNMENT)
+      if (1 || STRICT_ALIGNMENT)
        store_unaligned_arguments_into_pseudos (args, num_actuals);

       /* Now store any partially-in-registers parm.

also fixes this.


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[eric dot rannaud at gmail dot com]

------- Comment #10 from eric dot rannaud at gmail dot com  2009-03-17 22:48 -------
Witnessed in g++ 4.3.2

g++ (GCC) 4.3.2 20081105 (Red Hat 4.3.2-7)


-- 

eric dot rannaud at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |eric dot rannaud at gmail
                   |                            |dot com


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #11 from rguenth at gcc dot gnu dot org  2009-03-18 09:11 -------
Still broken on the trunk as well.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.1.2 4.3.3 4.4.0


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[lordhoto at gmail dot com]

------- Comment #12 from lordhoto at gmail dot com  2009-09-15 21:24 -------
Happens for me too on Linux/AMD64 with:

gcc (Debian 4.3.4-2) 4.3.4

and

gcc (Debian 4.4.1-4) 4.4.1

It also happens with structs of sizes 3, 5 and 7 for me.

An easy (non-runtime) test case for different struct sizes can be made via:

struct test
{
  unsigned char size[TEST_SIZE];
};

void bar(struct test);

void foo(struct test *t)
{
  bar(*t);
}

Just define the struct size via "-DTEST_SIZE=size", where "size" is the desired
size of the struct, on the gcc command line.

One can witness that for struct sizes of 1, 2, 4, 8 and everything above 8 it
works just fine.

Here's the assembly output, generated via 4.3.4, for the working struct sizes:

struct size of 1:

        movq    -8(%rbp), %rax
        movzbl  (%rax), %edi
        call    bar

struct size of 2:

        movq    -8(%rbp), %rax
        movzwl  (%rax), %edi
        call    bar


struct size of 4:

        movq    -8(%rbp), %rax
        movl    (%rax), %edi
        call    bar


struct size of 8:

        movq    -8(%rbp), %rax
        movq    (%rax), %edi
        call    bar

For all sizes above 8, it does also generate correct assembly, that means it
will only read the correct number of bytes. Here is an example with a struct
size of 11:

        movq    -8(%rbp), %rdx
        movq    (%rdx), %rdi
        movzbl  8(%rdx), %ecx
        movzbl  9(%rdx), %eax
        salq    $8, %rax
        orq     %rax, %rcx
        movzbl  10(%rdx), %eax
        salq    $16, %rax
        movq    %rax, %rsi
        orq     %rcx, %rsi
        call    bar

As you can see it uses one "movq" to read eight bytes at once and then three
"movzbl" to read the remaining three bytes. The assembly for this case was
generated with -O0. It will look differently for other optimization levels, but
is still using the same basic logic.

For struct sizes of 3, 5, 6 or 7 the compiler always generates:

        movq    -8(%rbp), %rax
        movq    (%rax), %rdi
        call    bar

for me, which is the reported incorrect behavior.


-- 

lordhoto at gmail dot com changed:

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


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[a dot heider at gmail dot com]

------- Comment #13 from a dot heider at gmail dot com  2009-09-15 21:54 -------
Still present on a recent GCC snapshot:

Johannes' Code compiled with "(Ubuntu 20090912-1ubuntu2) 4.5.0 20090912
(experimental) [trunk revision 151650]" and -O0 -DTEST_SIZE=5:

        subq    $16, %rsp
        movq    %rdi, -8(%rbp)
        movq    -8(%rbp), %rax
        movq    (%rax), %rdi
        call    bar


-- 

a dot heider at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |a dot heider at gmail dot
                   |                            |com


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[matt at use dot net]

------- Comment #14 from matt at use dot net  2010-01-03 20:30 -------
Still happening on 4.5.0 20091228 (and gcc 4.4.1) on Ubuntu 9.10/amd64). I
found that it goes away in 3 separate instances when turning on
-finline-functions. The last 2 instances of memory corruption I am running into
are functions in vtables, which can't be inlined (and therefore work around
this bug) until de-virtualization is eliminated.

This seems like a pretty serious and common problem; is there any chance it
will be fixed in 4.5?


-- 


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

[Bug target/36043] gcc reads 8 bytes for a struct of size 6 which leads to sigsegv

[rguenth at gcc dot gnu dot org]

------- Comment #15 from rguenth at gcc dot gnu dot org  2010-01-03 22:32 -------
The issue is in a very twisted piece of GCC where the chances to break sth are
bigger that to fix sth ;)

And it isn't a regression, so it's not on too many peoples radar (nor does
it seem to happen in practice and thus affect those who may be able to fix it).

But, let's CC some more people ;)


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rth at gcc dot gnu dot org
      Known to fail|4.1.2 4.3.3 4.4.0           |4.1.2 4.3.3 4.4.0 4.5.0


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