|
From: Carl E. L. <ce...@us...> - 2015-07-22 15:34:14
|
Julian:
The Solaris support was recently added in Valgrind commit 15426. It
included the following change:
Index: memcheck/tests/leak_cpp_interior.cpp
===================================================================
--- memcheck/tests/leak_cpp_interior.cpp (revision 15425)
+++ memcheck/tests/leak_cpp_interior.cpp (revision 15426)
@@ -1,3 +1,4 @@
+#include <inttypes.h>
#include <stdio.h>
#include <unistd.h>
#include <stdint.h>
@@ -107,7 +108,8 @@
// prepare the who_points_at cmd we will run.
// Do it here to avoid having ptr or its exterior ptr kept in a register.
- sprintf(who_points_at_cmd, "who_points_at %p 20", (char*)ptr - sizeof(void*));
+ sprintf(who_points_at_cmd, "who_points_at %#" PRIxPTR " 20",
+ (uintptr_t) (char*)ptr - sizeof(void*));
ptr2 = new MyClass[0]; // "interior but exterior ptr".
// ptr2 points after the chunk, is wrongly considered by memcheck as definitely leaked.
The change doesn't seem to compile on a PPC64 Power 7 Big endian
machine. I haven't check the other PPC64 platforms yet. I saw that the
nightly regression test had failed. I did a fresh pull of the code and
tried compiling and verified that I see the compile failure. I then
reverted the change locally and was able to get the compile to
successfully complete. The regression test also runs but as expected we
get an additional failure on that particular test.
I am not much of a C++ programmer so I am not sure what the correct fix
should be for this issue. If you can take a look at it I would
appreciate it. Let me know if I can help out testing a fix. Thanks.
Carl Love
|
|
From: Ivo R. <ivo...@gm...> - 2015-07-22 16:10:42
|
2015-07-22 17:34 GMT+02:00 Carl E. Love <ce...@us...>: > Julian: > > The Solaris support was recently added in Valgrind commit 15426. It > included the following change: > > Index: memcheck/tests/leak_cpp_interior.cpp > =================================================================== > --- memcheck/tests/leak_cpp_interior.cpp (revision 15425) > +++ memcheck/tests/leak_cpp_interior.cpp (revision 15426) > @@ -1,3 +1,4 @@ > +#include <inttypes.h> > #include <stdio.h> > #include <unistd.h> > #include <stdint.h> > @@ -107,7 +108,8 @@ > > // prepare the who_points_at cmd we will run. > // Do it here to avoid having ptr or its exterior ptr kept in a > register. > - sprintf(who_points_at_cmd, "who_points_at %p 20", (char*)ptr - > sizeof(void*)); > + sprintf(who_points_at_cmd, "who_points_at %#" PRIxPTR " 20", > + (uintptr_t) (char*)ptr - sizeof(void*)); > > ptr2 = new MyClass[0]; // "interior but exterior ptr". > // ptr2 points after the chunk, is wrongly considered by memcheck as > definitely leaked. > > > The change doesn't seem to compile on a PPC64 Power 7 Big endian > machine. I haven't check the other PPC64 platforms yet. I saw that the > nightly regression test had failed. I did a fresh pull of the code and > tried compiling and verified that I see the compile failure. I then > reverted the change locally and was able to get the compile to > successfully complete. The regression test also runs but as expected we > get an additional failure on that particular test. > > I am not much of a C++ programmer so I am not sure what the correct fix > should be for this issue. If you can take a look at it I would > appreciate it. Let me know if I can help out testing a fix. Thanks. > It seems that PRIxPTR is not recognized by the g++ compiler. Please try this fix: -#include <inttypes.h> ... + sprintf(who_points_at_cmd, "who_points_at %#llx 20", + (unsigned long long) (char*)ptr - sizeof(void*)); The same problem exists on arm-linux, I hope the same fix will suffice. Kind regards, I. |
|
From: Carl E. L. <ce...@us...> - 2015-07-22 16:12:06
|
On Wed, 2015-07-22 at 16:53 +0100, Tom Hughes wrote:
> On 22/07/15 16:34, Carl E. Love wrote:
>
> > The Solaris support was recently added in Valgrind commit 15426. It
> > included the following change:
> >
> > Index: memcheck/tests/leak_cpp_interior.cpp
> > ===================================================================
> > --- memcheck/tests/leak_cpp_interior.cpp (revision 15425)
> > +++ memcheck/tests/leak_cpp_interior.cpp (revision 15426)
> > @@ -1,3 +1,4 @@
> > +#include <inttypes.h>
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <stdint.h>
> > @@ -107,7 +108,8 @@
> >
> > // prepare the who_points_at cmd we will run.
> > // Do it here to avoid having ptr or its exterior ptr kept in a register.
> > - sprintf(who_points_at_cmd, "who_points_at %p 20", (char*)ptr - sizeof(void*));
> > + sprintf(who_points_at_cmd, "who_points_at %#" PRIxPTR " 20",
> > + (uintptr_t) (char*)ptr - sizeof(void*));
> >
> > ptr2 = new MyClass[0]; // "interior but exterior ptr".
> > // ptr2 points after the chunk, is wrongly considered by memcheck as definitely leaked.
> >
> >
> > The change doesn't seem to compile on a PPC64 Power 7 Big endian
> > machine. I haven't check the other PPC64 platforms yet. I saw that the
> > nightly regression test had failed. I did a fresh pull of the code and
> > tried compiling and verified that I see the compile failure. I then
> > reverted the change locally and was able to get the compile to
> > successfully complete. The regression test also runs but as expected we
> > get an additional failure on that particular test.
> >
> > I am not much of a C++ programmer so I am not sure what the correct fix
> > should be for this issue. If you can take a look at it I would
> > appreciate it. Let me know if I can help out testing a fix. Thanks.
>
> Well what is the error it is reporting?
>
Sorry, I thought I had pasted it in.
I../../include -I../../VEX/pub -I../../VEX/pub -DVGA_ppc64be=1 -DVGO_linux=1 -D\
VGP_ppc64be_linux=1 -DVGPV_ppc64be_linux_vanilla=1 -DVGA_SEC_ppc32=1 -DVGP_SEC_\
ppc64be_linux=1 -Winline -Wall -Wshadow -Wno-long-long -g -fno-stack-protector\
-m64 -MT leak_cpp_interior.o -MD -MP -MF $depbase.Tpo -c -o leak_cpp_interio\
r.o leak_cpp_interior.cpp &&\
mv -f $depbase.Tpo $depbase.Po
leak_cpp_interior.cpp: In function ‘void doit()’:
leak_cpp_interior.cpp:111: error: expected ‘)’ before ‘PRIxPTR’
leak_cpp_interior.cpp:112: warning: conversion lacks type at end of format
leak_cpp_interior.cpp:112: warning: too many arguments for format
make[5]: *** [leak_cpp_interior.o] Error 1
Sorry for being a space cadet and forgetting that little detail. Argh!
Carl Love
|
|
From: Tom H. <to...@co...> - 2015-07-22 16:18:50
|
On 22/07/15 17:11, Carl E. Love wrote: > I../../include -I../../VEX/pub -I../../VEX/pub -DVGA_ppc64be=1 -DVGO_linux=1 -D\ > VGP_ppc64be_linux=1 -DVGPV_ppc64be_linux_vanilla=1 -DVGA_SEC_ppc32=1 -DVGP_SEC_\ > ppc64be_linux=1 -Winline -Wall -Wshadow -Wno-long-long -g -fno-stack-protector\ > -m64 -MT leak_cpp_interior.o -MD -MP -MF $depbase.Tpo -c -o leak_cpp_interio\ > r.o leak_cpp_interior.cpp &&\ > mv -f $depbase.Tpo $depbase.Po > leak_cpp_interior.cpp: In function ‘void doit()’: > leak_cpp_interior.cpp:111: error: expected ‘)’ before ‘PRIxPTR’ > leak_cpp_interior.cpp:112: warning: conversion lacks type at end of format > leak_cpp_interior.cpp:112: warning: too many arguments for format > make[5]: *** [leak_cpp_interior.o] Error 1 That seems odd... Can you do it with -E and see what the preprocessor is expanded PRIxPTR to? Actually I guess given than message it isn't - maybe there is an include missing? It has inttypes.h though, which ought to be enough. Can you try this: gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR Hopefully it will print something like: #define PRIxPTR __PRIPTR_PREFIX "x" Tom -- Tom Hughes (to...@co...) http://compton.nu/ |
|
From: Florian K. <fl...@ei...> - 2015-07-22 16:21:55
|
C99 says on page 197, footnote 2, that C++ programs must explicitly define __STDC__FORMAT_MACROS to get the format macros. Ditto for the limit macros. */ #ifdef __cplusplus # ifndef __STDC_FORMAT_MACROS # define __STDC_FORMAT_MACROS # endif # ifndef __STDC_LIMIT_MACROS # define __STDC_LIMIT_MACROS # endif #endif #include <inttypes.h> Florian On 22.07.2015 17:34, Carl E. Love wrote: > Julian: > > The Solaris support was recently added in Valgrind commit 15426. It > included the following change: > > Index: memcheck/tests/leak_cpp_interior.cpp > =================================================================== > --- memcheck/tests/leak_cpp_interior.cpp (revision 15425) > +++ memcheck/tests/leak_cpp_interior.cpp (revision 15426) > @@ -1,3 +1,4 @@ > +#include <inttypes.h> > #include <stdio.h> > #include <unistd.h> > #include <stdint.h> > @@ -107,7 +108,8 @@ > > // prepare the who_points_at cmd we will run. > // Do it here to avoid having ptr or its exterior ptr kept in a register. > - sprintf(who_points_at_cmd, "who_points_at %p 20", (char*)ptr - sizeof(void*)); > + sprintf(who_points_at_cmd, "who_points_at %#" PRIxPTR " 20", > + (uintptr_t) (char*)ptr - sizeof(void*)); > > ptr2 = new MyClass[0]; // "interior but exterior ptr". > // ptr2 points after the chunk, is wrongly considered by memcheck as definitely leaked. > > > The change doesn't seem to compile on a PPC64 Power 7 Big endian > machine. I haven't check the other PPC64 platforms yet. I saw that the > nightly regression test had failed. I did a fresh pull of the code and > tried compiling and verified that I see the compile failure. I then > reverted the change locally and was able to get the compile to > successfully complete. The regression test also runs but as expected we > get an additional failure on that particular test. > > I am not much of a C++ programmer so I am not sure what the correct fix > should be for this issue. If you can take a look at it I would > appreciate it. Let me know if I can help out testing a fix. Thanks. > > Carl Love > > > ------------------------------------------------------------------------------ > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |
|
From: Patrick J. L. <lop...@gm...> - 2015-07-22 16:37:17
|
On Wed, Jul 22, 2015 at 9:21 AM, Florian Krohm <fl...@ei...> wrote: > C99 says on page 197, footnote 2, that C++ programs must explicitly > define __STDC__FORMAT_MACROS to get the format macros. Ditto for > the limit macros. C99 also says the intptr_t and uintptr_t typedefs are optional (section 7.18.1.4). Modern POSIX (http://pubs.opengroup.org/stage7tc1/basedefs/inttypes.h.html and http://pubs.opengroup.org/stage7tc1/basedefs/stdint.h.html) requires uintptr_t for XSI conformance, and it does not say anything about defining any macros to get the definitions. Although it does not contemplate C++ at all... C++11 has uintptr_t as optional and technically puts these macros in <cinttypes>. - Pat |
|
From: Tom H. <to...@co...> - 2015-07-22 16:25:50
|
On 22/07/15 16:34, Carl E. Love wrote: > The Solaris support was recently added in Valgrind commit 15426. It > included the following change: > > Index: memcheck/tests/leak_cpp_interior.cpp > =================================================================== > --- memcheck/tests/leak_cpp_interior.cpp (revision 15425) > +++ memcheck/tests/leak_cpp_interior.cpp (revision 15426) > @@ -1,3 +1,4 @@ > +#include <inttypes.h> > #include <stdio.h> > #include <unistd.h> > #include <stdint.h> > @@ -107,7 +108,8 @@ > > // prepare the who_points_at cmd we will run. > // Do it here to avoid having ptr or its exterior ptr kept in a register. > - sprintf(who_points_at_cmd, "who_points_at %p 20", (char*)ptr - sizeof(void*)); > + sprintf(who_points_at_cmd, "who_points_at %#" PRIxPTR " 20", > + (uintptr_t) (char*)ptr - sizeof(void*)); > > ptr2 = new MyClass[0]; // "interior but exterior ptr". > // ptr2 points after the chunk, is wrongly considered by memcheck as definitely leaked. > > > The change doesn't seem to compile on a PPC64 Power 7 Big endian > machine. I haven't check the other PPC64 platforms yet. I saw that the > nightly regression test had failed. I did a fresh pull of the code and > tried compiling and verified that I see the compile failure. I then > reverted the change locally and was able to get the compile to > successfully complete. The regression test also runs but as expected we > get an additional failure on that particular test. > > I am not much of a C++ programmer so I am not sure what the correct fix > should be for this issue. If you can take a look at it I would > appreciate it. Let me know if I can help out testing a fix. Thanks. Well what is the error it is reporting? Tom -- Tom Hughes (to...@co...) http://compton.nu/ |
|
From: Carl E. L. <ce...@us...> - 2015-07-22 16:41:23
|
Tom, Ivo:
I tested the change suggested by Ivo and the compile test from Tom.
On Wed, 2015-07-22 at 17:18 +0100, Tom Hughes wrote:
> gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR
On Power 7, big endian:
gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR
#define PRIxPTR __PRIPTR_PREFIX "x"
On Power 8, little endian:
gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR
#define PRIxPTR __PRIPTR_PREFIX "x"
On Power 8, big endian
gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR
#define PRIxPTR __PRIPTR_PREFIX "x"
So that seems to work on all of the platforms.
I have also tested the fix suggested by Ivo Raisr
+ sprintf(who_points_at_cmd, "who_points_at %#llx 20",
+ (unsigned long long) (char*)ptr - sizeof(void*));
The change also compiles successfully. The regression tests run but I
get an additional error on the leak_cpp_interior test.
== 614 tests, 2 stderr failures, 0 stdout failures, 0 stderrB failures, 0 stdoutB failures, 0 post failures ==
memcheck/tests/leak_cpp_interior (stderr)
helgrind/tests/tc20_verifywrap (stderr)
So, it looks like either fix will take care of the compiling issue. We still have an issue with the testcase.
The test case output is as follows:
--- leak_cpp_interior.stderr.exp-64bit 2015-07-22 09:38:34.383232305 -0500
+++ leak_cpp_interior.stderr.out 2015-07-22 11:17:44.028761872 -0500
@@ -2,8 +2,8 @@
valgrind output will go to log
VALGRIND_DO_LEAK_CHECK
8 bytes in 1 blocks are definitely lost in loss record ... of ...
- by 0x........: doit() (leak_cpp_interior.cpp:114)
- by 0x........: main (leak_cpp_interior.cpp:129)
+ by 0x........: doit() (leak_cpp_interior.cpp:123)
+ by 0x........: main (leak_cpp_interior.cpp:138)
Looks like the error is just due to the fact that the code has changed causing the line number to change. That
should be an easy fix.
Carl Love
|
|
From: Tom H. <to...@co...> - 2015-07-22 16:43:18
|
On 22/07/15 17:41, Carl E. Love wrote: > I tested the change suggested by Ivo and the compile test from Tom. > > On Wed, 2015-07-22 at 17:18 +0100, Tom Hughes wrote: >> gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR > > On Power 7, big endian: > > gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR > #define PRIxPTR __PRIPTR_PREFIX "x" > > > On Power 8, little endian: > > gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR > #define PRIxPTR __PRIPTR_PREFIX "x" > > On Power 8, big endian > > gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR > #define PRIxPTR __PRIPTR_PREFIX "x" > > > So that seems to work on all of the platforms. From what Florian says you may get a different answer using g++ though, which is the issue here? Tom -- Tom Hughes (to...@co...) http://compton.nu/ |
|
From: Carl E. L. <ce...@us...> - 2015-07-22 16:53:41
|
On Wed, 2015-07-22 at 17:43 +0100, Tom Hughes wrote:
> > On Power 8, big endian
> >
> > gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR
> > #define PRIxPTR __PRIPTR_PREFIX "x"
> >
> >
> > So that seems to work on all of the platforms.
>
> From what Florian says you may get a different answer using g++ though,
> which is the issue here?
Hmm, OK, I tried changing gcc to g++
g++ -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR
I do get the same results. But there does seem to be some question on
what macros are supplied? Not sure about the answer to that question.
Carl Love
|
|
From: Philippe W. <phi...@sk...> - 2015-07-22 21:42:57
|
On Wed, 2015-07-22 at 09:53 -0700, Carl E. Love wrote: > On Wed, 2015-07-22 at 17:43 +0100, Tom Hughes wrote: > > > > On Power 8, big endian > > > > > > gcc -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR > > > #define PRIxPTR __PRIPTR_PREFIX "x" > > > > > > > > > So that seems to work on all of the platforms. > > > > From what Florian says you may get a different answer using g++ though, > > which is the issue here? > > Hmm, OK, I tried changing gcc to g++ > > g++ -include inttypes.h -E -dM - < /dev/null | fgrep PRIxPTR > > I do get the same results. But there does seem to be some question on > what macros are supplied? Not sure about the answer to that question. Revision 15435 should fix the problem, tested on x86 and ppc64 (gcc110). Philippe |