|
From: Konstantin S. <kon...@gm...> - 2009-05-04 19:04:05
|
Hi Memcheckers! [I didn't find this in FAQ...] When copying structures of size 5, g++ (4.3.1) generates memcpy(dst, src, 5). It may happen that src==dst (see example below). src==dst is not allowed by the standard, but this code is not written by humans and the compiler knows it is correct (I asume that glibc will not explode with src==dst) Memcheck reports a warning. A similar problem was discussed here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32667 But that reproducer does not work any more. Even if this is a bug in gcc (is it?) we still need to workaround it in memcheck. Comments? Thanks, --kcc % cat swap5.cc #include <algorithm> #include <string.h> struct Foo { int e4; char e1; Foo() { e4 = 0; e1 = 0; } }; int main(int argc, char **argv) { Foo x[20]; // swap calls memcpy with dst==src. std::swap(x[1], x[argc]); return x[1].e4; } % g++ -g swap5.cc && ~/valgrind/trunk/Inst/bin/valgrind -q ./a.out ==12052== Source and destination overlap in memcpy(0x7FF0006B8, 0x7FF0006B8, 5) ==12052== at 0x4C1E22A: memcpy (mc_replace_strmem.c:380) ==12052== by 0x4006BC: void std::swap<Foo>(Foo&, Foo&) (stl_move.h:86) ==12052== by 0x40066B: main (swap5.cc:12) % |
|
From: Nicholas N. <n.n...@gm...> - 2009-05-04 22:30:52
|
On Tue, May 5, 2009 at 5:03 AM, Konstantin Serebryany <kon...@gm...> wrote: > > When copying structures of size 5, g++ (4.3.1) generates memcpy(dst, src, 5). > It may happen that src==dst (see example below). > src==dst is not allowed by the standard, but this code is not written > by humans and the compiler knows it is correct > (I asume that glibc will not explode with src==dst) > > Memcheck reports a warning. > > Even if this is a bug in gcc (is it?) we still need to workaround it > in memcheck. > Comments? These ones are always tricky, where the language spec doesn't match what happens in practice. This case is particularly annoying because we can't distinguish between a user-inserted memcpy() and a GCC-generated one. It's arguably that GCC shouldn't generate such code -- you say you assume glibc will not explode in this case, but that is an assumption. So I don't know what to do. Maybe Julian will have some ideas. Nick |
|
From: Konstantin S. <kon...@gm...> - 2009-05-06 07:42:09
|
On Tue, May 5, 2009 at 2:30 AM, Nicholas Nethercote <n.n...@gm...> wrote: > On Tue, May 5, 2009 at 5:03 AM, Konstantin Serebryany > <kon...@gm...> wrote: >> >> When copying structures of size 5, g++ (4.3.1) generates memcpy(dst, src, 5). >> It may happen that src==dst (see example below). >> src==dst is not allowed by the standard, but this code is not written >> by humans and the compiler knows it is correct >> (I asume that glibc will not explode with src==dst) >> >> Memcheck reports a warning. >> >> Even if this is a bug in gcc (is it?) we still need to workaround it >> in memcheck. >> Comments? > > These ones are always tricky, where the language spec doesn't match > what happens in practice. This case is particularly annoying because > we can't distinguish between a user-inserted memcpy() and a > GCC-generated one. It's arguably that GCC shouldn't generate such > code -- you say you assume glibc will not explode in this case, but > that is an assumption. > > So I don't know what to do. Maybe Julian will have some ideas. I would suggest two "workarounds": - add a flag that disables warnings on src=dst (off by default) or - generate a different kind of warning for src=dst. Right now memcheck reports warning of type Memcheck:Overlap for cases when the memory regions partially overlap and when they are equal. We can generate Memcheck:MemcpySrcEqualsDst for the latter case. This looks better but may break someone's suppression files. Which one do you prefer (if any)? --kcc > > Nick > |
|
From: Julian S. <js...@ac...> - 2009-05-06 08:20:21
|
On Tuesday 05 May 2009, Nicholas Nethercote wrote: > On Tue, May 5, 2009 at 5:03 AM, Konstantin Serebryany > > <kon...@gm...> wrote: > > When copying structures of size 5, g++ (4.3.1) generates memcpy(dst, src, > > 5). It may happen that src==dst (see example below). > > src==dst is not allowed by the standard, but this code is not written > > by humans and the compiler knows it is correct > > (I asume that glibc will not explode with src==dst) > > > > Memcheck reports a warning. > > > > Even if this is a bug in gcc (is it?) we still need to workaround it > > in memcheck. > > Comments? > > These ones are always tricky, where the language spec doesn't match > what happens in practice. This case is particularly annoying because > we can't distinguish between a user-inserted memcpy() and a > GCC-generated one. It's arguably that GCC shouldn't generate such > code -- you say you assume glibc will not explode in this case, but > that is an assumption. I think you should file this as a gcc bug. AIUI, POSIX says the src==dst case is not allowed (along with all other overlapping cases) because (eg) on PowerPC, it is possible to make a high performance memcpy that preallocates the destination area in D1 using dcbz instructions, which create the line in D1 and fill it full of zeroes. This avoids dragging the destination line up the memory hierarchy only to completely overwrite it with stuff from the source. Result is however that if the src and dst overlap, in any way, including completely, then this causes zeroes to be written into the src area (!) which is certainly not what you want. J |
|
From: Konstantin S. <kon...@gm...> - 2009-05-06 08:23:34
|
On Wed, May 6, 2009 at 12:24 PM, Julian Seward <js...@ac...> wrote: > On Tuesday 05 May 2009, Nicholas Nethercote wrote: >> On Tue, May 5, 2009 at 5:03 AM, Konstantin Serebryany >> >> <kon...@gm...> wrote: >> > When copying structures of size 5, g++ (4.3.1) generates memcpy(dst, src, >> > 5). It may happen that src==dst (see example below). >> > src==dst is not allowed by the standard, but this code is not written >> > by humans and the compiler knows it is correct >> > (I asume that glibc will not explode with src==dst) >> > >> > Memcheck reports a warning. >> > >> > Even if this is a bug in gcc (is it?) we still need to workaround it >> > in memcheck. >> > Comments? >> >> These ones are always tricky, where the language spec doesn't match >> what happens in practice. This case is particularly annoying because >> we can't distinguish between a user-inserted memcpy() and a >> GCC-generated one. It's arguably that GCC shouldn't generate such >> code -- you say you assume glibc will not explode in this case, but >> that is an assumption. > > I think you should file this as a gcc bug. Yea, probably. But we still need a workaround in Memcheck. The existing versions of gcc will be there for years... --kcc > > AIUI, POSIX says the src==dst case is not allowed (along with all other > overlapping cases) because (eg) on PowerPC, it is possible to make a high > performance memcpy that preallocates the destination area in D1 using > dcbz instructions, which create the line in D1 and fill it full of > zeroes. This avoids dragging the destination line up the memory > hierarchy only to completely overwrite it with stuff from the source. > > Result is however that if the src and dst overlap, in any way, including > completely, then this causes zeroes to be written into the src area (!) > which is certainly not what you want. > > J > |
|
From: Konstantin S. <kon...@gm...> - 2009-05-06 08:32:12
|
On Wed, May 6, 2009 at 12:23 PM, Konstantin Serebryany <kon...@gm...> wrote: > On Wed, May 6, 2009 at 12:24 PM, Julian Seward <js...@ac...> wrote: >> On Tuesday 05 May 2009, Nicholas Nethercote wrote: >>> On Tue, May 5, 2009 at 5:03 AM, Konstantin Serebryany >>> >>> <kon...@gm...> wrote: >>> > When copying structures of size 5, g++ (4.3.1) generates memcpy(dst, src, >>> > 5). It may happen that src==dst (see example below). >>> > src==dst is not allowed by the standard, but this code is not written >>> > by humans and the compiler knows it is correct >>> > (I asume that glibc will not explode with src==dst) >>> > >>> > Memcheck reports a warning. >>> > >>> > Even if this is a bug in gcc (is it?) we still need to workaround it >>> > in memcheck. >>> > Comments? >>> >>> These ones are always tricky, where the language spec doesn't match >>> what happens in practice. This case is particularly annoying because >>> we can't distinguish between a user-inserted memcpy() and a >>> GCC-generated one. It's arguably that GCC shouldn't generate such >>> code -- you say you assume glibc will not explode in this case, but >>> that is an assumption. >> >> I think you should file this as a gcc bug. > Yea, probably. > But we still need a workaround in Memcheck. > The existing versions of gcc will be there for years... Actually, it seems to be fixed in gcc 4.4 But gcc 4.3.1 and earlier will be everywhere for quite some time... --kcc > > --kcc > >> >> AIUI, POSIX says the src==dst case is not allowed (along with all other >> overlapping cases) because (eg) on PowerPC, it is possible to make a high >> performance memcpy that preallocates the destination area in D1 using >> dcbz instructions, which create the line in D1 and fill it full of >> zeroes. This avoids dragging the destination line up the memory >> hierarchy only to completely overwrite it with stuff from the source. >> >> Result is however that if the src and dst overlap, in any way, including >> completely, then this causes zeroes to be written into the src area (!) >> which is certainly not what you want. >> >> J >> > |
|
From: Nicholas N. <n.n...@gm...> - 2009-05-07 23:02:14
|
On Wed, May 6, 2009 at 6:31 PM, Konstantin Serebryany <kon...@gm...> wrote: >> The existing versions of gcc will be there for years... > > Actually, it seems to be fixed in gcc 4.4 > But gcc 4.3.1 and earlier will be everywhere for quite some time... Maybe we need a --workaround-gcc43-bugs option just like we have --workaround-gcc296-bugs. Sigh. (The latter option doesn't seem necessary any more since Valgrind refuses to build with anything less than gcc 3.0.0.) N |
|
From: Greg P. <gp...@ap...> - 2009-05-07 23:12:23
|
On May 7, 2009, at 4:02 PM, Nicholas Nethercote wrote: > Maybe we need a --workaround-gcc43-bugs option just like we have > --workaround-gcc296-bugs. Sigh. > > (The latter option doesn't seem necessary any more since Valgrind > refuses to build with anything less than gcc 3.0.0.) That option works around bugs compiled into the client code. So the question isn't "does Valgrind build with 2.9.6" but rather "can Valgrind run client code built with 2.9.6". (I don't care about supporting anything that old, but it's still important to ask the right question here.) -- Greg Parker gp...@ap... Runtime Wrangler |
|
From: Nicholas N. <n.n...@gm...> - 2009-05-08 00:16:13
|
On Fri, May 8, 2009 at 9:12 AM, Greg Parker <gp...@ap...> wrote: > On May 7, 2009, at 4:02 PM, Nicholas Nethercote wrote: >> >> Maybe we need a --workaround-gcc43-bugs option just like we have >> --workaround-gcc296-bugs. Sigh. >> >> (The latter option doesn't seem necessary any more since Valgrind >> refuses to build with anything less than gcc 3.0.0.) > > That option works around bugs compiled into the client code. So the question > isn't "does Valgrind build with 2.9.6" but rather "can Valgrind run client > code built with 2.9.6". Oh, yes. My mistake. Thanks. N |