[PATCH] Fix mtrace vs. mudflap coexistence

[PATCH] Fix mtrace vs. mudflap coexistence

[Jakub Jelinek]

Hi!

When an application linked with alternate malloc implementation that
internally calls the glibc (next) malloc (such as when built with
mudflap) is mtraced and the alternate malloc doesn't allow reentrancy,
the application hangs.
The hang is:
main
malloc (in mudflap)
malloc (in glibc, pointer from dlsym (RTLD_NEXT, "malloc"))
tr_mallochook (mtrace registered libc malloc hook)
malloc (in mudflap)
and as mudflap uses a non-recursive mutex, it gets stuck.
The following patch fixes this by making sure the glibc malloc (free,
realloc, memalign) is always called from mtrace and mcheck hooks.

2005-12-19  Jakub Jelinek  <jakub@redhat.com>

	* malloc/mtrace.c (__libc_malloc, __libc_free, __libc_realloc,
	__libc_memalign): Add prototypes and libc_hidden_proto resp.
	defines for !_LIBC.
	(tr_freehook): Call __libc_free instead of free.
	(tr_mallochook): Call __libc_malloc instead of malloc.
	(tr_reallochook): Call __libc_realloc instead of realloc.
	(tr_memalignhook): Call __libc_memalign instead of memalign.
	* malloc/mcheck.c: Include stdlib.h.
	(__libc_malloc, __libc_free, __libc_realloc,
	__libc_memalign): Add prototypes and libc_hidden_proto resp.
	defines for !_LIBC.
	(freehook): Call __libc_free instead of free.
	(mallochook): Call __libc_malloc instead of malloc.
	(reallochook): Call __libc_realloc instead of realloc.
	(memalignhook): Call __libc_memalign instead of memalign.
	(mcheck): Call __libc_malloc and __libc_free instead of
	malloc and free.

--- libc/malloc/mtrace.c.jj	2004-09-16 11:35:22.000000000 +0200
+++ libc/malloc/mtrace.c	2005-12-13 18:38:03.000000000 +0100
@@ -40,6 +40,18 @@
 # include <libio/iolibio.h>
 # define setvbuf(s, b, f, l) INTUSE(_IO_setvbuf) (s, b, f, l)
 # define fwrite(buf, size, count, fp) _IO_fwrite (buf, size, count, fp)
+extern __typeof (malloc) __libc_malloc;
+extern __typeof (free) __libc_free;
+extern __typeof (realloc) __libc_realloc;
+libc_hidden_proto (__libc_malloc)
+libc_hidden_proto (__libc_realloc)
+libc_hidden_proto (__libc_free)
+libc_hidden_proto (__libc_memalign)
+#else
+# define __libc_malloc(sz) malloc (sz)
+# define __libc_free(ptr) free (ptr)
+# define __libc_realloc(ptr, sz) realloc (ptr, sz)
+# define __libc_memalign(al, sz) memalign (al, sz)
 #endif
 
 #ifndef attribute_hidden
@@ -154,7 +166,7 @@ tr_freehook (ptr, caller)
   if (tr_old_free_hook != NULL)
     (*tr_old_free_hook) (ptr, caller);
   else
-    free (ptr);
+    __libc_free (ptr);
   __free_hook = tr_freehook;
   __libc_lock_unlock (lock);
 }
@@ -173,7 +185,7 @@ tr_mallochook (size, caller)
   if (tr_old_malloc_hook != NULL)
     hdr = (__ptr_t) (*tr_old_malloc_hook) (size, caller);
   else
-    hdr = (__ptr_t) malloc (size);
+    hdr = (__ptr_t) __libc_malloc (size);
   __malloc_hook = tr_mallochook;
 
   tr_where (caller);
@@ -209,7 +221,7 @@ tr_reallochook (ptr, size, caller)
   if (tr_old_realloc_hook != NULL)
     hdr = (__ptr_t) (*tr_old_realloc_hook) (ptr, size, caller);
   else
-    hdr = (__ptr_t) realloc (ptr, size);
+    hdr = (__ptr_t) __libc_realloc (ptr, size);
   __free_hook = tr_freehook;
   __malloc_hook = tr_mallochook;
   __realloc_hook = tr_reallochook;
