|
From: Behdad E. <be...@cs...> - 2006-05-25 08:54:25
|
Hello Valgrinders,
I am adding client requests to GNOME's GLib library. GLib has a
memory allocator called GSlice that allocates superblocks using
memalign(3) and gives smaller chunks away to user. I'm using the
MALLOCLIKE/FREELIKE hints, to detect memory leaks with the new
allocator, but this seems to hit an assertion in the leakcheck
code because the malloclike blocks apparently overlap with the
superblock that was allocated using memalign. The
memcheck/tests/custom_alloc.c sample doesn't have this problem as
it uses mmap to allocate superblocks, but that is not possible
for us, as we do not necessarily allocate in multiples of
pagesize. This also made me understand that memcheck doesn't
keep track of mmap/munmap regions and so cannot detect
address-space leaks, but that's another issue...
Here is the assertion:
Memcheck: mc_leakcheck.c:707 (vgMemCheck_do_detect_memory_leaks):
Assertion 'lc_shadows[i]->data + lc_shadows[i]->size <=
lc_shadows[i+1]->data' failed.
==8900== at 0xB00152B9: report_and_quit (m_libcassert.c:136)
==8900== by 0x62283523: ???
==8900== by 0x6228354B: ???
==8900== by 0x6228354B: ???
==8900== by 0x62283523: ???
==8900== by 0xB002548F: vgPlain_do_syscall (m_syscall.c:258)
==8900== by 0x6228354B: ???
==8900== by 0x59: ???
==8900== by 0x62283603: ???
==8900== by 0x59: ???
sched status:
running_tid=0
One way I could "fix" it was to make leakcheck simply ignore heap
blocks that contain other blocks in them:
Index: memcheck/mc_leakcheck.c
===================================================================
--- memcheck/mc_leakcheck.c (revision 5905)
+++ memcheck/mc_leakcheck.c (working copy)
@@ -703,6 +703,11 @@
/* Sanity check -- make sure they don't overlap */
for (i = 0; i < lc_n_shadows-1; i++) {
+ /* For custom-allocator uses, it's ok if one block is
+ * completely contained in another. Just zero the super-block */
+ if (lc_shadows[i]->data + lc_shadows[i]->size
+ >= lc_shadows[i+1]->data + lc_shadows[i+1]->size)
+ lc_shadows[i]->size = 0;
tl_assert( lc_shadows[i]->data + lc_shadows[i]->size
<= lc_shadows[i+1]->data );
}
But I understand this is not a proper fix. Simply removing the
sanity check doesn't work either, as the binary search in
find_shadow_for falls loose, as well as all the rest of the
leakcheck logic.
Any comments, hints, or ideas on how to fix this either in glib,
or valgrind? Note that the mempool hints do not help, as they
don't detect leaks, among other reasons.
The patch and relevant discussion can be found in this GNOME bug:
http://bugzilla.gnome.org/show_bug.cgi?id=335126
Thanks,
--behdad
http://behdad.org/
"Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill"
-- Dan Bern, "New American Language"
|
|
From: Tom H. <to...@co...> - 2006-05-25 09:26:47
|
In message <Pine.LNX.4.58.0605190615480.32442@epoch.cs>
Behdad Esfahbod <be...@cs...> wrote:
> I am adding client requests to GNOME's GLib library. GLib has a
> memory allocator called GSlice that allocates superblocks using
> memalign(3) and gives smaller chunks away to user. I'm using the
> MALLOCLIKE/FREELIKE hints, to detect memory leaks with the new
> allocator, but this seems to hit an assertion in the leakcheck
> code because the malloclike blocks apparently overlap with the
> superblock that was allocated using memalign. The
> memcheck/tests/custom_alloc.c sample doesn't have this problem as
> it uses mmap to allocate superblocks, but that is not possible
> for us, as we do not necessarily allocate in multiples of
> pagesize. This also made me understand that memcheck doesn't
> keep track of mmap/munmap regions and so cannot detect
> address-space leaks, but that's another issue...
Should you not being using the mempool stuff rather than the
malloclike/freelike stuff? I've never used it but I believe this
is the sort of thing it was intended for.
Tom
--
Tom Hughes (to...@co...)
http://www.compton.nu/
|
|
From: Ashley P. <as...@qu...> - 2006-05-25 10:00:38
|
On Thu, 2006-05-25 at 04:54 -0400, Behdad Esfahbod wrote: > Any comments, hints, or ideas on how to fix this either in glib, > or valgrind? I don't think this is going to be possible using the MALLOCLICK block macros, the problem is you are trying to embed one allocator inside another so even if you got gslice to work properly then when you called the real free() in your superblock valgrind would have lost the state for the block, valgrind has an assumption that each allocator lives in it's own address space. It is possible to create allocators with malloced memory, I think you call VALGRIND_MAKE_NOACCESS() on the whole region just after you allocate it and before you chop it up in your own allocator, you just can't free the memory afterwards... As Tom says maybe the mempool stuff will help but again I've not used it. I got caught up in the gslice issues a few weeks ago with evolution, it strikes me that if you use G_SLICE=malloc then you'd get 90% of the benefits you want from valgrind for zero effort, the 10% you don't get of course being the mismatched malloc()/free() between glibc/gslice. Given that gslice is the default though this case probably isn't to common as these would be picked up as the code was written (now the teething problems have been thrashed out). What I would recommend however is that you add a redzone to the start of your superblock, having g_slice_alloc() able to return a address that it *might* be possible to free with free() seems like a recipe for diaster, having mutually exclusive pointers returned from gslice and malloc would be a good thing (TM). In fact this would claw back the 10% of functionality you lost before... Ashley, |
|
From: Behdad E. <be...@cs...> - 2006-05-26 03:18:22
|
Thanks for the replies. Please keep me CCed, as I'm not subscribed to the list. My comments below: On Thu, 25 May 2006, Ashley Pittman wrote: > > Any comments, hints, or ideas on how to fix this either in glib, > > or valgrind? > > [...] > > It is possible to create allocators with malloced memory, I think you > call VALGRIND_MAKE_NOACCESS() on the whole region just after you > allocate it and before you chop it up in your own allocator, you just > can't free the memory afterwards... I already do the NOACCESS/READABLE dance and it works pretty good. Donno why you are saying the memory can't be freed. But that doesn't solve my problem: that leaks are not detected. > As Tom says maybe the mempool stuff will help but again I've not used > it. I already mentioned in my original mail that mempools don't help. Mempools are useful when you allocate memory from a pool, but have the option to free the pool without freeing individual block first. Since you can always free pools, valgrind doesn't search/report leaks that are allocated from mempools. So, no help really. > I got caught up in the gslice issues a few weeks ago with evolution, it > strikes me that if you use G_SLICE=malloc then you'd get 90% of the > benefits you want from valgrind for zero effort, True. But the reason I was looking into it was to free users from having to anything special to get the best out of valgrind. Making glib switch to G_SLICE=malloc if VALGRIND_IS_RUNNING is an option, not the best though. > the 10% you don't get > of course being the mismatched malloc()/free() between glibc/gslice. > Given that gslice is the default though this case probably isn't to > common as these would be picked up as the code was written (now the > teething problems have been thrashed out). True. > What I would recommend however is that you add a redzone to the start of > your superblock, having g_slice_alloc() able to return a address that it > *might* be possible to free with free() seems like a recipe for diaster, > having mutually exclusive pointers returned from gslice and malloc would > be a good thing (TM). In fact this would claw back the 10% of > functionality you lost before... I've already done that when writing the valgrind patch. Simply moved the SlabInfo struct to the front and it's much better now. > Ashley, Thanks for your comments. I think I'll give up on MALLOCLIKE, although it's really simple to add some kind of support for it in valgrind... --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |
|
From: Ashley P. <as...@qu...> - 2006-05-26 14:21:52
|
On Thu, 2006-05-25 at 23:18 -0400, Behdad Esfahbod wrote: > > It is possible to create allocators with malloced memory, I think you > > call VALGRIND_MAKE_NOACCESS() on the whole region just after you > > allocate it and before you chop it up in your own allocator, you just > > can't free the memory afterwards... > > I already do the NOACCESS/READABLE dance and it works pretty > good. Donno why you are saying the memory can't be freed. Perhaps I should have been clearer, calling NOACCESS et. al. on the memory will cause valgrind to get confused, clearly you *can* free it but I wouldn't expect valgrind to track it properly after it's freed. > > I got caught up in the gslice issues a few weeks ago with evolution, it > > strikes me that if you use G_SLICE=malloc then you'd get 90% of the > > benefits you want from valgrind for zero effort, > > True. But the reason I was looking into it was to free users > from having to anything special to get the best out of valgrind. > Making glib switch to G_SLICE=malloc if VALGRIND_IS_RUNNING is an > option, not the best though. Well it's easy enough to detect valgrind at runtime so users wouldn't have to do anything special at all. Other than not being able to debug the allocator is there any functionality you'd like to have which this approach wouldn't get you? > Thanks for your comments. I think I'll give up on MALLOCLIKE, > although it's really simple to add some kind of support for it in > valgrind... I think the root of the problem is as you said to start with, this code was never designed for allocators to exist in malloced areas of memory, how difficult would it be to switch to using mmap() for gslice? Ashley, |
|
From: Jeroen N. W. <jn...@xs...> - 2006-05-26 15:09:38
|
An attempt to improve Valgrind's memory pool support can be found at http://www.xs4all.nl/~jnw/valgrind/custom-alloc/custom-alloc.html Highlights: * Memory access errors are reported relative to the memory pool block or the custom allocated block where appropriate. * Leak checks are produced separately for memory pools, custom allocated blocks and malloc'd blocks. Each leak report is preceeded and followed by a line starting with 'LEAKCHECK:' and describing the type of memory being checked. * Memory pools are checked for leaks when they are destroyed, or when they themselves are leaked. Jeroen. |
|
From: Behdad E. <be...@cs...> - 2006-05-28 23:02:18
|
On Fri, 26 May 2006, Ashley Pittman wrote: > > True. But the reason I was looking into it was to free users > > from having to anything special to get the best out of valgrind. > > Making glib switch to G_SLICE=malloc if VALGRIND_IS_RUNNING is an > > option, not the best though. > > Well it's easy enough to detect valgrind at runtime so users wouldn't > have to do anything special at all. Other than not being able to debug > the allocator is there any functionality you'd like to have which this > approach wouldn't get you? I cannot think of anything else, but given how hard-to-track memory errors can be, I won't be surprised if that hides other problems as well. But I may be wrong. > > Thanks for your comments. I think I'll give up on MALLOCLIKE, > > although it's really simple to add some kind of support for it in > > valgrind... > > I think the root of the problem is as you said to start with, this code > was never designed for allocators to exist in malloced areas of memory, > how difficult would it be to switch to using mmap() for gslice? The smallest superblock that GSlice allocates now is 128 bytes. I think Tim Janik who wrote GSlice won't like to change it just to get valgrind working, at the cost of more meory consumption, etc. > Ashley, --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |
|
From: Nicholas N. <nj...@cs...> - 2006-05-26 14:45:00
|
On Thu, 25 May 2006, Behdad Esfahbod wrote:
> I am adding client requests to GNOME's GLib library. GLib has a
> memory allocator called GSlice that allocates superblocks using
> memalign(3) and gives smaller chunks away to user. I'm using the
> MALLOCLIKE/FREELIKE hints, to detect memory leaks with the new
> allocator, but this seems to hit an assertion in the leakcheck
> code because the malloclike blocks apparently overlap with the
> superblock that was allocated using memalign. The
> memcheck/tests/custom_alloc.c sample doesn't have this problem as
> it uses mmap to allocate superblocks, but that is not possible
> for us, as we do not necessarily allocate in multiples of
> pagesize. This also made me understand that memcheck doesn't
> keep track of mmap/munmap regions and so cannot detect
> address-space leaks, but that's another issue...
>
> Here is the assertion:
>
> Memcheck: mc_leakcheck.c:707 (vgMemCheck_do_detect_memory_leaks):
> Assertion 'lc_shadows[i]->data + lc_shadows[i]->size <=
> lc_shadows[i+1]->data' failed.
>
> One way I could "fix" it was to make leakcheck simply ignore heap
> blocks that contain other blocks in them:
>
> Index: memcheck/mc_leakcheck.c
> ===================================================================
> --- memcheck/mc_leakcheck.c (revision 5905)
> +++ memcheck/mc_leakcheck.c (working copy)
> @@ -703,6 +703,11 @@
>
> /* Sanity check -- make sure they don't overlap */
> for (i = 0; i < lc_n_shadows-1; i++) {
> + /* For custom-allocator uses, it's ok if one block is
> + * completely contained in another. Just zero the super-block */
> + if (lc_shadows[i]->data + lc_shadows[i]->size
> + >= lc_shadows[i+1]->data + lc_shadows[i+1]->size)
> + lc_shadows[i]->size = 0;
> tl_assert( lc_shadows[i]->data + lc_shadows[i]->size
> <= lc_shadows[i+1]->data );
> }
>
>
> But I understand this is not a proper fix. Simply removing the
> sanity check doesn't work either, as the binary search in
> find_shadow_for falls loose, as well as all the rest of the
> leakcheck logic.
What we really need is another client request that tells Valgrind that an
allocated heap block should not be considered a heap block, eg.
VALGRIND_IGNORE_HEAP_BLOCK(addr). You could then call this on the
superblock as soon as it is created. I think that would solve the problem.
It might be useful for other purposes, too, eg. as an alternative to a leak
suppression for a leak you don't care about.
Nick
|
|
From: Nicholas N. <nj...@cs...> - 2006-05-28 07:13:11
Attachments:
ignore_heap_block.patch
|
On Sat, 27 May 2006, Nicholas Nethercote wrote: >> Here is the assertion: >> >> Memcheck: mc_leakcheck.c:707 (vgMemCheck_do_detect_memory_leaks): >> Assertion 'lc_shadows[i]->data + lc_shadows[i]->size <= >> lc_shadows[i+1]->data' failed. > > What we really need is another client request that tells Valgrind that an > allocated heap block should not be considered a heap block, eg. > VALGRIND_IGNORE_HEAP_BLOCK(addr). You could then call this on the superblock > as soon as it is created. I think that would solve the problem. It might be > useful for other purposes, too, eg. as an alternative to a leak suppression > for a leak you don't care about. I've implemented this, a patch is attached. It would be great if you can try it out and see if it works. (Ashley, if it's suitable for you, it would be great if you can try it too.) Try just running your code as-is, you should get an error message that is informative enough to work out how to fix things. Thanks. Nick |
|
From: Behdad E. <be...@cs...> - 2006-05-28 17:41:48
|
On Sun, 28 May 2006, Nicholas Nethercote wrote: > On Sat, 27 May 2006, Nicholas Nethercote wrote: > > >> Here is the assertion: > >> > >> Memcheck: mc_leakcheck.c:707 (vgMemCheck_do_detect_memory_leaks): > >> Assertion 'lc_shadows[i]->data + lc_shadows[i]->size <= > >> lc_shadows[i+1]->data' failed. > > > > What we really need is another client request that tells Valgrind that an > > allocated heap block should not be considered a heap block, eg. > > VALGRIND_IGNORE_HEAP_BLOCK(addr). You could then call this on the superblock > > as soon as it is created. I think that would solve the problem. It might be > > useful for other purposes, too, eg. as an alternative to a leak suppression > > for a leak you don't care about. > > I've implemented this, a patch is attached. It would be great if you can > try it out and see if it works. (Ashley, if it's suitable for you, it would > be great if you can try it too.) Try just running your code as-is, you > should get an error message that is informative enough to work out how to > fix things. Thanks. Thanks, almost works. Except for: ==29152== Invalid free() / delete / delete[] ==29152== at 0x4004EF1: free (vg_replace_malloc.c:233) ==29152== by 0x406AD5B: slab_allocator_free_chunk (gslice.c:1157) ==29152== by 0x406CEBD: g_slice_free_chain_with_offset (gslice.c:923) ==29152== by 0x406D8F5: g_slist_free (gslist.c:53) ==29152== by 0x4070125: g_strsplit (gstrfuncs.c:2281) ==29152== by 0x407CF8B: g_get_language_names (gutils.c:2864) ==29152== by 0x407D389: _g_utils_thread_init (gutils.c:2992) ==29152== by 0x4073C89: g_thread_init_glib (gthread.c:151) ==29152== by 0x400A496: g_thread_init (gthread-impl.c:353) ==29152== by 0x8048EED: main (slice-test.c:273) ==29152== Address 0x40BD100 is 16 bytes before a block of size 8 alloc'd ==29152== at 0x406B54F: g_slice_alloc (gslice.c:815) ==29152== by 0x406D679: g_slist_prepend (gslist.c:91) ==29152== by 0x4070169: g_strsplit (gstrfuncs.c:2272) ==29152== by 0x407CF8B: g_get_language_names (gutils.c:2864) ==29152== by 0x407D389: _g_utils_thread_init (gutils.c:2992) ==29152== by 0x4073C89: g_thread_init_glib (gthread.c:151) ==29152== by 0x400A496: g_thread_init (gthread-impl.c:353) ==29152== by 0x8048EED: main (slice-test.c:273) Which is expected I guess: when the ignored superblock is freed, valgrind will not find a matching malloc. I think simply marking those blocks as ignored (instead of removing them from the list) may be easier. Having said all these, I've also got a question about valgrind's client requests and versioning. I observe that client requests generated using headers from valgrind-3.1.0-2 do not work with valgrind HEAD and vice versa. Given that valgrind.h/memcheck.h are suggested to be copied into user projects, I'm wondering what stability promises do valgrind make. Or for example, if this new hint is added, is there a way to detect that and not enable the hints if the new hint is not available? > Nick Thanks a lot, --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |
|
From: Ashley P. <as...@qu...> - 2006-05-30 13:23:32
|
On Sun, 2006-05-28 at 13:41 -0400, Behdad Esfahbod wrote: > Given that valgrind.h/memcheck.h > are suggested to be copied into user projects Is that true? As a header file surely it should be included via a #include line rather than copied and distributed with the user project. > I'm wondering what > stability promises do valgrind make. Or for example, if this new > hint is added, is there a way to detect that and not enable the > hints if the new hint is not available? There are two issues here, firstly there was a major change (in practical terms at least) where Julian changed the magic for making client requests, there is now a "old method" and a "new method", the new code coming into force with r5603 in February. The client requests themselves did not change at this time, merely the underlying magic sequence so the same client code will work with either set, as long as it was compiled with the correct header for the running version. This is a good reason for not shipping your own copies of the valgrind headers. As regards promises about stability, I certainly noticed and raised it on the list (I'm a valgrind user, not a developer) when this change was made, the consensus at the time was that the need for a ABI wasn't something that had been considered up to that point. Effort was taken at that time to ensure that the new method was future-proof so there should be no need for more changes if future. The second issue is the question of how to tell if a specific hint is supported, most of them have a return code of 0 if not running on valgrind and 1 for success, other values for different error cases. Valgrind itself gives a warning to the user if you make a client request it doesn't know about. All the client requests return 0 if you are using the wrong headers. Ashley, |
|
From: Behdad E. <be...@cs...> - 2006-05-30 13:55:47
|
On Tue, 30 May 2006, Ashley Pittman wrote: > On Sun, 2006-05-28 at 13:41 -0400, Behdad Esfahbod wrote: > > Given that valgrind.h/memcheck.h > > are suggested to be copied into user projects > > Is that true? As a header file surely it should be included via a > #include line rather than copied and distributed with the user project. Yes, it is true. I certainly know how to #include a header! Just read the second comment block in valgrind.h: /* This file is for inclusion into client (your!) code. ... > > I'm wondering what > > stability promises do valgrind make. Or for example, if this new > > hint is added, is there a way to detect that and not enable the > > hints if the new hint is not available? > > There are two issues here, firstly there was a major change (in > practical terms at least) where Julian changed the magic for making > client requests, there is now a "old method" and a "new method", the new > code coming into force with r5603 in February. I expected a major version change with such a breakage. > The client requests themselves did not change at this time, merely the > underlying magic sequence so the same client code will work with either > set, as long as it was compiled with the correct header for the running > version. This is a good reason for not shipping your own copies of the > valgrind headers. But there are other reasons for why including the headers is a good idea, and that's the recommended way anyway. > The second issue is the question of how to tell if a specific hint is > supported, most of them have a return code of 0 if not running on > valgrind and 1 for success, other values for different error cases. > Valgrind itself gives a warning to the user if you make a client request > it doesn't know about. All the client requests return 0 if you are > using the wrong headers. Humm, didn't find this documented anywhere in the headers, thanks. > Ashley, --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |
|
From: Ashley P. <as...@qu...> - 2006-05-30 14:56:49
|
On Tue, 2006-05-30 at 09:55 -0400, Behdad Esfahbod wrote: > On Tue, 30 May 2006, Ashley Pittman wrote: > > > On Sun, 2006-05-28 at 13:41 -0400, Behdad Esfahbod wrote: > > > Given that valgrind.h/memcheck.h > > > are suggested to be copied into user projects > > > > Is that true? As a header file surely it should be included via a > > #include line rather than copied and distributed with the user project. > > Yes, it is true. I certainly know how to #include a header! > Just read the second comment block in valgrind.h: > > /* This file is for inclusion into client (your!) code. > ... That was the comment I thought you were referring, I didn't take it to mean the files should be copied however, simply included via #include as needed. > > The second issue is the question of how to tell if a specific hint is > > supported, most of them have a return code of 0 if not running on > > valgrind and 1 for success, other values for different error cases. > > Valgrind itself gives a warning to the user if you make a client request > > it doesn't know about. All the client requests return 0 if you are > > using the wrong headers. > > Humm, didn't find this documented anywhere in the headers, I'm thinking specifically about the client requests in memcheck.h. Ashley, |
|
From: Nicholas N. <nj...@cs...> - 2006-05-28 20:39:58
|
On Sun, 28 May 2006, Behdad Esfahbod wrote: >>> What we really need is another client request that tells Valgrind that an >>> allocated heap block should not be considered a heap block, eg. >>> VALGRIND_IGNORE_HEAP_BLOCK(addr). > > Thanks, almost works. Except for: > > ==29152== Invalid free() / delete / delete[] > [...] > > Which is expected I guess: when the ignored superblock is freed, > valgrind will not find a matching malloc. I think simply marking > those blocks as ignored (instead of removing them from the list) > may be easier. Hmm, that's frustrating. > Having said all these, I've also got a question about valgrind's > client requests and versioning. I observe that client requests > generated using headers from valgrind-3.1.0-2 do not work with > valgrind HEAD and vice versa. Can you give more details? They're supposed to keep working between different versions. > Given that valgrind.h/memcheck.h are suggested to be copied into user > projects, I'm wondering what stability promises do valgrind make. Or for > example, if this new hint is added, is there a way to detect that and not > enable the hints if the new hint is not available? I don't understand what you're asking. If Valgrind sees a client request it doesn't recognise (eg. if you're using a client request from a newer valgrind.h with an older Valgrind) it just ignores it, and possibly prints a warning (depending on the verbosity setting). Nick |
|
From: Behdad E. <be...@cs...> - 2006-05-28 21:08:25
|
On Sun, 28 May 2006, Nicholas Nethercote wrote:
> > Having said all these, I've also got a question about valgrind's
> > client requests and versioning. I observe that client requests
> > generated using headers from valgrind-3.1.0-2 do not work with
> > valgrind HEAD and vice versa.
>
> Can you give more details? They're supposed to keep working between
> different versions.
If I compile with valgrind.h and memcheck.h from
/usr/include/valgrind on this FC5 with valgrind-3.1.0-2
installed, but run under valgrind from svn HEAD, the
VALGRIND_IS_RUNNING macro returns zero. If I compile with
headers from svn HEAD and run with /usr/bin/valgrind, again,
VALGRIND_IS_RUNNING returns zero. This is on a x86 laptop.
This is the magic sequence in /usr/include/valgrind/valgrind.h:
__asm__ volatile("roll $29, %%eax ; roll $3, %%eax\n\t" \
"rorl $27, %%eax ; rorl $5, %%eax\n\t" \
"roll $13, %%eax ; roll $19, %%eax" \
and special instruction from svn HEAD:
#define __SPECIAL_INSTRUCTION_PREAMBLE \
"roll $3, %%edi ; roll $13, %%edi\n\t" \
"roll $29, %%edi ; roll $19, %%edi\n\t"
> > Given that valgrind.h/memcheck.h are suggested to be copied into user
> > projects, I'm wondering what stability promises do valgrind make. Or for
> > example, if this new hint is added, is there a way to detect that and not
> > enable the hints if the new hint is not available?
>
> I don't understand what you're asking. If Valgrind sees a client request it
> doesn't recognise (eg. if you're using a client request from a newer
> valgrind.h with an older Valgrind) it just ignores it, and possibly prints a
> warning (depending on the verbosity setting).
I want to make sure users never hit the assertion failure in
valgrind. So, if the newer client request is not available at
runtime, I'd rather turn all the hints off. Adding a client
request to get the version of the running valgrind solves that.
> Nick
Thanks,
--behdad
|
|
From: Nicholas N. <nj...@cs...> - 2006-05-28 21:54:57
|
On Sun, 28 May 2006, Behdad Esfahbod wrote:
> This is the magic sequence in /usr/include/valgrind/valgrind.h:
>
> __asm__ volatile("roll $29, %%eax ; roll $3, %%eax\n\t" \
> "rorl $27, %%eax ; rorl $5, %%eax\n\t" \
> "roll $13, %%eax ; roll $19, %%eax" \
>
> and special instruction from svn HEAD:
>
> #define __SPECIAL_INSTRUCTION_PREAMBLE \
> "roll $3, %%edi ; roll $13, %%edi\n\t" \
> "roll $29, %%edi ; roll $19, %%edi\n\t"
Oh, that's bad.
It looks like these were changed when function wrapping was added for 3.2.0.
Normally client requests are backwards compatible, but in this case they're
not.
Julian, we should document this, yes? Both in the NEWS file and maybe the
manual?
> I want to make sure users never hit the assertion failure in
> valgrind. So, if the newer client request is not available at
> runtime, I'd rather turn all the hints off. Adding a client
> request to get the version of the running valgrind solves that.
Ah. Perhaps better would be to provide new versions of MALLOCLIKE_BLOCK and
FREELIKE_BLOCK, which would do the same thing as they currently do, which
should be used in conjunction with IGNORE_HEAP_BLOCK. Then MALLOCLIKE_BLOCK
and FREELIKE_BLOCK could be deprecated and removed in the future.
I like that approach better than version numbers, which tend to be a big fat
pain in practice.
Nick
|
|
From: Behdad E. <be...@cs...> - 2006-05-28 23:17:01
|
On Sun, 28 May 2006, Nicholas Nethercote wrote: > > I want to make sure users never hit the assertion failure in > > valgrind. So, if the newer client request is not available at > > runtime, I'd rather turn all the hints off. Adding a client > > request to get the version of the running valgrind solves that. > > Ah. Perhaps better would be to provide new versions of MALLOCLIKE_BLOCK and > FREELIKE_BLOCK, which would do the same thing as they currently do, which > should be used in conjunction with IGNORE_HEAP_BLOCK. Then MALLOCLIKE_BLOCK > and FREELIKE_BLOCK could be deprecated and removed in the future. Having new MALLOCLIKE_BLOCK/FREELIKE_BLOCK hints that do not work with older valgrind versions can produce tons of errors, as those allocated areas will essentially become NOACCESS. > I like that approach better than version numbers, which tend to be a big fat > pain in practice. I'm not sure I agree. Returning a single integer encoding of the version should be very easy to implement and use, like returning 30100 for 3.1.0. > Nick --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |
|
From: Nicholas N. <nj...@cs...> - 2006-05-30 23:03:53
|
On Tue, 30 May 2006, Ashley Pittman wrote: > On Tue, 2006-05-30 at 09:55 -0400, Behdad Esfahbod wrote: >> On Tue, 30 May 2006, Ashley Pittman wrote: >> >>> On Sun, 2006-05-28 at 13:41 -0400, Behdad Esfahbod wrote: >>>> Given that valgrind.h/memcheck.h >>>> are suggested to be copied into user projects >>> >>> Is that true? As a header file surely it should be included via a >>> #include line rather than copied and distributed with the user project. >> >> Yes, it is true. I certainly know how to #include a header! >> Just read the second comment block in valgrind.h: >> >> /* This file is for inclusion into client (your!) code. >> ... > > That was the comment I thought you were referring, I didn't take it to > mean the files should be copied however, simply included via #include as > needed. Behdad, I can think of three possible ways of incorporating valgrind.h: 1. copying it directly into a C file (clearly silly) 2. making a local copy in your project and #include'ing that 3. #include'ing the installed copy from $INSTALL/include Number 3 is the intended method. Are you doing number 2? It's not clear to me because if you do number 3 I don't see why you should have problems with version mismatches between valgrind.h and the Valgrind program. Nick |
|
From: Patrick O. <pat...@in...> - 2006-05-31 06:46:05
|
On Wed, 2006-05-31 at 09:03 +1000, Nicholas Nethercote wrote: > Behdad, I can think of three possible ways of incorporating valgrind.h: > > 1. copying it directly into a C file (clearly silly) > 2. making a local copy in your project and #include'ing that > 3. #include'ing the installed copy from $INSTALL/include > > Number 3 is the intended method. Are you doing number 2? It's not clear to > me because if you do number 3 I don't see why you should have problems with > version mismatches between valgrind.h and the Valgrind program. Even with method 3 Behad (or a Linux distribution) might distribute libraries/binaries which were compiled against a valgrind.h with the old magic sequences, but the user then runs a more recent, incompatible valgrind. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. |
|
From: Olly B. <ol...@su...> - 2006-06-01 10:48:55
|
On 2006-05-30, Nicholas Nethercote <nj...@cs...> wrote: > Behdad, I can think of three possible ways of incorporating valgrind.h: > > 1. copying it directly into a C file (clearly silly) > 2. making a local copy in your project and #include'ing that > 3. #include'ing the installed copy from $INSTALL/include > > Number 3 is the intended method. Are you doing number 2? It's not clear to > me because if you do number 3 I don't see why you should have problems with > version mismatches between valgrind.h and the Valgrind program. The manual explicitly recommends method 2, and has for ages. See: http://valgrind.org/docs/manual/manual-core.html which says: You are encouraged to copy the valgrind/*.h headers into your project's include directory, so your program doesn't have a compile-time dependency on Valgrind being installed. The Valgrind headers, unlike the rest of the code, are under a BSD-style license so you may include them without worrying about license incompatibility. If the magic sequence needs to be changed, perhaps valgrind should issue a warning when it sees the old sequence, at least for the next few releases? Cheers, Olly |
|
From: Behdad E. <be...@cs...> - 2006-06-02 19:27:55
|
On Tue, 30 May 2006, Nicholas Nethercote wrote: > On Tue, 30 May 2006, Ashley Pittman wrote: > > > On Tue, 2006-05-30 at 09:55 -0400, Behdad Esfahbod wrote: > >> On Tue, 30 May 2006, Ashley Pittman wrote: > >> > >>> On Sun, 2006-05-28 at 13:41 -0400, Behdad Esfahbod wrote: > >>>> Given that valgrind.h/memcheck.h > >>>> are suggested to be copied into user projects > >>> > >>> Is that true? As a header file surely it should be included via a > >>> #include line rather than copied and distributed with the user project. > >> > >> Yes, it is true. I certainly know how to #include a header! > >> Just read the second comment block in valgrind.h: > >> > >> /* This file is for inclusion into client (your!) code. > >> ... > > > > That was the comment I thought you were referring, I didn't take it to > > mean the files should be copied however, simply included via #include as > > needed. > > Behdad, I can think of three possible ways of incorporating valgrind.h: > > 1. copying it directly into a C file (clearly silly) > 2. making a local copy in your project and #include'ing that > 3. #include'ing the installed copy from $INSTALL/include > > Number 3 is the intended method. Are you doing number 2? It's not clear to > me because if you do number 3 I don't see why you should have problems with > version mismatches between valgrind.h and the Valgrind program. Ah, ok. Yes, I was doing number 2, and the gslice maintainer had my same reading of "for inclusion into client (your!) code". BTW, I'm willing to work on a patch to make MALLOCLIKE inside malloc'ed block work, should you tell me what kind of approach you prefer. > Nick Thanks, --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |
|
From: Nicholas N. <nj...@cs...> - 2006-06-03 01:04:25
|
On Fri, 2 Jun 2006, Behdad Esfahbod wrote: >> Behdad, I can think of three possible ways of incorporating valgrind.h: >> >> 1. copying it directly into a C file (clearly silly) >> 2. making a local copy in your project and #include'ing that >> 3. #include'ing the installed copy from $INSTALL/include >> >> Number 3 is the intended method. Are you doing number 2? It's not clear to >> me because if you do number 3 I don't see why you should have problems with >> version mismatches between valgrind.h and the Valgrind program. > > Ah, ok. Yes, I was doing number 2, and the gslice maintainer had > my same reading of "for inclusion into client (your!) code". It turns out the manual recommends #2. Hmm. > BTW, I'm willing to work on a patch to make MALLOCLIKE inside > malloc'ed block work, should you tell me what kind of approach > you prefer. My IGNORE_HEAP_BLOCK approach is flawed because it didn't handle freeing of the superblock. One possibility would be to introduce IGNORE_NEXT_MALLOC and IGNORE_NEXT_FREE requests, which would do the obvious thing. They're simple (no arguments!) which is good, but they're stateful, which is not so nice. You mentioned having a valgrind.h versioning system to avoid the problem of a Valgrind possibly having MALLOCLIKE/FREELIKE but not IGNORE_*. Better I think would be another request SUPPORTS_IGNORE_REQUESTS. You could query that first. Version numbers are a pain, because they're arbitrary, and it's easy to forget to update them. (We've found that with the internal core/tool versioning system we have.) Nick |
|
From: Behdad E. <be...@cs...> - 2006-06-03 01:56:50
|
On Fri, 2 Jun 2006, Nicholas Nethercote wrote: > On Fri, 2 Jun 2006, Behdad Esfahbod wrote: > > >> Behdad, I can think of three possible ways of incorporating valgrind.h: > >> > >> 1. copying it directly into a C file (clearly silly) > >> 2. making a local copy in your project and #include'ing that > >> 3. #include'ing the installed copy from $INSTALL/include > >> > >> Number 3 is the intended method. Are you doing number 2? It's not clear to > >> me because if you do number 3 I don't see why you should have problems with > >> version mismatches between valgrind.h and the Valgrind program. > > > > Ah, ok. Yes, I was doing number 2, and the gslice maintainer had > > my same reading of "for inclusion into client (your!) code". > > It turns out the manual recommends #2. Hmm. And #2 is a lot easier maintenance-wise, fewer dependencies on client package, even if it's a soft dependency. > > BTW, I'm willing to work on a patch to make MALLOCLIKE inside > > malloc'ed block work, should you tell me what kind of approach > > you prefer. > > My IGNORE_HEAP_BLOCK approach is flawed because it didn't handle freeing of > the superblock. One way is to add a FAKE_MALLOC(mem) that should be called right before the free() call to create a block entry (with zero length). > You mentioned having a valgrind.h versioning system to avoid the problem of > a Valgrind possibly having MALLOCLIKE/FREELIKE but not IGNORE_*. Better I > think would be another request SUPPORTS_IGNORE_REQUESTS. You could query > that first. Version numbers are a pain, because they're arbitrary, and it's > easy to forget to update them. (We've found that with the internal > core/tool versioning system we have.) The problem with a SUPPORTS_IGNORE_REQUESTS is that the approach doesn't scale well. The version number on the other hand works pretty well, assuming that valgrind doesn't break backwards compatibility. Then one can simply for the minimum version they require, instead of asking for a new SUPPORTS_* hint to be added to the next version. As for updating them, I'm not suggesting a new interface version, just the valgrind version, and that's handled automatically by the build system. > Nick --behdad http://behdad.org/ "Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill" -- Dan Bern, "New American Language" |