Message ID | 864d75fe-ce65-ba8b-dc9e-b93804ce5e22@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2] malloc: Set and reset all hooks for tracing (Bug 16573) | expand |
Hello Carlos Thank you for looking at this old issue! :-) It has been a while, but the patch looks reasonable to me too. Regards Kwok Yeung On 09/04/2019 1:52 am, Carlos O'Donell wrote: > DJ noticed I accidentally cargo-culted the old __ptr_t usage and should > just use void * as we use today. The code is still not MT-safe, but we > won't get there until we finish the hook deprecation followed by the > new preloadable tracer replacement. > > Still build and passes on x86_64. > > v2 > - Remove the old __ptr_t usage and replace with newer void * usage > > 8< --- 8< --- 8< > If an error occurs during the tracing operation, particularly during a > call to lock_and_info() which calls _dl_addr, we may end up calling back > into the malloc-subsystem and relock the loader lock and deadlock. For > all intents and purposes the call to _dl_addr can call any of the malloc > family API functions and so we should disable all tracing before calling > such loader functions. This is similar to the strategy that the new > malloc tracer takes when calling the real malloc, namely that all > tracing ceases at the boundary to the real function and any faults at > that point are the purvue of the library (though the new tracer does > this on a per-thread basis in an MT-safe fashion). Since the new tracer > and the hook deprecation are not yet complete we must fix these issues > where we can. > > Tested on x86_64 with no regressions. > > Co-authored-by: Kwok Cheung Yeung <kcy@codesourcery.com> > --- > ChangeLog | 15 +++++++++++ > malloc/mtrace.c | 72 +++++++++++++++++++++++++++++++------------------ > 2 files changed, 61 insertions(+), 26 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 6b7f19a1f3..786bb52400 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,18 @@ > +2019-04-08 Carlos O'Donell <carlos@redhat.com> > + Kwok Cheung Yeung <kcy@codesourcery.com> > + > + [BZ #16573] > + * malloc/mtrace.c: Define prototypes for all hooks. > + (set_default_hooks): New function. > + (set_trace_hooks): Likewise. > + (save_default_hooks): Likewise. > + (tr_freehook): Use new s*_hooks functions. > + (tr_mallochook): Likewise. > + (tr_reallochook): Likewise. > + (tr_memalignhook): Likewise. > + (mtrace): Likewise. > + (muntrace): Likewise. > + > 2019-04-08 Florian Weimer <fweimer@redhat.com> > > * resolv/resolv.h (RES_INSECURE1, RES_INSECURE2): Remove > diff --git a/malloc/mtrace.c b/malloc/mtrace.c > index a2facf65ea..2fda262508 100644 > --- a/malloc/mtrace.c > +++ b/malloc/mtrace.c > @@ -121,6 +121,41 @@ lock_and_info (const void *caller, Dl_info *mem) > return res; > } > > +static void tr_freehook (void *, const void *); > +static void * tr_mallochook (size_t, const void *); > +static void * tr_reallochook (void *, size_t, const void *); > +static void * tr_memalignhook (size_t, size_t, const void *); > + > +/* Set all the default non-trace hooks. */ > +static __always_inline void > +set_default_hooks (void) > +{ > + __free_hook = tr_old_free_hook; > + __malloc_hook = tr_old_malloc_hook; > + __realloc_hook = tr_old_realloc_hook; > + __memalign_hook = tr_old_memalign_hook; > +} > + > +/* Set all of the tracing hooks used for mtrace. */ > +static __always_inline void > +set_trace_hooks (void) > +{ > + __free_hook = tr_freehook; > + __malloc_hook = tr_mallochook; > + __realloc_hook = tr_reallochook; > + __memalign_hook = tr_memalignhook; > +} > + > +/* Save the current set of hooks as the default hooks. */ > +static __always_inline void > +save_default_hooks (void) > +{ > + tr_old_free_hook = __free_hook; > + tr_old_malloc_hook = __malloc_hook; > + tr_old_realloc_hook = __realloc_hook; > + tr_old_memalign_hook = __memalign_hook; > +} > + > static void > tr_freehook (void *ptr, const void *caller) > { > @@ -138,12 +173,12 @@ tr_freehook (void *ptr, const void *caller) > tr_break (); > __libc_lock_lock (lock); > } > - __free_hook = tr_old_free_hook; > + set_default_hooks (); > if (tr_old_free_hook != NULL) > (*tr_old_free_hook)(ptr, caller); > else > free (ptr); > - __free_hook = tr_freehook; > + set_trace_hooks (); > __libc_lock_unlock (lock); > } > > @@ -155,12 +190,12 @@ tr_mallochook (size_t size, const void *caller) > Dl_info mem; > Dl_info *info = lock_and_info (caller, &mem); > > - __malloc_hook = tr_old_malloc_hook; > + set_default_hooks (); > if (tr_old_malloc_hook != NULL) > hdr = (void *) (*tr_old_malloc_hook)(size, caller); > else > hdr = (void *) malloc (size); > - __malloc_hook = tr_mallochook; > + set_trace_hooks (); > > tr_where (caller, info); > /* We could be printing a NULL here; that's OK. */ > @@ -185,16 +220,12 @@ tr_reallochook (void *ptr, size_t size, const void > *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; > + set_default_hooks (); > if (tr_old_realloc_hook != NULL) > hdr = (void *) (*tr_old_realloc_hook)(ptr, size, caller); > else > hdr = (void *) realloc (ptr, size); > - __free_hook = tr_freehook; > - __malloc_hook = tr_mallochook; > - __realloc_hook = tr_reallochook; > + set_trace_hooks (); > > tr_where (caller, info); > if (hdr == NULL) > @@ -230,14 +261,12 @@ tr_memalignhook (size_t alignment, size_t size, > const void *caller) > Dl_info mem; > Dl_info *info = lock_and_info (caller, &mem); > > - __memalign_hook = tr_old_memalign_hook; > - __malloc_hook = tr_old_malloc_hook; > + set_default_hooks (); > if (tr_old_memalign_hook != NULL) > hdr = (void *) (*tr_old_memalign_hook)(alignment, size, caller); > else > hdr = (void *) memalign (alignment, size); > - __memalign_hook = tr_memalignhook; > - __malloc_hook = tr_mallochook; > + set_trace_hooks (); > > tr_where (caller, info); > /* We could be printing a NULL here; that's OK. */ > @@ -305,14 +334,8 @@ mtrace (void) > malloc_trace_buffer = mtb; > setvbuf (mallstream, malloc_trace_buffer, _IOFBF, > TRACE_BUFFER_SIZE); > fprintf (mallstream, "= Start\n"); > - tr_old_free_hook = __free_hook; > - __free_hook = tr_freehook; > - tr_old_malloc_hook = __malloc_hook; > - __malloc_hook = tr_mallochook; > - tr_old_realloc_hook = __realloc_hook; > - __realloc_hook = tr_reallochook; > - tr_old_memalign_hook = __memalign_hook; > - __memalign_hook = tr_memalignhook; > + save_default_hooks (); > + set_trace_hooks (); > #ifdef _LIBC > if (!added_atexit_handler) > { > @@ -338,10 +361,7 @@ muntrace (void) > file. */ > FILE *f = mallstream; > mallstream = NULL; > - __free_hook = tr_old_free_hook; > - __malloc_hook = tr_old_malloc_hook; > - __realloc_hook = tr_old_realloc_hook; > - __memalign_hook = tr_old_memalign_hook; > + set_default_hooks (); > > fprintf (f, "= End\n"); > fclose (f);
diff --git a/ChangeLog b/ChangeLog index 6b7f19a1f3..786bb52400 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2019-04-08 Carlos O'Donell <carlos@redhat.com> + Kwok Cheung Yeung <kcy@codesourcery.com> + + [BZ #16573] + * malloc/mtrace.c: Define prototypes for all hooks. + (set_default_hooks): New function. + (set_trace_hooks): Likewise. + (save_default_hooks): Likewise. + (tr_freehook): Use new s*_hooks functions. + (tr_mallochook): Likewise. + (tr_reallochook): Likewise. + (tr_memalignhook): Likewise. + (mtrace): Likewise. + (muntrace): Likewise. + 2019-04-08 Florian Weimer <fweimer@redhat.com> * resolv/resolv.h (RES_INSECURE1, RES_INSECURE2): Remove diff --git a/malloc/mtrace.c b/malloc/mtrace.c index a2facf65ea..2fda262508 100644 --- a/malloc/mtrace.c +++ b/malloc/mtrace.c @@ -121,6 +121,41 @@ lock_and_info (const void *caller, Dl_info *mem) return res; } +static void tr_freehook (void *, const void *); +static void * tr_mallochook (size_t, const void *); +static void * tr_reallochook (void *, size_t, const void *); +static void * tr_memalignhook (size_t, size_t, const void *); + +/* Set all the default non-trace hooks. */ +static __always_inline void +set_default_hooks (void) +{ + __free_hook = tr_old_free_hook; + __malloc_hook = tr_old_malloc_hook; + __realloc_hook = tr_old_realloc_hook; + __memalign_hook = tr_old_memalign_hook; +} + +/* Set all of the tracing hooks used for mtrace. */ +static __always_inline void +set_trace_hooks (void) +{ + __free_hook = tr_freehook; + __malloc_hook = tr_mallochook; + __realloc_hook = tr_reallochook; + __memalign_hook = tr_memalignhook; +} + +/* Save the current set of hooks as the default hooks. */ +static __always_inline void +save_default_hooks (void) +{ + tr_old_free_hook = __free_hook; + tr_old_malloc_hook = __malloc_hook; + tr_old_realloc_hook = __realloc_hook; + tr_old_memalign_hook = __memalign_hook; +} + static void tr_freehook (void *ptr, const void *caller) { @@ -138,12 +173,12 @@ tr_freehook (void *ptr, const void *caller) tr_break (); __libc_lock_lock (lock); } - __free_hook = tr_old_free_hook; + set_default_hooks (); if (tr_old_free_hook != NULL) (*tr_old_free_hook)(ptr, caller); else free (ptr); - __free_hook = tr_freehook; + set_trace_hooks (); __libc_lock_unlock (lock); } @@ -155,12 +190,12 @@ tr_mallochook (size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __malloc_hook = tr_old_malloc_hook; + set_default_hooks (); if (tr_old_malloc_hook != NULL) hdr = (void *) (*tr_old_malloc_hook)(size, caller); else hdr = (void *) malloc (size); - __malloc_hook = tr_mallochook; + set_trace_hooks (); tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -185,16 +220,12 @@ tr_reallochook (void *ptr, size_t size, const void *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; + set_default_hooks (); if (tr_old_realloc_hook != NULL) hdr = (void *) (*tr_old_realloc_hook)(ptr, size, caller); else hdr = (void *) realloc (ptr, size); - __free_hook = tr_freehook; - __malloc_hook = tr_mallochook; - __realloc_hook = tr_reallochook; + set_trace_hooks (); tr_where (caller, info); if (hdr == NULL) @@ -230,14 +261,12 @@ tr_memalignhook (size_t alignment, size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __memalign_hook = tr_old_memalign_hook; - __malloc_hook = tr_old_malloc_hook; + set_default_hooks (); if (tr_old_memalign_hook != NULL) hdr = (void *) (*tr_old_memalign_hook)(alignment, size, caller); else hdr = (void *) memalign (alignment, size); - __memalign_hook = tr_memalignhook; - __malloc_hook = tr_mallochook; + set_trace_hooks (); tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -305,14 +334,8 @@ mtrace (void) malloc_trace_buffer = mtb; setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE); fprintf (mallstream, "= Start\n"); - tr_old_free_hook = __free_hook; - __free_hook = tr_freehook; - tr_old_malloc_hook = __malloc_hook; - __malloc_hook = tr_mallochook; - tr_old_realloc_hook = __realloc_hook; - __realloc_hook = tr_reallochook; - tr_old_memalign_hook = __memalign_hook; - __memalign_hook = tr_memalignhook; + save_default_hooks (); + set_trace_hooks (); #ifdef _LIBC if (!added_atexit_handler) { @@ -338,10 +361,7 @@ muntrace (void) file. */ FILE *f = mallstream; mallstream = NULL; - __free_hook = tr_old_free_hook; - __malloc_hook = tr_old_malloc_hook; - __realloc_hook = tr_old_realloc_hook; - __memalign_hook = tr_old_memalign_hook; + set_default_hooks (); fprintf (f, "= End\n"); fclose (f);