@@ -251,7 +263,7 @@ tr_memalignhook (alignment, size, caller
   if (tr_old_memalign_hook != NULL)
     hdr = (__ptr_t) (*tr_old_memalign_hook) (alignment, size, caller);
   else
-    hdr = (__ptr_t) memalign (alignment, size);
+    hdr = (__ptr_t) __libc_memalign (alignment, size);
   __memalign_hook = tr_memalignhook;
   __malloc_hook = tr_mallochook;
 
--- libc/malloc/mcheck.c.jj	2005-09-12 09:29:03.000000000 +0200
+++ libc/malloc/mcheck.c	2005-12-19 14:23:44.000000000 +0100
@@ -24,9 +24,25 @@
 # include <mcheck.h>
 # include <stdint.h>
 # include <stdio.h>
+# include <stdlib.h>
 # include <libintl.h>
 #endif
 
+#ifdef _LIBC
+extern __typeof (malloc) __libc_malloc;
+extern __typeof (free) __libc_free;
+extern __typeof (realloc) __libc_realloc;
+libc_hidden_proto (__libc_malloc)
+libc_hidden_proto (__libc_realloc)
+libc_hidden_proto (__libc_free)
+libc_hidden_proto (__libc_memalign)
+#else
+# define __libc_malloc(sz) malloc (sz)
+# define __libc_free(ptr) free (ptr)
+# define __libc_realloc(ptr, sz) realloc (ptr, sz)
+# define __libc_memalign(al, sz) memalign (al, sz)
+#endif
+
 /* Old hook values.  */
 static void (*old_free_hook) (__ptr_t ptr, __const __ptr_t);
 static __ptr_t (*old_malloc_hook) (__malloc_size_t size, const __ptr_t);
@@ -197,7 +213,7 @@ freehook (__ptr_t ptr, const __ptr_t cal
   if (old_free_hook != NULL)
     (*old_free_hook) (ptr, caller);
   else
-    free (ptr);
+    __libc_free (ptr);
   __free_hook = freehook;
 }
 
@@ -214,7 +230,7 @@ mallochook (__malloc_size_t size, const 
     hdr = (struct hdr *) (*old_malloc_hook) (sizeof (struct hdr) + size + 1,
 					     caller);
   else
-    hdr = (struct hdr *) malloc (sizeof (struct hdr) + size + 1);
+    hdr = (struct hdr *) __libc_malloc (sizeof (struct hdr) + size + 1);
   __malloc_hook = mallochook;
   if (hdr == NULL)
     return NULL;
@@ -245,7 +261,7 @@ memalignhook (__malloc_size_t alignment,
   if (old_memalign_hook != NULL)
     block = (*old_memalign_hook) (alignment, slop + size + 1, caller);
   else
-    block = memalign (alignment, slop + size + 1);
+    block = __libc_memalign (alignment, slop + size + 1);
   __memalign_hook = memalignhook;
   if (block == NULL)
     return NULL;
@@ -294,8 +310,8 @@ reallochook (__ptr_t ptr, __malloc_size_
 					      sizeof (struct hdr) + size + 1,
 					      caller);
   else
-    hdr = (struct hdr *) realloc ((__ptr_t) hdr,
-				  sizeof (struct hdr) + size + 1);
+    hdr = (struct hdr *) __libc_realloc ((__ptr_t) hdr,
+					 sizeof (struct hdr) + size + 1);
   __free_hook = freehook;
   __malloc_hook = mallochook;
   __memalign_hook = memalignhook;
@@ -355,8 +371,8 @@ mcheck (func)
   if (__malloc_initialized <= 0 && !mcheck_used)
     {
       /* We call malloc() once here to ensure it is initialized.  */
-      void *p = malloc (0);
-      free (p);
+      void *p = __libc_malloc (0);
+      __libc_free (p);
 
       old_free_hook = __free_hook;
       __free_hook = freehook;

	Jakub

Re: [PATCH] Fix mtrace vs. mudflap coexistence

[Wolfram Gloger]

Hi,

> When an application linked with alternate malloc implementation that
> internally calls the glibc (next) malloc (such as when built with
> mudflap) is mtraced and the alternate malloc doesn't allow reentrancy,
> the application hangs.

An alternative viewpoint would be that the alternate malloc
implementation which _knows_ it is calling the glibc malloc (and which
_knows_ that itself is not reentrant-safe...) needs to reset the glibc
malloc hooks to zero before calling dlsym (RTLD_NEXT, "malloc") and
friends.

Of course this would lose the ability to mtrace that particular
alternate malloc implementation, however with your patch we
effectively lose the ability to mtrace _any_ alternate malloc
implementation.  I'm unsure which is better.

Regards,
Wolfram.

Re: [PATCH] Fix mtrace vs. mudflap coexistence

[Ulrich Drepper]

Wolfram Gloger wrote:
> Of course this would lose the ability to mtrace that particular
> alternate malloc implementation, however with your patch we
> effectively lose the ability to mtrace _any_ alternate malloc
> implementation.

I agree with this.  But the source of the problem is the at least very 
antiquated hook mechanism.  It's on my list to replace.  We need 
something which is thread safe and can be used in situations like this.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