[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2015-04-30 Thread amodra at gmail dot com 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744
Bug 58744 depends on bug 36043, which changed state.

Bug 36043 Summary: gcc reads 8 bytes for a struct of size 6 which leads to 
sigsegv
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36043

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2015-04-30 Thread amodra at gmail dot com 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744

Alan Modra amodra at gmail dot com changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED
   Target Milestone|--- |5.2

--- Comment #6 from Alan Modra amodra at gmail dot com ---
Fixed for 5.2

[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2015-04-30 Thread amodra at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744

--- Comment #5 from Alan Modra amodra at gcc dot gnu.org ---
Author: amodra
Date: Thu Apr 30 11:11:34 2015
New Revision: 222616

URL: https://gcc.gnu.org/viewcvs?rev=222616root=gccview=rev
Log:
gcc/
PR target/65408
PR target/58744
PR middle-end/36043
* calls.c (load_register_parameters): Don't load past end of
mem unless suitably aligned.
gcc/testsuite/
* gcc.dg/pr65408.c: New.


Added:
branches/gcc-5-branch/gcc/testsuite/gcc.dg/pr65408.c
Modified:
branches/gcc-5-branch/gcc/ChangeLog
branches/gcc-5-branch/gcc/calls.c
branches/gcc-5-branch/gcc/testsuite/ChangeLog

[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2015-04-15 Thread amodra at gcc dot gnu.org 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744

--- Comment #4 from Alan Modra amodra at gcc dot gnu.org ---
Author: amodra
Date: Wed Apr 15 07:29:01 2015
New Revision: 222115

URL: https://gcc.gnu.org/viewcvs?rev=222115root=gccview=rev
Log:
PR target/65408
PR target/58744
PR middle-end/36043
* calls.c (load_register_parameters): Don't load past end of
mem unless suitably aligned.


Added:
trunk/gcc/testsuite/gcc.dg/pr65408.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/calls.c
trunk/gcc/testsuite/ChangeLog

[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2015-03-13 Thread amodra at gmail dot com 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744

Alan Modra amodra at gmail dot com changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |amodra at gmail dot com

[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2013-10-28 Thread marcovanotti15+gcc at gmail dot com 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744

--- Comment #3 from Marco Vanotti marcovanotti15+gcc at gmail dot com ---
(In reply to Richard Biener from comment #2)
 Confirmed.  On i?86 we properly do a 16byte and a 8byte access (but we copy
 to stack anyway).


Yes, if the value is passed on the stack, it gets copied right. (For example,
if it is the seventh parameter of a function, it will be passed on the stack
and will be copied right).

The thing is that in the x86_64 calling convention it has to be passed on
registers while the are available (rdi, rsi, rdx, rcx, r8 and r9).

Reading the source code, precisely the gcc/calls.c file:
http://gcc.gnu.org/viewcvs/gcc/trunk/gcc/calls.c?revision=203967view=markup

---

The params that are passed on the stack are handled in line 3027, which says:

/* Now store (and compute if necessary) all non-register parms.
These come before register parms, since they can require block-moves,
which could clobber the registers used for register parms.
Parms which have partial registers are not stored here,
but we do preallocate space here if they want that.  */

Assuming that the registers may not require block-moves.

It uses the function store_one_arg to store the arg in the stack (it doesn't
work with a non-partial register).

---

After a while, the function load_register_parameters (line 1860) is called,
in this function, it falls in the case:

move_block_to_reg (REGNO (reg), mem, nregs, args[i].mode);

where nregs == 1.

So a whole register is copied.

---

I don't know how this issue should be fixed, should we copy the register into
pseudos before the load_register_parameters ? Or should we change the
move_block_to_reg function to make the right size of move instructions, for
x86_64 we don't need backup-registers, but maybe this bug is also in another
arch. 

size 3:

mov di, [rax]
sal rdi, 16
mov dil, [rax]

---

size 5:

mov edi, [rax]
sal rdi, 8
mov dil, [rax]

---

size 6:

mov edi, [rax]
sal rdi, 16
mov di, [rax]

---

size 7:

mov edi, [rax] ;move 4
sal rdi, 24
mov di, [rax]  ;move 3
sal rdi, 16
mov dil, [rax]

-

I would gladly submit a patch if I can get some advice on how this should be
fixed :)

[Bug target/58744] Illegal Memory Access on 3-byte packed struct ARCH: x86_64

2013-10-23 Thread rguenth at gcc dot gnu.org 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58744

Richard Biener rguenth at gcc dot gnu.org changed:

   What|Removed |Added

   Keywords||wrong-code
 Target||x86_64-*-*
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2013-10-23
  Component|c   |target
 Ever confirmed|0   |1
  Known to fail||4.9.0

--- Comment #2 from Richard Biener rguenth at gcc dot gnu.org ---
Confirmed.  On i?86 we properly do a 16byte and a 8byte access (but we copy to
stack anyway).

typedef struct{
unsigned char B;
unsigned char G;
unsigned char R;
} __attribute__((packed)) pixel ;

void apply_filter(pixel p);

void color_filter_c(pixel *src)
{
  apply_filter(*src);
}