|
From: Jeremy F. <je...@go...> - 2005-02-28 21:18:41
|
(Note, last time I talked about these calls, I was making a bad mistake;
I assumed the size parameter of FREELIKE was the size of the block. It
is not; it is the size of the redzone, and this was causing me great
confusion.)
VALGRIND_MALLOCLIKE_BLOCK:
The main problem with MALLOCLIKE is that if you use it on a piece of
memory obtained from malloc(), it makes the leak-checker explode with an
assertion failure. This is because it creates a AllocCusttom heap chunk
embedded within a AllocMalloc chunk, which breaks the invarient that
heap chunks can't overlap.
It seems to me that if we allow heap chunks to be nested, it would give
us a lot of extra power. I think it would subsume the mempool
functionality in a more general way, and allow us to define sub-chunk of
memory for all sorts of purposes.
The semantics would be:
1. A new heap chunk is either not enclosed by another chunk, or is
completely contained by an outer chunk.
* It follows that if a chunk has sub-chunks, they're
completely contained.
2. If you free a chunk, it and all its subchunks are freed.
1 means that chunks are always properly nested. 2 defines the lifespan
of a chunk (never longer than its containing chunks).
I'm not precisely sure what the semantics of leak-checking would be in
the presence of nested chunks. It would depend on how they're being used:
1. If you have a memory pool, where all memory in the pool is freed
together, then the nested chunks should be scanned and marked if
the outer chunks are.
2. If you have an allocator which is carving a superblock into
independent allocations, then scanning and marking should avoid
sub-chunks; sub-chunks need to be explicitly referenced to be
found not leaked.
VALGRIND_FREELIKE_BLOCK
The main problem with VALGRIND_FREELIKE_BLOCK is that it immediately
forgets about the freed block. This prevents use-after-free messages
from being reported. I think it should remember freed blocks until the
memory is recycled (ie, overlapped by a VALGRIND_MALLOCLIKE_BLOCK).
J
|
|
From: Julian S. <js...@ac...> - 2005-02-28 21:54:00
|
> It seems to me that if we allow heap chunks to be nested, it would give > us a lot of extra power. I think it would subsume the mempool > functionality in a more general way, and allow us to define sub-chunk of > memory for all sorts of purposes. > > The semantics would be: > > 1. A new heap chunk is either not enclosed by another chunk, or is > completely contained by an outer chunk. > * It follows that if a chunk has sub-chunks, they're > completely contained. > 2. If you free a chunk, it and all its subchunks are freed. > > 1 means that chunks are always properly nested. 2 defines the lifespan > of a chunk (never longer than its containing chunks). What happens if you try to create a chunk which straddles the boundary of some other chunk? > I'm not precisely sure what the semantics of leak-checking would be in > the presence of nested chunks. It would depend on how they're being used: > > 1. If you have a memory pool, where all memory in the pool is freed > together, then the nested chunks should be scanned and marked if > the outer chunks are. > 2. If you have an allocator which is carving a superblock into > independent allocations, then scanning and marking should avoid > sub-chunks; sub-chunks need to be explicitly referenced to be > found not leaked. I'm loathe to allow more complexity like this into the code base without a clear customer-needs-this case. Although it's an interesting idea, I am very concerned with spiralling complexity and the resulting maintenance burden it gives. At some point we must stop adding functionality and instead concentrate on stabilising and making portable what we already have. And that point is now. If anything is to happen to functionality, I'd prefer to prune away little-used features to make V easier to maintain. J |
|
From: Jeremy F. <je...@go...> - 2005-02-28 23:23:48
|
Julian Seward wrote:
>What happens if you try to create a chunk which straddles the boundary
>of some other chunk?
>
>
That would indicate a bug in your allocator, so it would report a
problem at the very least. The options would be:
1. ignore the request and do nothing
2. split the new chunk into properly formed pieces
3. remove conflicting chunks and let the new chunk stand unmolested
Not sure which would be the best option. 1 is definitely simplest.
>I'm loathe to allow more complexity like this into the code base without
>a clear customer-needs-this case. Although it's an interesting idea,
>I am very concerned with spiralling complexity and the resulting
>maintenance burden it gives. At some point we must stop adding
>functionality and instead concentrate on stabilising and making
>portable what we already have. And that point is now.
>
>If anything is to happen to functionality, I'd prefer to prune
>away little-used features to make V easier to maintain.
>
>
Yeah, I'd agree with that (ie: VALGRIND_MAKE_*).
The MALLOCLIKE/FREELIKE stuff clearly serves a need for anyone who has
their own allocator (Valgrind, for example), but I'm guessing they
haven't been used much. The mempool changes are presumably useful,
otherwise Robert wouldn't have bothered with them. I'm looking to
condense the normal heap stuff and the mempool stuff into a single
unified mechanism, and make that mechanism actually work in a useful way.
But I'm not planning on doing anything beyond bug-fixing for now (which
I consider the MAKE_* issue to be).
J
|
|
From: Nicholas N. <nj...@cs...> - 2005-03-01 00:24:52
|
On Mon, 28 Feb 2005, Jeremy Fitzhardinge wrote: >> If anything is to happen to functionality, I'd prefer to prune >> away little-used features to make V easier to maintain. >> > Yeah, I'd agree with that (ie: VALGRIND_MAKE_*). > > The MALLOCLIKE/FREELIKE stuff clearly serves a need for anyone who has > their own allocator (Valgrind, for example), but I'm guessing they > haven't been used much. The mempool changes are presumably useful, > otherwise Robert wouldn't have bothered with them. I'm looking to > condense the normal heap stuff and the mempool stuff into a single > unified mechanism, and make that mechanism actually work in a useful way. > > But I'm not planning on doing anything beyond bug-fixing for now (which > I consider the MAKE_* issue to be). I'd be happy for VALGRIND_MAKE_* to not create its own blocks, I don't think we'd lose much with that. In fact, I vaguely recall that the memory address identification code has to do some awkward things to deal with those special blocks. As for MALLOCLIKE/FREELIKE, they're good enough for simple custom allocators. See memcheck/tests/custom_alloc.c for an example. I'm not surprised that they causes problems if you use it with malloc(); that's not something I'd considered, because I didn't think anyone would build a custom allocator on top of malloc(). There may certainly be room for improvement. N |
|
From: Jeremy F. <je...@go...> - 2005-03-01 00:49:49
|
Nicholas Nethercote wrote:
> I'd be happy for VALGRIND_MAKE_* to not create its own blocks, I don't
> think we'd lose much with that. In fact, I vaguely recall that the
> memory address identification code has to do some awkward things to
> deal with those special blocks.
Well, it checks to see if the address is within one. Do you think we
can do without them altogether, or there should be a client call to
create them? I've never used them, but I can see a use, particularly if
you can associate a descriptive string with them which is displayed when
hit.
> As for MALLOCLIKE/FREELIKE, they're good enough for simple custom
> allocators. See memcheck/tests/custom_alloc.c for an example.
custom_alloc.c is a little bit *too* simplistic, since it doesn't
implement free(), or any kind of freelist, or any block recycling. Its
the freelist management which bites you.
> I'm not surprised that they causes problems if you use it with
> malloc(); that's not something I'd considered, because I didn't think
> anyone would build a custom allocator on top of malloc(). There may
> certainly be room for improvement.
I ran into it by accident while trying to come up with a minimal but
slightly more complete replacement for custom_alloc.c. At the very
least the leak checker shouldn't assert over them. Should it simply
ignore AllocCustom chunks if they're embedded within another chunk?
J
|