Re: [Valgrind-developers] malloc (a new lib) not in uclibc, and we could not use valgrind, could you give me some advice ?

[Andrew C. M. <and...@gm...>]

2012/3/7 Julian Seward <js...@ac...>:
>
>> I've attached a sketch for a patch that moves all of the interception
>> applications to a common area at the end of vg_replace_malloc.c, right
>> before 'init'. In addition, the patch merges the relevant VGO_linux
>> and VGO_darwin conditional compilation sections,
>
> We used to have combined linux and darwin intercepts in most of the
> file.  I separated them all out some time last year.  The all-merged-
> together scheme looks good at first glance, but leads to maintenance
> and validation problems: I found it hard to figure out if a change to
> the intercept schemes for (eg) darwin would break linux, or vice
> versa.  This got particularly bad when dealing with memmove-vs-memcpy
> complexity in Linux last year.  In the end decided to make them
> independent, even if it meant a bit of code duplication.
>
> From where I'm sitting, one of the biggest problems with we have is
> not so much one of "fix bug X" or "add feature Y", but simply that
> of validating (usually around release time) that what we have actually
> works correctly.  As the number of supported architectures, OSs,
> OS versions, libc versions, other C libraries etc increases, the
> validation problem expands exponentially.
>
> J

Thank you for the detailed reply. If I'd realized that my patch
amounted to backing out your earlier work I would not have proposed
it. I do have a few follow up questions though:

- My motivation for the reorganization was to make it easier to
maintain my own custom valgrind fork that adds support for tcmalloc.
I'm not the only one doing this. I realize that adding support for
even one third-party allocator opens the door for everyone to clamor
for support for their personal favorite, and in addition makes the
release-time validation work that you mentioned above that much
harder. But providing some best effort level of support for one or two
widely used third-party allocators would be a popular feature. In
addition, a well integrated solution would provide guidance for others
who need to add support for other custom allocators in local forks.
Would a patch that integrated optional support for tcmalloc have a
shot at inclusion? If so I'm willing to work on it.

- If the answer is yes, do you have any thoughts on how best to
structure it? I don't think that just adding a big #ifdef'ed block of
tcmalloc specific interceptions at the end of vg_replace_malloc.c is
at all the right way to go. Scattering tcmalloc interceptions
throughout the file and further complicating the platform #ifdef
blocks doesn't seem right either. It might be nice to have all of the
tcmalloc specific items in a separate file, but that doesn't work
because 'init' in vg_replace_malloc.c needs to see all of the
interceptions. One possibility would be to place the tcmalloc
interceptions in a separate 'vg_replace_tcmalloc.c' file, and then
conditionally include it near the end of vg_replace_malloc.c. But
#including .c files is not particularly appealing either. Any
suggestions?

- The patch that I posted provided per-VG_WORDSIZE macros for the
mangled C++ names (to remove repeating the easy to misread or mistype
mangled names), created VG_WORDSIZE independent macros from those, and
then collapsed the conditional compilation around VG_WORDSIZE for the
C++ interceptions by using the new macros. Would there be interest in
a patch that did just this (or maybe just the first part of it), or do
you prefer to keep the variation along this axis explicit as well?

Sorry about the long questions...

Thanks,
Andrew