|
From: Jason E. <ja...@ca...> - 2005-05-02 23:30:28
|
I've run into two problems with valgrind that the following patch works
around:
========================================================================
diff -ru valgrind-2.4.0.orig/memcheck/mac_leakcheck.c valgrind-2.4.0/memcheck/mac_leakcheck.c
--- valgrind-2.4.0.orig/memcheck/mac_leakcheck.c 2005-03-10 22:28:15.000000000 -0800
+++ valgrind-2.4.0/memcheck/mac_leakcheck.c 2005-05-02 16:16:22.659375486 -0700
@@ -431,7 +431,6 @@
lc_do_leakcheck(i);
sk_assert(lc_markstack_top == -1);
- sk_assert(lc_markstack[i].state == IndirectLeak);
lc_markstack[i].state = Unreached; /* Return to unreached state,
to indicate its a clique
@@ -589,7 +588,7 @@
/* Sanity check -- make sure they don't overlap */
for (i = 0; i < lc_n_shadows-1; i++) {
sk_assert( lc_shadows[i]->data + lc_shadows[i]->size
- < lc_shadows[i+1]->data );
+ <= lc_shadows[i+1]->data );
}
if (lc_n_shadows == 0) {
========================================================================
I ran into these problems when adding VALGRIND_{MALLOC,FREE}LIKE_BLOCK()
calls to a malloc library that I implemented. The malloc library
completely replaces all malloc(3)-related APIs. In order to avoid UMRs, I
have to tell valgrind that the separators between regions are allocated,
and the same applies to data structures that are stored in cached free
regions.
I don't understand why the IndirectLeak assertion fails for my code, but
I'm guessing that it's an assertion that is only valid for valgrind's own
malloc replacement.
The lc_shadows assertion fails because it assumes that allocations cannot
be directly adjacent.
Thanks,
Jason
P.S. As an aside, I must say that I am very impressed at how gracefully
valgrind deals with the application replacing malloc. My initial
assumption was that it wouldn't work at all.
|
|
From: Jeremy F. <je...@go...> - 2005-05-06 09:11:59
|
Jason Evans wrote:
>I've run into two problems with valgrind that the following patch works
>around:
>
>========================================================================
>diff -ru valgrind-2.4.0.orig/memcheck/mac_leakcheck.c valgrind-2.4.0/memcheck/mac_leakcheck.c
>--- valgrind-2.4.0.orig/memcheck/mac_leakcheck.c 2005-03-10 22:28:15.000000000 -0800
>+++ valgrind-2.4.0/memcheck/mac_leakcheck.c 2005-05-02 16:16:22.659375486 -0700
>@@ -431,7 +431,6 @@
> lc_do_leakcheck(i);
>
> sk_assert(lc_markstack_top == -1);
>- sk_assert(lc_markstack[i].state == IndirectLeak);
>
> lc_markstack[i].state = Unreached; /* Return to unreached state,
> to indicate its a clique
>@@ -589,7 +588,7 @@
> /* Sanity check -- make sure they don't overlap */
> for (i = 0; i < lc_n_shadows-1; i++) {
> sk_assert( lc_shadows[i]->data + lc_shadows[i]->size
>- < lc_shadows[i+1]->data );
>+ <= lc_shadows[i+1]->data );
> }
>
> if (lc_n_shadows == 0) {
>========================================================================
>
>I ran into these problems when adding VALGRIND_{MALLOC,FREE}LIKE_BLOCK()
>calls to a malloc library that I implemented. The malloc library
>completely replaces all malloc(3)-related APIs. In order to avoid UMRs, I
>have to tell valgrind that the separators between regions are allocated,
>and the same applies to data structures that are stored in cached free
>regions.
>
>I don't understand why the IndirectLeak assertion fails for my code, but
>I'm guessing that it's an assertion that is only valid for valgrind's own
>malloc replacement.
>
>
It's a bit more subtle than that. Valgrind has an assumption that
malloc blocks are not nested, so you can't have a user-defined allocated
block within a malloced block. The workaround is to use mmap() to
allocate memory for your malloc replacement.
J
|
|
From: Jason E. <ja...@ca...> - 2005-05-06 15:18:54
|
On Thu, May 05, 2005 at 11:06:03PM -0700, Jeremy Fitzhardinge wrote:
> Jason Evans wrote:
>
> >========================================================================
> >diff -ru valgrind-2.4.0.orig/memcheck/mac_leakcheck.c valgrind-2.4.0/memcheck/mac_leakcheck.c
> >--- valgrind-2.4.0.orig/memcheck/mac_leakcheck.c 2005-03-10 22:28:15.000000000 -0800
> >+++ valgrind-2.4.0/memcheck/mac_leakcheck.c 2005-05-02 16:16:22.659375486 -0700
> >@@ -589,7 +588,7 @@
> > /* Sanity check -- make sure they don't overlap */
> > for (i = 0; i < lc_n_shadows-1; i++) {
> > sk_assert( lc_shadows[i]->data + lc_shadows[i]->size
> >- < lc_shadows[i+1]->data );
> >+ <= lc_shadows[i+1]->data );
> > }
> >
> > if (lc_n_shadows == 0) {
> >========================================================================
> >
> >I ran into these problems when adding VALGRIND_{MALLOC,FREE}LIKE_BLOCK()
> >calls to a malloc library that I implemented. The malloc library
> >completely replaces all malloc(3)-related APIs. In order to avoid UMRs, I
> >have to tell valgrind that the separators between regions are allocated,
> >and the same applies to data structures that are stored in cached free
> >regions.
> >
> >
> It's a bit more subtle than that. Valgrind has an assumption that
> malloc blocks are not nested, so you can't have a user-defined allocated
> block within a malloced block. The workaround is to use mmap() to
> allocate memory for your malloc replacement.
If I understand correctly, this is bad (overlapping):
[------]
[-------]
And this is bad (nested):
[----------]
[----]
But this is the scenario that is causing me trouble (adjacent):
[------I------]
The lc_shadows assertion requires there to be a gap of at least one byte
between allocations; merely being non-overlapping isn't good enough to
avoid triggering the assertion:
[------] [------]
Is it in fact important to valgrind that there be gaps between allocations?
Thanks,
Jason
P.S. As for my malloc implementation, it does use mmap(), and it does take
care to avoid ever telling valgrind that allocations overlap/nest
(which BTW made adding the valgrind macro calls very tricky). It's a
complete malloc replacement, rather than an allocator that builds on
top of the system malloc.
|
|
From: Nicholas N. <nj...@cs...> - 2005-05-06 15:26:47
|
On Fri, 6 May 2005, Jason Evans wrote: > If I understand correctly, this is bad (overlapping): > > [------] > [-------] > > And this is bad (nested): > > [----------] > [----] > > But this is the scenario that is causing me trouble (adjacent): > > [------I------] > > The lc_shadows assertion requires there to be a gap of at least one byte > between allocations; merely being non-overlapping isn't good enough to > avoid triggering the assertion: > > [------] [------] > > Is it in fact important to valgrind that there be gaps between allocations? Off the top of my head, I'd guess no, and that the lc_shadows assertion is too restrictive. (I don't know about the lc_markstack assertion, that might be a bug.) One downside of such allocations is that if your allocator doesn't have any redzones or metadata between blocks, you can't detect an overrun from one block into another, because it's all considered addressable by Memcheck. N |
|
From: Jason E. <ja...@ca...> - 2005-05-06 17:04:42
|
On Fri, May 06, 2005 at 10:26:35AM -0500, Nicholas Nethercote wrote:
>
> One downside of such allocations is that if your allocator doesn't have
> any redzones or metadata between blocks, you can't detect an overrun from
> one block into another, because it's all considered addressable by
> Memcheck.
Ahh, good point. I do have optional red zones embedded in the separators
that delimit user-allocated regions:
RRRR....RRRR
---------
RRRR....RRRR
----------
RRRR....RRRR
However, I was making the mistake of both telling valgrind that the
allocations (-----) have red zones (RRRR), and that the red zones are
allocated as part of the separators (RRRR....RRRR).
I've fixed my code to never tell valgrind that the red zones are allocated,
and it longer trips on the assertions, as long as red zones are enabled.
However, if red zones are disabled, my code still trips on both assertions
that the original patch addressed. In that case, memory looks like:
....
--------
....
------------
....
And valgrind has been told that the user allocations (----) and the
separators (....) are allocated. None of the allocations overlap, and I'm
no longer making the mistake of telling valgrind that red zones are
allocated.
So, as near as I can tell, the patch is still relevant, though my code no
longer trips on the bad assertions.
Thanks,
Jason
|