|
From: Solomon, B. <be...@ug...> - 2008-03-17 22:29:52
|
I have been trying valgrind for earnest for the first time and have a couple of questions. We have custom allocators so I used the mempool interface as that matches our internals nicely - but I found the check_mempool_sane calls made things excessively slow so commented them out of alloc and free which works fine for me. These checks seemed overly paranoid for a well tested allocator to me and was wondering why they are there all the time (at least after my integration of the custom allocator is tested and working for us). The second issue is the user requests like VALGRIND_CHECK_MEM_IS_DEFINED are documented to return the address of the first bad byte - yet internally they use a unsigned int type is which does not seem correct on 64 bit machines I would have expected that to be unsigned long or perhaps void *. Thanks Bernie Solomon |
|
From: Julian S. <js...@ac...> - 2008-03-18 01:29:00
|
> We have custom allocators so I used the mempool > interface as that matches our internals nicely - but I found the > check_mempool_sane > calls made things excessively slow so commented them out of alloc and > free which works > fine for me. These checks seemed overly paranoid for a well tested > allocator > to me and was wondering why they are there all the time (at least after > my > integration of the custom allocator is tested and working for us). I don't know. > The second issue is the user requests like VALGRIND_CHECK_MEM_IS_DEFINED > are documented to return the address of the first bad byte - yet > internally > they use a unsigned int type is which does not seem correct on 64 bit > machines > I would have expected that to be unsigned long or perhaps void *. That sounds like a straightforward 32-vs-64-bit bug. Can you fix both of these in a way that seems right to you, and send along a patch? Also, what version of V was this? J |
|
From: Solomon, B. <be...@ug...> - 2008-03-24 23:42:21
Attachments:
diff.txt
|
I am not quite sure where I am supposed to post patches. This is the one removing the excessive checks though of course other options are possible (check every so often, have a command line switch etc.) I was using 3.3 but SVN seems the same. The 64 bit change I'll send separately if this is the right place to send it. Bernie -----Original Message----- From: Julian Seward [mailto:js...@ac...] Sent: Monday, March 17, 2008 6:25 PM To: val...@li... Cc: Solomon, Bernard Subject: Re: [Valgrind-users] Couple of questions > We have custom allocators so I used the mempool > interface as that matches our internals nicely - but I found the > check_mempool_sane > calls made things excessively slow so commented them out of alloc and > free which works > fine for me. These checks seemed overly paranoid for a well tested > allocator > to me and was wondering why they are there all the time (at least after > my > integration of the custom allocator is tested and working for us). I don't know. > The second issue is the user requests like VALGRIND_CHECK_MEM_IS_DEFINED > are documented to return the address of the first bad byte - yet > internally > they use a unsigned int type is which does not seem correct on 64 bit > machines > I would have expected that to be unsigned long or perhaps void *. That sounds like a straightforward 32-vs-64-bit bug. Can you fix both of these in a way that seems right to you, and send along a patch? Also, what version of V was this? J |
|
From: Bart V. A. <bar...@gm...> - 2008-03-25 07:26:18
|
On Tue, Mar 25, 2008 at 12:42 AM, Solomon, Bernard <be...@ug...> wrote: > I am not quite sure where I am supposed to post patches. > This is the one removing the excessive checks though of > course other options are possible (check every so often, > have a command line switch etc.) > > I was using 3.3 but SVN seems the same. > > The 64 bit change I'll send separately if this is the > right place to send it. The check_mempool_sane() calls might be very useful to people who are debugging their added memory allocation client requests. Maybe it's better to modify the check_mempool_sane() function itself and to keep the check_mempool_sane() calls. E.g. a command line option could be added to Valgrind that lets the user specify whether or not to let checking happen when check_mempool_sane() is called. Bart. |
|
From: Solomon, B. <be...@ug...> - 2008-03-25 20:19:28
|
Yes a command line option is a possibility. Another is for mempool alloc to check that the memory used is marked noaccess initially and free to check that it is not noaccess which feels as if it should cover basically the same sort of problems as check_mempool_sane does rather slowly (and indeed it seems to have checks on itself in that it verifies the sort it does sorts!). But I don't know if that would cause issues with other peoples use of mempools. Bernie -----Original Message----- From: Bart Van Assche [mailto:bar...@gm...] Sent: Tuesday, March 25, 2008 12:26 AM To: Solomon, Bernard Cc: val...@li...; Julian Seward Subject: Re: [Valgrind-users] Couple of questions On Tue, Mar 25, 2008 at 12:42 AM, Solomon, Bernard <be...@ug...> wrote: > I am not quite sure where I am supposed to post patches. > This is the one removing the excessive checks though of > course other options are possible (check every so often, > have a command line switch etc.) > > I was using 3.3 but SVN seems the same. > > The 64 bit change I'll send separately if this is the > right place to send it. The check_mempool_sane() calls might be very useful to people who are debugging their added memory allocation client requests. Maybe it's better to modify the check_mempool_sane() function itself and to keep the check_mempool_sane() calls. E.g. a command line option could be added to Valgrind that lets the user specify whether or not to let checking happen when check_mempool_sane() is called. Bart. |
|
From: Solomon, B. <be...@ug...> - 2008-03-25 21:11:35
Attachments:
diff64.txt
|
This is the change I made for 64 bits and VALGRIND_CHECK_MEM_IS_ADDRESSABLE/DEFINED Again I am not sure if this is to standard: - It changes the type of the macro to char * so presumably might cause client code to need change. - I reused the OrigFn type so there wasn't the need to create a new typedef for a pointer sized int type even though this case isn't a function pointer so it is a bit of a cheat. I just wasn't sure whether it was worth adding a new typedef and if so what naming scheme to use. Bernie |