Message ID | 53E5290E.1000907@codesourcery.com |
---|---|
State | New |
Headers | show |
On 8/8/14 3:46 PM, Kwok Cheung Yeung wrote: > A hang can occur if the mtrace output is diverted to a file (by > setting the MALLOC_TRACE environment variable) and an mtrace problem > is found. The mtrace hook leads to a call of __libc_message(), which > eventually results in a call to malloc(), causing another mtrace hook > to be called. This hook then attempts to reacquire the same lock > that was already claimed by the first hook, resulting in an infinite > hang. > > This patch fixes this by making all mtrace hooks temporarily > uninstall all mtrace hooks for the duration of the call, so that > there will not be another call to an mtrace hook further down the > call chain. I'm cleaning up the queue of old malloc patches. I'm proposing a similar patch but refactored slightly. Thank you for waiting almost 5 years :} > 2014-08-08 Kwok Cheung Yeung <kcy@codesourcery.com> > > [BZ #16573] > * malloc/mtrace.c (tr_freehook): Move function prototype to > before the hook definitions. Disable all hooks before calling > the caller or old hook, and re-enable all hooks afterwards. > (tr_mallochook): Likewise. > (tr_reallochook): Likewise. > (tr_freehook): Likewise. > --- > diff --git a/malloc/mtrace.c b/malloc/mtrace.c > index 91e5710..593b212 100644 > --- a/malloc/mtrace.c > +++ b/malloc/mtrace.c > @@ -120,6 +120,10 @@ lock_and_info (const __ptr_t caller, Dl_info *mem) > return res; > } > > +static __ptr_t tr_mallochook (size_t, const __ptr_t); > +static __ptr_t tr_reallochook (__ptr_t, size_t, const __ptr_t); > +static __ptr_t tr_memalignhook (size_t, size_t, const __ptr_t); > + > static void > tr_freehook (__ptr_t ptr, const __ptr_t caller) > { > @@ -138,11 +142,17 @@ tr_freehook (__ptr_t ptr, const __ptr_t caller) > __libc_lock_lock (lock); > } > __free_hook = tr_old_free_hook; > + __malloc_hook = tr_old_malloc_hook; > + __realloc_hook = tr_old_realloc_hook; > + __memalign_hook = tr_old_memalign_hook; > if (tr_old_free_hook != NULL) > (*tr_old_free_hook)(ptr, caller); > else > free (ptr); > __free_hook = tr_freehook; > + __malloc_hook = tr_mallochook; > + __realloc_hook = tr_reallochook; > + __memalign_hook = tr_memalignhook; > __libc_lock_unlock (lock); > } > > @@ -154,12 +164,18 @@ tr_mallochook (size_t size, const __ptr_t caller) > Dl_info mem; > Dl_info *info = lock_and_info (caller, &mem); > > + __free_hook = tr_old_free_hook; > __malloc_hook = tr_old_malloc_hook; > + __realloc_hook = tr_old_realloc_hook; > + __memalign_hook = tr_old_memalign_hook; > if (tr_old_malloc_hook != NULL) > hdr = (__ptr_t) (*tr_old_malloc_hook)(size, caller); > else > hdr = (__ptr_t) malloc (size); > + __free_hook = tr_freehook; > __malloc_hook = tr_mallochook; > + __realloc_hook = tr_reallochook; > + __memalign_hook = tr_memalignhook; > > tr_where (caller, info); > /* We could be printing a NULL here; that's OK. */ > @@ -187,6 +203,7 @@ tr_reallochook (__ptr_t ptr, size_t size, const __ptr_t caller) > __free_hook = tr_old_free_hook; > __malloc_hook = tr_old_malloc_hook; > __realloc_hook = tr_old_realloc_hook; > + __memalign_hook = tr_old_memalign_hook; > if (tr_old_realloc_hook != NULL) > hdr = (__ptr_t) (*tr_old_realloc_hook)(ptr, size, caller); > else > @@ -194,6 +211,7 @@ tr_reallochook (__ptr_t ptr, size_t size, const __ptr_t caller) > __free_hook = tr_freehook; > __malloc_hook = tr_mallochook; > __realloc_hook = tr_reallochook; > + __memalign_hook = tr_memalignhook; > > tr_where (caller, info); > if (hdr == NULL) > @@ -229,14 +247,18 @@ tr_memalignhook (size_t alignment, size_t size, const __ptr_t caller) > Dl_info mem; > Dl_info *info = lock_and_info (caller, &mem); > > - __memalign_hook = tr_old_memalign_hook; > + __free_hook = tr_old_free_hook; > __malloc_hook = tr_old_malloc_hook; > + __realloc_hook = tr_old_realloc_hook; > + __memalign_hook = tr_old_memalign_hook; > if (tr_old_memalign_hook != NULL) > hdr = (__ptr_t) (*tr_old_memalign_hook)(alignment, size, caller); > else > hdr = (__ptr_t) memalign (alignment, size); > - __memalign_hook = tr_memalignhook; > + __free_hook = tr_freehook; > __malloc_hook = tr_mallochook; > + __realloc_hook = tr_reallochook; > + __memalign_hook = tr_memalignhook; > > tr_where (caller, info); > /* We could be printing a NULL here; that's OK. */
diff --git a/malloc/mtrace.c b/malloc/mtrace.c index 91e5710..593b212 100644 --- a/malloc/mtrace.c +++ b/malloc/mtrace.c @@ -120,6 +120,10 @@ lock_and_info (const __ptr_t caller, Dl_info *mem) return res; } +static __ptr_t tr_mallochook (size_t, const __ptr_t); +static __ptr_t tr_reallochook (__ptr_t, size_t, const __ptr_t); +static __ptr_t tr_memalignhook (size_t, size_t, const __ptr_t); + static void tr_freehook (__ptr_t ptr, const __ptr_t caller) { @@ -138,11 +142,17 @@ tr_freehook (__ptr_t ptr, const __ptr_t caller) __libc_lock_lock (lock); } __free_hook = tr_old_free_hook; + __malloc_hook = tr_old_malloc_hook; + __realloc_hook = tr_old_realloc_hook; + __memalign_hook = tr_old_memalign_hook; if (tr_old_free_hook != NULL) (*tr_old_free_hook)(ptr, caller); else free (ptr); __free_hook = tr_freehook; + __malloc_hook = tr_mallochook; + __realloc_hook = tr_reallochook; + __memalign_hook = tr_memalignhook; __libc_lock_unlock (lock); } @@ -154,12 +164,18 @@ tr_mallochook (size_t size, const __ptr_t caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); + __free_hook = tr_old_free_hook; __malloc_hook = tr_old_malloc_hook; + __realloc_hook = tr_old_realloc_hook; + __memalign_hook = tr_old_memalign_hook; if (tr_old_malloc_hook != NULL) hdr = (__ptr_t) (*tr_old_malloc_hook)(size, caller); else hdr = (__ptr_t) malloc (size); + __free_hook = tr_freehook; __malloc_hook = tr_mallochook; + __realloc_hook = tr_reallochook; + __memalign_hook = tr_memalignhook; tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -187,6 +203,7 @@ tr_reallochook (__ptr_t ptr, size_t size, const __ptr_t caller) __free_hook = tr_old_free_hook; __malloc_hook = tr_old_malloc_hook; __realloc_hook = tr_old_realloc_hook; + __memalign_hook = tr_old_memalign_hook; if (tr_old_realloc_hook != NULL) hdr = (__ptr_t) (*tr_old_realloc_hook)(ptr, size, caller);