[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #18 from jakub at gcc dot gnu dot org 2007-06-26 11:47 --- Subject: Bug 29272 Author: jakub Date: Tue Jun 26 11:47:19 2007 New Revision: 126023 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=126023 Log: PR middle-end/29272 * builtins.c (fold_builtin_memset, fold_builtin_memory_op): Restrict single entry optimization to variables and components thereof. * gcc.c-torture/execute/20060930-2.c: New test. Added: branches/redhat/gcc-4_1-branch/gcc/testsuite/gcc.c-torture/execute/20060930-2.c Modified: branches/redhat/gcc-4_1-branch/gcc/ChangeLog branches/redhat/gcc-4_1-branch/gcc/builtins.c branches/redhat/gcc-4_1-branch/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #17 from cvs-commit at developer dot classpath dot org 2006-12-08 10:32 --- Subject: Bug 29272 CVSROOT:/cvsroot/classpath Module name:classpath Branch: classpath-0_93-branch Changes by: Mark Wielaard 06/12/08 10:31:50 Modified files: . : ChangeLog gnu/xml/dom: DomAttr.java DomNode.java gnu/xml/stream : SAXParser.java XMLStreamWriterImpl.java javax/xml/parsers: DocumentBuilderFactory.java javax/xml/validation: SchemaFactory.java Log message: 2006-12-06 Ben Konrath <[EMAIL PROTECTED]> Fixes PR 29853. * gnu/xml/dom/DomAttr.java: Don't report mutation if oldValue and newValue are the same. * gnu/xml/dom/DomNode.java: Set parent if null during mutation. 2006-12-06 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 29272. * javax/xml/parsers/DocumentBuilderFactory.java: Fix broken Javadoc. * gnu/xml/stream/SAXParser.java: Fix file descriptor leak. 2006-12-06 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 29264. * gnu/xml/stream/XMLStreamWriterImpl.java: Allow arbitrary text in writeDTD method. 2006-12-056 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 28816. * javax/xml/validation/SchemaFactory.java: Use correct algorithm to discover schema factory implementation class. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.8897.2.14&r2=1.8897.2.15 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/dom/DomAttr.java?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.3&r2=1.3.12.1 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/dom/DomNode.java?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.16&r2=1.16.2.1 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/stream/SAXParser.java?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.21&r2=1.21.4.1 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/stream/XMLStreamWriterImpl.java?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.5&r2=1.5.12.1 http://cvs.savannah.gnu.org/viewcvs/classpath/javax/xml/parsers/DocumentBuilderFactory.java?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.5&r2=1.5.10.1 http://cvs.savannah.gnu.org/viewcvs/classpath/javax/xml/validation/SchemaFactory.java?cvsroot=classpath&only_with_tag=classpath-0_93-branch&r1=1.5&r2=1.5.6.1 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #16 from cvs-commit at developer dot classpath dot org 2006-12-08 10:30 --- Subject: Bug 29272 CVSROOT:/cvsroot/classpath Module name:classpath Branch: generics-branch Changes by: Mark Wielaard 06/12/08 10:30:10 Modified files: . : ChangeLog gnu/xml/dom: DomAttr.java DomNode.java gnu/xml/stream : SAXParser.java XMLStreamWriterImpl.java javax/xml/parsers: DocumentBuilderFactory.java javax/xml/validation: SchemaFactory.java Log message: 2006-12-06 Ben Konrath <[EMAIL PROTECTED]> Fixes PR 29853. * gnu/xml/dom/DomAttr.java: Don't report mutation if oldValue and newValue are the same. * gnu/xml/dom/DomNode.java: Set parent if null during mutation. 2006-12-06 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 29272. * javax/xml/parsers/DocumentBuilderFactory.java: Fix broken Javadoc. * gnu/xml/stream/SAXParser.java: Fix file descriptor leak. 2006-12-06 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 29264. * gnu/xml/stream/XMLStreamWriterImpl.java: Allow arbitrary text in writeDTD method. 2006-12-056 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 28816. * javax/xml/validation/SchemaFactory.java: Use correct algorithm to discover schema factory implementation class. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&only_with_tag=generics-branch&r1=1.2386.2.358&r2=1.2386.2.359 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/dom/DomAttr.java?cvsroot=classpath&only_with_tag=generics-branch&r1=1.1.2.3&r2=1.1.2.4 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/dom/DomNode.java?cvsroot=classpath&only_with_tag=generics-branch&r1=1.1.2.10&r2=1.1.2.11 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/stream/SAXParser.java?cvsroot=classpath&only_with_tag=generics-branch&r1=1.14.2.4&r2=1.14.2.5 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/stream/XMLStreamWriterImpl.java?cvsroot=classpath&only_with_tag=generics-branch&r1=1.1.2.4&r2=1.1.2.5 http://cvs.savannah.gnu.org/viewcvs/classpath/javax/xml/parsers/DocumentBuilderFactory.java?cvsroot=classpath&only_with_tag=generics-branch&r1=1.1.2.5&r2=1.1.2.6 http://cvs.savannah.gnu.org/viewcvs/classpath/javax/xml/validation/SchemaFactory.java?cvsroot=classpath&only_with_tag=generics-branch&r1=1.1.2.5&r2=1.1.2.6 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #15 from cvs-commit at developer dot classpath dot org 2006-12-06 11:37 --- Subject: Bug 29272 CVSROOT:/cvsroot/classpath Module name:classpath Changes by: Chris Burdess 06/12/06 11:36:42 Modified files: . : ChangeLog gnu/xml/stream : SAXParser.java javax/xml/parsers: DocumentBuilderFactory.java Log message: 2006-12-06 Chris Burdess <[EMAIL PROTECTED]> Fixes PR 29272. * javax/xml/parsers/DocumentBuilderFactory.java: Fix broken Javadoc. * gnu/xml/stream/SAXParser.java: Fix file descriptor leak. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/classpath/ChangeLog?cvsroot=classpath&r1=1.8928&r2=1.8929 http://cvs.savannah.gnu.org/viewcvs/classpath/gnu/xml/stream/SAXParser.java?cvsroot=classpath&r1=1.21&r2=1.22 http://cvs.savannah.gnu.org/viewcvs/classpath/javax/xml/parsers/DocumentBuilderFactory.java?cvsroot=classpath&r1=1.5&r2=1.6 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #14 from rguenth at gcc dot gnu dot org 2006-10-11 22:04 --- This is fixed now. Or was invalid. -- rguenth at gcc dot gnu dot org changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #13 from jakub at gcc dot gnu dot org 2006-10-10 09:47 --- Subject: Bug 29272 Author: jakub Date: Tue Oct 10 09:46:59 2006 New Revision: 117599 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117599 Log: PR middle-end/29272 * builtins.c (var_decl_component_p): New function. (fold_builtin_memset, fold_builtin_memory_op): Restrict single entry optimization to variables and components thereof. * gcc.c-torture/execute/20060930-2.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/execute/20060930-2.c Modified: trunk/gcc/ChangeLog trunk/gcc/builtins.c trunk/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
-- mmitchel at gcc dot gnu dot org changed: What|Removed |Added Priority|P3 |P1 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #12 from jakub at gcc dot gnu dot org 2006-10-01 22:35 --- Wrt #8, can you come up with a C++ testcase where using var = x; is invalid, but using memcpy (&var, &x, sizeof (var)); (where sizeof (var) == sizeof (x)) makes it valid C++ (from aliasing point of view)? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #11 from jakub at gcc dot gnu dot org 2006-09-30 11:49 --- The primary advantage of the single entry optimization is actually that at tree level we can find out if something is only addressable because of the memcpy/memset/mempcpy/memmove and not for other reasons. So, creating a pointer to the object, adding may_alias attribute to the type and dereferencing it doesn't buy us much, because it would still be addressable. So, either we have a way to mark the VAR_DECL (or component thereof) read resp. write with alias set 0, or we need the patch I posted, possibly with additional check for C++/ObjC++ and don't do the optimization at all for C++. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #10 from rguenther at suse dot de 2006-09-30 11:47 --- Subject: Re: [4.2 Regression] memcpy optimization causes wrong-code On Sat, 29 Sep 2006, mrs at apple dot com wrote: > --- Comment #8 from mrs at apple dot com 2006-09-29 23:15 --- > > If it is a VAR_DECL, then I believe the optimization is always safe > > Not in C++. As safe as using memcpy, which does not add any semantics to an access using the VAR_DECL. So I believe this is safe even for C++. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #9 from rguenther at suse dot de 2006-09-30 11:46 ---
Subject: Re: [4.2 Regression] memcpy optimization
causes wrong-code
On Sat, 29 Sep 2006, pinskia at physics dot uc dot edu wrote:
> --- Comment #7 from pinskia at physics dot uc dot edu 2006-09-29 22:13
> ---
> Subject: Re: [4.2 Regression] memcpy optimization causes wrong-code
> > --- Comment #6 from jakub at gcc dot gnu dot org 2006-09-29 22:04
> > ---
> > Is:
> > extern void abort (void);
> >
> > struct S { struct S *s; } s;
> > struct T { struct T *t; } t;
> >
> > static inline void
> > foo (void *s)
> > {
> > struct T *p = s;
> > __builtin_memcpy (&p->t, &t.t, sizeof (t.t));
>
> I think the problem is really is &p->t include an access or not?
> I know if p is NULL, this is undefined as &p->t is now have a NULL pointer
> access but does that include using memcpy?
If you write it as memcpy (p, &t, ...) then it is ok. Now I think
by applying common sense 6.5.3.2/3 can be applied to &p->t, too.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #8 from mrs at apple dot com 2006-09-29 23:15 --- > If it is a VAR_DECL, then I believe the optimization is always safe Not in C++. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #7 from pinskia at physics dot uc dot edu 2006-09-29 22:13
---
Subject: Re: [4.2 Regression] memcpy optimization causes wrong-code
>
>
>
> --- Comment #6 from jakub at gcc dot gnu dot org 2006-09-29 22:04 ---
> Is:
> extern void abort (void);
>
> struct S { struct S *s; } s;
> struct T { struct T *t; } t;
>
> static inline void
> foo (void *s)
> {
> struct T *p = s;
> __builtin_memcpy (&p->t, &t.t, sizeof (t.t));
I think the problem is really is &p->t include an access or not?
I know if p is NULL, this is undefined as &p->t is now have a NULL pointer
access but does that include using memcpy?
-- Pinski
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
Re: [Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
>
>
>
> --- Comment #6 from jakub at gcc dot gnu dot org 2006-09-29 22:04 ---
> Is:
> extern void abort (void);
>
> struct S { struct S *s; } s;
> struct T { struct T *t; } t;
>
> static inline void
> foo (void *s)
> {
> struct T *p = s;
> __builtin_memcpy (&p->t, &t.t, sizeof (t.t));
I think the problem is really is &p->t include an access or not?
I know if p is NULL, this is undefined as &p->t is now have a NULL pointer
access but does that include using memcpy?
-- Pinski
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #6 from jakub at gcc dot gnu dot org 2006-09-29 22:04 ---
Is:
extern void abort (void);
struct S { struct S *s; } s;
struct T { struct T *t; } t;
static inline void
foo (void *s)
{
struct T *p = s;
__builtin_memcpy (&p->t, &t.t, sizeof (t.t));
}
void *
__attribute__((noinline))
bar (void *p, struct S *q)
{
q->s = &s;
foo (p);
return q->s;
}
int
main (void)
{
t.t = &t;
if (bar (&s, &s) != (void *) &t)
abort ();
return 0;
}
valid? I'd say yes, we either access s.s using its declared (== effective)
type,
or through memcpy (which is supposed tobe a char array access and therefore
ok wrt. aliasing).
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #5 from pinskia at gcc dot gnu dot org 2006-09-29 17:30 --- I think we can declare this as invalid code for C and unkown for C++. For C++, inplacement new is supposed to "fix" the problem with different types as C++ defines aliasing based on the dynamic type but we get a different bug with that being PR 29286. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #4 from rguenth at gcc dot gnu dot org 2006-09-29 09:14 ---
Here's an executable testcase which is miscompiled at the tree-level.
extern void abort(void);
struct Foo {
int a;
int b;
};
struct Node;
struct Node {
struct Node *next;
};
struct Node *pool;
void grow (void)
{
char *mem = __builtin_malloc (4096);
struct Node *node = (struct Node *)mem;
struct Foo *foo;
__builtin_memcpy (&node->next, &pool, sizeof(struct Node*));
pool = node;
}
static inline void *alloc_one (void)
{
struct Node *node = pool;
__builtin_memcpy (&pool, &node->next, sizeof(struct Node*));
return node;
}
static inline void dealloc_one (void *p)
{
struct Node *node = p;
__builtin_memcpy (&node->next, &pool, sizeof(struct Node*));
pool = node;
}
int main()
{
grow();
struct Foo* foo = alloc_one();
foo->a = 0;
foo->b = -1;
dealloc_one (foo);
if (foo->b == -1)
abort ();
return 0;
}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
-- pinskia at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |NEW Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2006-09-29 04:40:41 date|| Target Milestone|--- |4.2.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #3 from dnovillo at gcc dot gnu dot org 2006-09-28 18:32 --- Excerpts from IRC session with jakub discussing this: 13:28 1grow (); 13:28 2node = pool; 13:28 3D.1928 = node->next; 13:28 4pool = D.1928; 13:28 5foo = (struct Foo *) node; 13:28 6foo->a = 0; 13:28 7foo->b = -1; 13:28 8node = (struct Node *) foo; <- redundant. node == foo already. 13:28 9node->next = D.1928; <- redundant. node->next == D.1928 already 13:28 10pool = node; <- redundant. node == pool already. 13:28 11return foo->a; 13:33 line 9 is not redundant, because node->next occupies the same memory as foo->a and foo->b 13:34 I think we have 2 options with this optimization 13:35 1) for each memcpy etc. operand, look through all handled components and if it is an actual VAR_DECL, we can surely optimize it, with the native alias set 13:36 but if it is a pointer, we can't be sure 13:37 now, either we figure out some way how to express that operation in an alias friendly way if it is a pointer, or we just bail and don't optimize 13:37 but, my point was that i don't see where the RTL optimizers may be screwing up. what's the exact operation that they remove that they shouldn't have? 13:37 it all looks removable to me. 13:39 RTL optimizers remove the node->next = D.1928 line 13:40 which means 1) pool->next is in the end 0 rather than old pool->next 13:40 and 2) 0 is returned rather than (int) pool->next 13:40 but D.1928 and node->next have the same value according to the tree dump. or am i misreading something? 14:03 foo == node, so foo->a and node->next occupy the same memory 14:03 oh, crap. 14:03 and eventhough those 2 have aliasing incompatible types, the use of memcpy makes it ok 14:03 i had missed that. 14:04 guess I'll now write just a quick patch to only do it for VAR_DECLs and components thereof 14:05 so that the bug is fixed and we can then keep discussing how even pointers can be safely optimized 14:05 so, going back to not apply this on pointers then? 14:05 yeah, for now that would be the safe approach. 14:10/* If var is a VAR_DECL or a component thereof, 14:10 we can use its alias set, otherwise we'd need to make 14:10 sure we go through alias set 0. */ 14:10inner = srcvar; 14:10while (handled_component_p (inner)) 14:10 inner = TREE_OPERAND (inner, 0); 14:10if (TREE_CODE (inner) != VAR_DECL) 14:10 return 0; 14:11 or should I use SSA_VAR_P (inner) instead? 14:12 I guess at least PARM_DECL or RESULT_DECL would be ok, not sure about SSA names 14:22 the reason we don't screw up in the trees is the presence of points-to information, most likely. i got tricked because i wasn't following who was pointing to who. 14:22 the store foo->a changes node->next. 14:23 bah. 14:23 RTL misses that because the stores have different alias sets. 14:23 the transformation we do on trees is invalid for RTL because we don't keep points-to info in RTL, only type info. 14:23 the Real Fix would tag the stores so that gimple->RTL changes the alias sets. 14:24 something along the lines of what you suggest may work. 14:26 yeah, now to find a way to force the alias set... -- dnovillo at gcc dot gnu dot org changed: What|Removed |Added Target Milestone|4.2.0 |--- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #2 from jakub at gcc dot gnu dot org 2006-09-28 18:17 --- Created an attachment (id=12352) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12352&action=view) gcc42-pr29272.patch Untested patch to just bail for non-vars. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
--- Comment #1 from jakub at gcc dot gnu dot org 2006-09-28 17:57 --- I think after we want to look through handled_component_p's of destvar/srcvar (in fold_builtin_memory_op) resp. var (in fold_builtin_memset) and see if inside is a VAR_DECL or not. If it is a VAR_DECL, then I believe the optimization is always safe, if it is INDIRECT_REF of a pointer, then it would be only safe if we can make the read resp. write use forcibly alias set 0. If we e.g. made a copy of the pointer and set DECL_POINTER_ALIAS_SET on it, would that work and not be optimized out? -- jakub at gcc dot gnu dot org changed: What|Removed |Added CC||rth at gcc dot gnu dot org, ||dnovillo at redhat dot com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
[Bug middle-end/29272] [4.2 Regression] memcpy optimization causes wrong-code
-- pinskia at gcc dot gnu dot org changed: What|Removed |Added CC||pinskia at gcc dot gnu dot ||org Target Milestone|--- |4.2.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29272
