Very Computer

Hi,

I believe that there is a flaw in the memory allocation hooks as they
implemented in the GNU C library that make impossible correct
implementation of the memory debugging software that suppose to work
in multi-threaded environment.

When I started to implement a simple memory de* (very similar to
mcheck/-lmcheck) I looked first at the manual. Here is what is written
in the manual:

Quote:> Variable __malloc_hook
> The value of this variable is a pointer to the function
> that malloc uses whenever it is called.
> ………

So I setup these hooks at the initialization time (using
__malloc_initialize_hook) and call called functions that are pointed
by old values by hook variables whenever I needed to call
malloc/free/.. functions internally, to avoid endless recursion.

What I’ve found is that it works only if MALLOC_CHECK_
environment variable
is set – otherwise initial value of __malloc_hook variable (and
others)
is zero and my code crash trying to call a function with zero address.

After studying libc sources and some experiments I found:

- Implementation of hooks is different from how it described in the
manual. Initial value of hooks is zero and “malloc.c” code
check it for zero and if
it is not zero calls the hook function. Otherwise it is just continue
the execution. And, which is important, this call of the hook is being
executed before taking a lock.

- Example code from the manual using a different method. When it needs
to call malloc internally it restore previous value of __malloc_hook,
calls the regular malloc() entry point and then set up its own hook
pointer to this variable again. Note that it could not work correctly
if there are more than one thread calls memory allocation functions
– there are short periods of time when hook variables are
restored to their old values and if a different thread calls the
corresponding memory allocation function at this time, it will go
directly to native malloc code (or to the previously installed hook)
rather than our hook code. And test I’ve written have showed
this.

After I studied MALLOC_CHECK_ implementation I’ve found that it
avoids this error by using internal malloc function (_int_malloc) and
never removes its hook. Another similar library function
mcheck(-lmcheck) does not do it – it uses technique similar to
the one used in the example from the manual and it will crash if used
in multi-threaded program with intensive use of memory allocation
functions.

So:
I would consider it as a bug in the GNU libc memory allocation
function. I see two ways to fix it:

1. To implement what is described in the manual. Make hook variables
pointing to the standard malloc/… functions implementation at
the very beginning and do indirect call from external
malloc/free/… functions.
2. Call hooks from under a global lock (arena locks do not good for
that and this solution could decrease performance).

And there is a way around (though also hitting performance) –
enable MALLOCK_CHECK_ (which is installed first) and then use my
initial techniques, calling function indirectly and never deinstalling
hooks.

         Any ideas / notes?

    Regards,

           Michael Furman  

Ok.  I think I understand your problem and can suggest a solution.

Quote:> When I started to implement a simple memory de* (very similar to
> mcheck/-lmcheck) I looked first at the manual. Here is what is written
> in the manual:

> > Variable __malloc_hook
> > The value of this variable is a pointer to the function
> > that malloc uses whenever it is called.
> > ………

> So I setup these hooks at the initialization time (using
> __malloc_initialize_hook) and call called functions that are pointed
> by old values by hook variables whenever I needed to call
> malloc/free/.. functions internally, to avoid endless recursion.

That won't work for multiple threads, as you've found out and
correctly analyzed below.

Note that the sentence in the manual can be read as:

    The value of this variable is _a pointer to the function_
    that malloc _uses_ whenever it is called.

So, the pointer (and not necessarily the function) is "used".  It
doesn't actually say that the pointer is _always_ dereferenced with a
function call.  Suggestions for clearer language here are welcome.

However, when you set up the hook variables to your own functions, you
can be sure they will be called always.  So you can provide your own
malloc implementation, that is the whole idea behind the hooks.

Quote:> - Implementation of hooks is different from how it described in the
> manual. Initial value of hooks is zero and “malloc.c” code
> check it for zero and if
> it is not zero calls the hook function. Otherwise it is just continue
> the execution. And, which is important, this call of the hook is being
> executed before taking a lock.

Yes, this is intentional and has been so for a long time.
Although the term "use" in the manual is admittedly vague, I believe
the behaviour is consistent with the manual.
Taking a lock here would slow down all (even single-threaded) apps
noticeably.

Quote:> - Example code from the manual using a different method. When it needs
> to call malloc internally it restore previous value of __malloc_hook,
> calls the regular malloc() entry point and then set up its own hook
> pointer to this variable again. Note that it could not work correctly
> if there are more than one thread calls memory allocation functions
> – there are short periods of time when hook variables are
> restored to their old values and if a different thread calls the
> corresponding memory allocation function at this time, it will go
> directly to native malloc code (or to the previously installed hook)
> rather than our hook code. And test I’ve written have showed
> this.

Yes this is correct; basically, the hook mechanism is not thread-safe
(and cannot possibly be!) _when the hooks keep changing_.

Quote:> After I studied MALLOC_CHECK_ implementation I’ve found that it
> avoids this error by using internal malloc function (_int_malloc) and
> never removes its hook. Another similar library function
> mcheck(-lmcheck) does not do it – it uses technique similar to
> the one used in the example from the manual and it will crash if used
> in multi-threaded program with intensive use of memory allocation
> functions.

