[Bug rtl-optimization/23561] New: nonoverlapping_memrefs_p returns true even for overlapping memory references

[Bug rtl-optimization/23561] New: nonoverlapping_memrefs_p returns true even for overlapping memory references

[jakub at gcc dot gnu dot org]

struct A
{
  char a1[1];
  char a2[5];
  char a3[1];
  char a4[2048 - 7];
} a;

typedef __SIZE_TYPE__ size_t;
extern void *memset (void *, int, size_t);
extern void *memcpy (void *, const void *, size_t);
extern int memcmp (const void *, const void *, size_t);
extern void abort (void);

void
bar (struct A *x)
{
  size_t i;
  if (memcmp (x, "\1HELLO\1", sizeof "\1HELLO\1"))
    abort ();
  for (i = 0; i < sizeof (x->a4); i++)
    if (x->a4[i])
      abort ();
}

int
foo (void)
{
  memset (&a, 0, sizeof (a));
  a.a1[0] = 1;
  memcpy (a.a2, "HELLO", sizeof "HELLO");
  a.a3[0] = 1;
  bar (&a);
  return 0;
}

int
main (void)
{
  foo ();
  return 0;
}

is miscompiled on ppc-linux at -O2 and -O3 (assuming the testcase is valid).
The 2 memcpy (a.a2, ...) instructions get swapped with a.a3[0] = 1 insn during
sched2, because nonoverlapping_memrefs_p says:
(mem/s:HI (plus:SI (reg/f:SI 29 29 [120]) (const_int 5 [0x5])) [0 a.a2+4 S2 A8])
and
(mem/s:QI (plus:SI (reg/f:SI 29 29 [120]) (const_int 6 [0x6])) [0 a.a3+0 S1 A8])
don't overlap (as they have recorded different fields of the same structure).
The patch that introduced this optimization was:
http://gcc.gnu.org/ml/gcc-patches/2001-12/msg00072.html
Now, is that valid C to overflow from one field into another one within the
same structure?  If yes, I think nonoverlapping_memrefs_p would need to take
into account offsets, sizes and relative distance of the fields.
If not, then perhaps glibc -D_FORTIFY_SOURCE=2 should use __builtin_offset_size
(dst, 1) rather than (dst, 0) even for memcpy/etc.

-- 
           Summary: nonoverlapping_memrefs_p returns true even for
                    overlapping memory references
           Product: gcc
           Version: 4.0.2
            Status: UNCONFIRMED
          Severity: critical
          Priority: P2
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
                CC: gcc-bugs at gcc dot gnu dot org,rth at gcc dot gnu dot
                    org
GCC target triplet: ppc-linux


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-08-25 15:38 -------
  memcpy (a.a2, "HELLO", sizeof "HELLO");
That is invalid as a.a2 is only 5 in size and "HELLO" is 6 in size.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[jakub at gcc dot gnu dot org]

------- Additional Comments From jakub at gcc dot gnu dot org  2005-08-25 15:47 -------
> That is invalid as a.a2 is only 5 in size and "HELLO" is 6 in size.
I know it overlaps into the next field.  Whether it is invalid is unclear to me.
How about memcpy ((char *) &a + offsetof (struct A, a2), "HELLO", 6)?
Is there supposed to be any semantic difference between the 2?

-- 


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

