|
From: Nicholas N. <nj...@ca...> - 2003-03-09 16:37:11
|
On Fri, 7 Mar 2003, Josef Weidendorfer wrote:
> > Thinking about it some more, the only reason I can think of why valgrind
> > even needs its own malloc() is this:
Thinking more, another important reason is this: for memcheck/addrcheck,
having the redzones around malloc'd blocks allows them to catch
over/under-runs.
> As I see, a skin is able to implement it's own GLIBC wrapper functions, no
> matter if they are called malloc() or perhaps open(), simply by defining the
> function in the skin.
> This is simply another (but static) way of defining the behaviour of a skin.
> E.g. in vg_skin.h, do a
>
> #define REDIRECT1(fn,rType,pType1) \
> rType fn(pType1 p1) { return VG_(fn)(p1); }
>
> and in the skin, put
>
> REDIRECT1(malloc,void*,size_t)
>
> Of course it would be good to be able to check at runtime if the skin defined
> its own wrapper for a function: Use dlsym(RTLD_DEFAULT,"malloc") and check if
> the address is defined inside of the skin. You can issue a warning if no
> wrapper was defined when e.g. "new_mem_heap" is requested.
Well, dlsym() is not available for use within Valgrind, unless someone
implements VG_(dlsym)(). I think.
It's an interesting idea, but feels a bit messy to me... there's one more
thing for a skin to get wrong (ask for "new_mem_heap" but forget the
REDIRECT). Doing things statically with macros is a bit hacky too, but
maybe unavoidable.
But I do like the idea of the default behavour being "glibc malloc(), no
wrapper" and that a skin should have to request different behaviour...
kind of a "principle of least interference" with the original program.
It's worth thinking about the different possibilities here in more detail.
----
This is the sequence of events when a client calls malloc(), and
VG_(running_on_simd_CPU) == True.
1. client
- calls malloc()
2. vg_clientfuncs.c:malloc() wrapper is called:
- prints info, if --trace-malloc=yes
- checks arg, if negative:
- if core_checks needed, gives a warning
- returns NULL
- else calls SIMPLE_REQUEST1(VG_USERREQ__MALLOC, n)
3. vg_scheduler.c:do_client_request()
- calls VG_(client_malloc)()
4. vg_clientmalloc.c:VG_(client_malloc)():
- updates malloc stats (number of calls, bytes allocated)
- calls VG_(arena_malloc)()
- add a ShadowChunk (holding extra info about the block), if
needs_shadow_chunks is set
- calls new_mem_heap() event tracker, if there is one
(I don't understand why vg_clientfuncs.c:malloc() needs to use the client
request, rather than just calling VG_(client_malloc)() directly...
although when I tried it I got lots of "invalid read"s within
vg_malloc2.c only some programs... Julian?)
----
When must a skin use Valgrind's malloc?
- when red zones are needed
When must a skin use a malloc() wrapper?
- to print out debug info about malloc() calls
- if 'core_errors' is specified (it prints warning on negative args)
- to use new_mem_heap, etc
- to use ShadowChunks
Valgrind's own malloc() must of course still be used within the core.
What are the possible combinations:
a. no wrapper, glibc malloc()
b. no wrapper, valgrind malloc()
c. wrapper, glibc malloc()
d. wrapper, valgrind malloc
Memcheck would use (d), Helgrind would use (c) I think (doesn't need
redzones), Cachegrind & CallTree would use (a). No skins currently in
existence would use (b).
I think (c) can be achieved using ld's "--wrap symbol" option.
----
Here's my ideal scenario:
i. default is no wrapper + glibc malloc()
ii. skin can specify with a need if it wants to use Valgrind's malloc()
(eg. needs_redzones, or needs_special_malloc, or something)
iii. core decides whether a malloc() wrapper is needed (eg. because
new_mem_heap is being tracked, or --trace-malloc=yes is set, or
core errors or shadow chunks are needed
iv. if wrapper is needed, core calls glibc malloc() or Valgrind
malloc() depending on the needs_redzones/whatever need
I don't think all this is attainable. Esp (iii), since the conditions
there are all dynamic, but the overwriting by Valgrind of glibc's malloc()
symbol must be done statically.
I think wrapper + glibc() malloc might be attainable with the --wrap
linker option, but I'm not certain.
Josef, your REDIRECT idea is heading in the right direction, but not quite
there, I think.
----
So, lots of words from me, but no firm conclusions. Hmm.
N
|
|
From: Josef W. <Jos...@gm...> - 2003-03-10 13:17:03
|
On Sunday 09 March 2003 17:37, Nicholas Nethercote wrote:
> On Fri, 7 Mar 2003, Josef Weidendorfer wrote:
> > > Thinking about it some more, the only reason I can think of why
> > > valgrind even needs its own malloc() is this:
>
> Thinking more, another important reason is this: for memcheck/addrcheck,
> having the redzones around malloc'd blocks allows them to catch
> over/under-runs.
Yes, but that's not needed for a pure tracing/profiling skin like cachegrind,
too.
> > As I see, a skin is able to implement it's own GLIBC wrapper functions,
> > no matter if they are called malloc() or perhaps open(), simply by
> > defining the function in the skin.
> > This is simply another (but static) way of defining the behaviour of a
> > skin. E.g. in vg_skin.h, do a
> >
> > #define REDIRECT1(fn,rType,pType1) \
> > rType fn(pType1 p1) { return VG_(fn)(p1); }
> >
> > and in the skin, put
> >
> > REDIRECT1(malloc,void*,size_t)
> >
> > Of course it would be good to be able to check at runtime if the skin
> > defined its own wrapper for a function: Use dlsym(RTLD_DEFAULT,"malloc")
> > and check if the address is defined inside of the skin. You can issue a
> > warning if no wrapper was defined when e.g. "new_mem_heap" is requested.
>
> Well, dlsym() is not available for use within Valgrind, unless someone
> implements VG_(dlsym)(). I think.
Ok. Next try:
#define REDIRECT1(fn,rType,pType1) \
rType fn(pType1 p1) { return VG_(fn)(p1); } \
Boolean VG_(fn##_wrapper_defined) = True;
For all symbols where checking for a wrapper is needed, put e.g.
Boolean VG_(malloc_wrapper_defined) = False;
into valgrind core. As the skin lib is ahead of the valgrind lib in
LD_PRELOAD, you will get the "True" value if the wrapper is defined
(Perhaps the boolean in valgrind core must be a weak symbol).
> It's an interesting idea, but feels a bit messy to me... there's one more
> thing for a skin to get wrong (ask for "new_mem_heap" but forget the
> REDIRECT). Doing things statically with macros is a bit hacky too, but
> maybe unavoidable.
Best is to make the "ask for new_mem_heap but forget the REDIRECT" a fatal
error, checked at startup.
> But I do like the idea of the default behavour being "glibc malloc(), no
> wrapper" and that a skin should have to request different behaviour...
> kind of a "principle of least interference" with the original program.
Especially as KDE comes with a malloc implementation of its own. With current
valgrind, cachegrind can't check if the "KDE fast malloc" is really faster
than the glibc provided one.
>
> Here's my ideal scenario:
>
> i. default is no wrapper + glibc malloc()
>
> ii. skin can specify with a need if it wants to use Valgrind's malloc()
> (eg. needs_redzones, or needs_special_malloc, or something)
>
> iii. core decides whether a malloc() wrapper is needed (eg. because
> new_mem_heap is being tracked, or --trace-malloc=yes is set, or
> core errors or shadow chunks are needed
>
> iv. if wrapper is needed, core calls glibc malloc() or Valgrind
> malloc() depending on the needs_redzones/whatever need
>
>
> I don't think all this is attainable. Esp (iii), since the conditions
> there are all dynamic, but the overwriting by Valgrind of glibc's malloc()
> symbol must be done statically.
Valgrind core must be able to check whether the skin provides a wrapper.
This is best done with a second symbol definition, and relying on the runtime
linker for this (see above), as this is the same mechanism.
> I think wrapper + glibc() malloc might be attainable with the --wrap
> linker option, but I'm not certain.
I'm not sure about this. A program to be run under supervison of valgrind is
already linked; you can't change symbol names in it.
But GLIBC is prepared for wrapping its symbols: To get the real malloc, use
"__libc_malloc". As said above, KDE does the same wrapping with its "fast
malloc implementation" as we want to do here. You can have a look at the
code: Use the KDE CVS web frontend and look into "kdelibs/kdecore/malloc".
Josef
|