Also correct, -lmcheck isn't thread-safe (it never was).

Quote:> I would consider it as a bug in the GNU libc memory allocation
> function. I see two ways to fix it:

> 1. To implement what is described in the manual. Make hook variables
> pointing to the standard malloc/… functions implementation at
> the very beginning and do indirect call from external
> malloc/free/… functions.

The reasons this cannot simply be done are somewhat involved; one is
that it would become _very_ difficult to ensure that malloc's
initialization function is called before the very first malloc call
(this can happen even before any other call to a libc function!).

Quote:> 2. Call hooks from under a global lock (arena locks do not good for
> that and this solution could decrease performance).

Exactly; for this reason this is unacceptable, even single-threaded
apps would be slowed down unnecessarily.

Quote:>          Any ideas / notes?

I'd add the following solution:

3. Change the hooks to permanent, constant pointers to your own
   functions.  Within your own functions, provide a thread-safe malloc
   yourself.  If you can't/don't want to provide that, just wrap
   _int_malloc() calls with a global lock, e.g. as follows

#include <pthread.h>
#include <malloc.h>

static mstate my_arena;
static int my_initialised = 0;
static pthread_mutex_t my_mutex = PTHREAD_MUTEX_INITIALIZER;

void * my_malloc(size_t sz)
{
        void * result;

        pthread_mutex_lock(&my_mutex);
        if (my_initialised == 0) {
                my_arena = _int_new_arena(sz + 4096); /* just a hint */
                my_initialised = 1;
        }
        result = _int_malloc(my_state, sz);
        pthread_mutex_unlock(&my_mutex);
        return result;

Quote:}

...

etc. for the other allocation functions.

The _int_malloc() etc. functions are here to stay (from glibc-2.3
onward), so don't hesitate to use them :-).

Regards,
Wolfram.

Hi, Wolfram,
thank you for your answer. Here are my couple of notes:

Quote:> [snip]

> Note that the sentence in the manual can be read as:

>     The value of this variable is _a pointer to the function_
>     that malloc _uses_ whenever it is called.

> So, the pointer (and not necessarily the function) is "used".  It
> doesn't actually say that the pointer is _always_ dereferenced with a
> function call.  Suggestions for clearer language here are welcome.

So, "pointer to the function" could be used for something else then to call
the function... And also you sugest that null pointer is refered as "pointer
to
the function" in this context... I agree that wording in the manual is
perfect, but I don't believe in your interpretation...

Quote:

> However, when you set up the hook variables to your own functions, you
> can be sure they will be called always.  So you can provide your own
> malloc implementation, that is the whole idea behind the hooks.

Yes, but what I am doing is the malloc tracer, so I want to use the original
malloc implementation. And, bu the way either athe sample from the
manual and -lmcheck use it in this way.

Quote:>  [snip]

> > I would consider it as a bug in the GNU libc memory allocation
> > function. I see two ways to fix it:

> > 1. To implement what is described in the manual. Make hook variables
> > pointing to the standard malloc/&#8230; functions implementation at
> > the very beginning and do indirect call from external
> > malloc/free/&#8230; functions.

> The reasons this cannot simply be done are somewhat involved; one is
> that it would become _very_ difficult to ensure that malloc's
> initialization function is called before the very first malloc call
> (this can happen even before any other call to a libc function!).

I thing you contradict to what you are saying below: why not to set it
statically? And also MALLOC_CHECK_ install itself dynamically,
but earlier then to __malloc_initialize_hook is being called.

Quote:> [snip]
> I'd add the following solution:

> 3. Change the hooks to permanent, constant pointers to your own
>    functions.  Within your own functions, provide a thread-safe malloc
>    yourself.  If you can't/don't want to provide that, just wrap
>    _int_malloc() calls with a global lock, e.g. as follows

> #include <pthread.h>
> #include <malloc.h>

> static mstate my_arena;
> static int my_initialised = 0;
> static pthread_mutex_t my_mutex = PTHREAD_MUTEX_INITIALIZER;

> void * my_malloc(size_t sz)
> {
> void * result;

> pthread_mutex_lock(&my_mutex);
> if (my_initialised == 0) {
> my_arena = _int_new_arena(sz + 4096); /* just a hint */
> my_initialised = 1;
> }
> result = _int_malloc(my_state, sz);
> pthread_mutex_unlock(&my_mutex);
> return result;
> }
> ...

> etc. for the other allocation functions.

> The _int_malloc() etc. functions are here to stay (from glibc-2.3
> onward), so don't hesitate to use them :-).

Yes, I can do it. But I prefer to use a little other software internals as I
can -
I believe there good reasons for them. Ind, BTW, I dont need the global lock
at the time I am calling malloc.

So, IMO changing the definitions of hook variables so they points to the
default implementation would be a best solution. The only problem I
can see with it that some people could already use Libc internals
in some contradicting way - and it is a good example of my reasons
not to use _int_malloc :-).

   Thank you for your response,

                regards,
                          Michael Furman