Re: [Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[Andrew Pinski]

> 
> 
> ------- Additional Comments From jakub at gcc dot gnu dot org  2005-08-25 15:47 -------
> > That is invalid as a.a2 is only 5 in size and "HELLO" is 6 in size.
> I know it overlaps into the next field.  Whether it is invalid is unclear to me.
> How about memcpy ((char *) &a + offsetof (struct A, a2), "HELLO", 6)?
> Is there supposed to be any semantic difference between the 2?

yes because one talks about the object a and the other one talks about the object
a.a2.

This is at least as I understand what the C standard says.

-- Pinski

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[pinskia at physics dot uc dot edu]

------- Additional Comments From pinskia at physics dot uc dot edu  2005-08-25 15:53 -------
Subject: Re:  nonoverlapping_memrefs_p returns true even for overlapping memory references

> 
> 
> ------- Additional Comments From jakub at gcc dot gnu dot org  2005-08-25 15:47 -------
> > That is invalid as a.a2 is only 5 in size and "HELLO" is 6 in size.
> I know it overlaps into the next field.  Whether it is invalid is unclear to me.
> How about memcpy ((char *) &a + offsetof (struct A, a2), "HELLO", 6)?
> Is there supposed to be any semantic difference between the 2?

yes because one talks about the object a and the other one talks about the object
a.a2.

This is at least as I understand what the C standard says.

-- Pinski


-- 


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2005-08-25 16:29 -------
Careful, Andrew.  Things are not as cut-and-dried as you're making it out.
Indeed, this is yet another example of the big structure member aliasing
discussion we had earlier this year.  I can't find the reference just now.

I vaguely seem to recall that for the nonce we decided to allow up-casts
from members back to the structure.

Which suggests that our string builtins should adjust the MEM_EXPR to
discard the member from the aliasing info, and refer only to the object.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at codesourcery dot com


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2005-08-25 16:35 -------
All that said, I personally would consider this a source code bug.  If you
really meant to initialize two members of the structure, I think it makes
logical sense that you refer to the object as a whole.  Otherwise we deny
the ability to apply sensible range checks to array members within objects.

And in the case in question, it's quite obviously an off-by-one bug on the
part of the programmer.  They did not really intend to initialize a3[0] twice.
So I think it would be useful if _FORTIFY_SOURCE complained about this usage
even if it turns out to be within the letter of the law.

-- 


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2005-08-25 17:22 -------
Subject: Re:  nonoverlapping_memrefs_p returns
 true even for overlapping memory references

rth at gcc dot gnu dot org wrote:

> And in the case in question, it's quite obviously an off-by-one bug on the
> part of the programmer.  They did not really intend to initialize a3[0] twice.
> So I think it would be useful if _FORTIFY_SOURCE complained about this usage
> even if it turns out to be within the letter of the law.

We did seem to reach the consensus that it was OK to upcast from a 
member of the structure to the containing structure, or, rather, that 
there was nothing that definitively made that invalid.

This is a bit different, in that the problematic memcpy is not 
mentioning a3 at all; it's just stepping on it.  I'm not sure whether 
this case is valid; my guess is that it is, simply in that the C 
standard says so little about the object model that one rather has to 
assume such things are legal.  Then again, you're not strictly pseaking 
allowed to index off the end of an array, so I'm not sure.

However, if memcpy were an arbitrary function, then by the conclusion in 
the first paragraph, it certainly might modify "a.a3".  So, the compiler 
must be making some special assumption about memcpy.  I'd suggest 
ceasing to make that assumption, in the name of caution.

I agree that in an error-checking capacity it makes sense to warn.  As 
RTH says, this is not something that programmers mean to do.



-- 


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[jakub at gcc dot gnu dot org]

------- Additional Comments From jakub at gcc dot gnu dot org  2005-08-25 19:01 -------
Ok, I'll play with get_memory_rtx.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jakub at gcc dot gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-08-25 19:01:04
               date|                            |


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-08-26 22:02 -------
Subject: Bug 23561

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jakub@gcc.gnu.org	2005-08-26 22:02:45

Modified files:
	gcc            : ChangeLog builtins.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: 20050826-1.c 

Log message:
	PR rtl-optimization/23561
	* builtins.c (get_memory_rtx): Add LEN argument.  If MEM_EXPR is
	a COMPONENT_REF, remove all COMPONENT_REF from MEM_EXPR unless
	at most LEN bytes long memory fits into the field.
	(expand_builtin_memcpy, expand_builtin_mempcpy, expand_movstr,
	expand_builtin_strncpy, expand_builtin_memset, expand_builtin_memcmp,
	expand_builtin_strcmp, expand_builtin_strncmp): Adjust callers.
	
	* gcc.c-torture/execute/20050826-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9833&r2=2.9834
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&r1=1.472&r2=1.473
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5968&r2=1.5969
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20050826-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-08-27 09:41 -------
Subject: Bug 23561

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	jakub@gcc.gnu.org	2005-08-27 09:41:08

Modified files:
	gcc            : ChangeLog builtins.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: 20050826-1.c 

Log message:
	PR rtl-optimization/23561
	* builtins.c (get_memory_rtx): Add LEN argument.  If MEM_EXPR is
	a COMPONENT_REF, remove all COMPONENT_REF from MEM_EXPR unless
	at most LEN bytes long memory fits into the field.
	(expand_builtin_memcpy, expand_builtin_mempcpy, expand_movstr,
	expand_builtin_strncpy, expand_builtin_memset, expand_builtin_memcmp,
	expand_builtin_strcmp, expand_builtin_strncmp): Adjust callers.
	
	* gcc.c-torture/execute/20050826-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.393&r2=2.7592.2.394
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.426.2.3&r2=1.426.2.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.353&r2=1.5084.2.354
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20050826-1.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.2.1



-- 


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[jakub at gcc dot gnu dot org]

------- Additional Comments From jakub at gcc dot gnu dot org  2005-09-02 08:49 -------
Should be fixed on 4.0/HEAD.

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


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

[Bug rtl-optimization/23561] nonoverlapping_memrefs_p returns true even for overlapping memory references

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.0.2


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