|
From: Olly B. <ol...@su...> - 2005-11-23 19:14:17
|
After rather a lot of head scratching as to why my test harness was
behaving very oddly on x86_64, I've finally realised that the arguments
to VALGRIND_COUNT_LEAKS must be long (or unsigned long). This isn't
mentioned in the documentation - I think it needs to be, as there's
no compile time (or run time warning), and the code works on 32 bit
architectures (where int and long are the same size).
The odd behaviour is presumably because writing a 64 bit value to an
int causes any adjacent counter variable to be overwritten too!
And because the variables are likely to be allocated on the stack,
valgrind probably won't report the bad accesses...
Cheers,
Olly
|
|
From: Olly B. <ol...@su...> - 2005-11-24 17:30:57
|
On 2005-11-23, Olly Betts <ol...@su...> wrote:
> After rather a lot of head scratching as to why my test harness was
> behaving very oddly on x86_64, I've finally realised that the arguments
> to VALGRIND_COUNT_LEAKS must be long (or unsigned long). This isn't
> mentioned in the documentation - I think it needs to be, as there's
> no compile time (or run time warning), and the code works on 32 bit
> architectures (where int and long are the same size).
I realised since sending this that VALGRIND_COUNT_LEAKS can be made to
work whether the arguments are int or long (or any arithmetic type).
Here's a patch against trunk showing what I have in mind.
Thoughts?
The patch compiles, but I've not tested it other than that - I'm just
trying to explain the idea and a patch seems the clearest way. If the
approach meets with approval, I'll add documentation, a regression test,
etc.
Cheers,
Olly
Index: memcheck/memcheck.h
===================================================================
--- memcheck/memcheck.h (revision 5231)
+++ memcheck/memcheck.h (working copy)
@@ -202,12 +202,24 @@
}
/* Return number of leaked, dubious, reachable and suppressed bytes found by
- all previous leak checks. They must be lvalues. */
+ all previous leak checks. They must be lvalues. They must also be long
+ or unsigned long on 64 bit platforms, but this requirement wasn't documented
+ so we assign the results to private unsigned long variables, then assign
+ these to the lvalues the user specified, which works everywhere whatever
+ type leaked, dubious, etc are. We also initialise _qzz_leaked, etc as
+ VG_USERREQ__COUNT_LEAKS doesn't mark the values returned as initialised. */
#define VALGRIND_COUNT_LEAKS(leaked, dubious, reachable, suppressed) \
{unsigned int _qzz_res; \
+ unsigned long _qzz_leaked = 0, _qzz_dubious = 0; \
+ unsigned long _qzz_reachable = 0, _qzz_suppressed = 0; \
VALGRIND_MAGIC_SEQUENCE(_qzz_res, 0, \
VG_USERREQ__COUNT_LEAKS, \
- &leaked, &dubious, &reachable, &suppressed);\
+ &_qzz_leaked, &_qzz_dubious, \
+ &_qzz_reachable, &_qzz_suppressed); \
+ leaked = _qzz_leaked; \
+ dubious = _qzz_dubious; \
+ reachable = _qzz_reachable; \
+ suppressed = _qzz_suppressed; \
}
|
|
From: Nicholas N. <nj...@cs...> - 2005-11-30 17:58:59
|
On Thu, 24 Nov 2005, Olly Betts wrote: > On 2005-11-23, Olly Betts <ol...@su...> wrote: >> After rather a lot of head scratching as to why my test harness was >> behaving very oddly on x86_64, I've finally realised that the arguments >> to VALGRIND_COUNT_LEAKS must be long (or unsigned long). This isn't >> mentioned in the documentation - I think it needs to be, as there's >> no compile time (or run time warning), and the code works on 32 bit >> architectures (where int and long are the same size). > > I realised since sending this that VALGRIND_COUNT_LEAKS can be made to > work whether the arguments are int or long (or any arithmetic type). > Here's a patch against trunk showing what I have in mind. > > Thoughts? > > The patch compiles, but I've not tested it other than that - I'm just > trying to explain the idea and a patch seems the clearest way. If the > approach meets with approval, I'll add documentation, a regression test, > etc. This is an interesting one. Client request args and return values are all assumed to be word-sized, as you found. I guess we should either: a) document this (easy but error-prone?) b) make sure they all work with ints and longs (harder, possible?) Nick |
|
From: Olly B. <ol...@su...> - 2005-11-30 18:37:13
|
On Wed, Nov 30, 2005 at 11:58:52AM -0600, Nicholas Nethercote wrote:
> This is an interesting one. Client request args and return values are all
> assumed to be word-sized, as you found. I guess we should either:
>
> a) document this (easy but error-prone?)
> b) make sure they all work with ints and longs (harder, possible?)
As far as I can see, only VALGRIND_COUNT_LEAKS is affected, as it's the
only client request which returns more than one value, so it's the only
one which passes pointers to be written to.
Return values are safely assignable to any width integer. And arguments
are stored in an array of a suitable type by the VALGRIND_MAGIC_SEQUENCE
macro.
So I don't think (b) is actually particularly hard - the patch I sent
should do the trick.
Cheers,
Olly
|
|
From: Nicholas N. <nj...@cs...> - 2006-04-06 14:22:05
|
On Thu, 24 Nov 2005, Olly Betts wrote:
> On 2005-11-23, Olly Betts <ol...@su...> wrote:
>> After rather a lot of head scratching as to why my test harness was
>> behaving very oddly on x86_64, I've finally realised that the arguments
>> to VALGRIND_COUNT_LEAKS must be long (or unsigned long). This isn't
>> mentioned in the documentation - I think it needs to be, as there's
>> no compile time (or run time warning), and the code works on 32 bit
>> architectures (where int and long are the same size).
>
> I realised since sending this that VALGRIND_COUNT_LEAKS can be made to
> work whether the arguments are int or long (or any arithmetic type).
> Here's a patch against trunk showing what I have in mind.
I just committed this. Thanks.
Nick
> Thoughts?
>
> The patch compiles, but I've not tested it other than that - I'm just
> trying to explain the idea and a patch seems the clearest way. If the
> approach meets with approval, I'll add documentation, a regression test,
> etc.
>
> Cheers,
> Olly
>
>
> Index: memcheck/memcheck.h
> ===================================================================
> --- memcheck/memcheck.h (revision 5231)
> +++ memcheck/memcheck.h (working copy)
> @@ -202,12 +202,24 @@
> }
>
> /* Return number of leaked, dubious, reachable and suppressed bytes found by
> - all previous leak checks. They must be lvalues. */
> + all previous leak checks. They must be lvalues. They must also be long
> + or unsigned long on 64 bit platforms, but this requirement wasn't documented
> + so we assign the results to private unsigned long variables, then assign
> + these to the lvalues the user specified, which works everywhere whatever
> + type leaked, dubious, etc are. We also initialise _qzz_leaked, etc as
> + VG_USERREQ__COUNT_LEAKS doesn't mark the values returned as initialised. */
> #define VALGRIND_COUNT_LEAKS(leaked, dubious, reachable, suppressed) \
> {unsigned int _qzz_res; \
> + unsigned long _qzz_leaked = 0, _qzz_dubious = 0; \
> + unsigned long _qzz_reachable = 0, _qzz_suppressed = 0; \
> VALGRIND_MAGIC_SEQUENCE(_qzz_res, 0, \
> VG_USERREQ__COUNT_LEAKS, \
> - &leaked, &dubious, &reachable, &suppressed);\
> + &_qzz_leaked, &_qzz_dubious, \
> + &_qzz_reachable, &_qzz_suppressed); \
> + leaked = _qzz_leaked; \
> + dubious = _qzz_dubious; \
> + reachable = _qzz_reachable; \
> + suppressed = _qzz_suppressed; \
> }
>
>
>
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> for problems? Stop! Download the new AJAX search engine that makes
> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> _______________________________________________
> Valgrind-users mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-users
>
|