From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 652D33836035 for ; Thu, 1 Jul 2021 21:53:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 652D33836035 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-540-WRqchVRiOFK5tYj_-ia7zg-1; Thu, 01 Jul 2021 17:53:11 -0400 X-MC-Unique: WRqchVRiOFK5tYj_-ia7zg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 237AF1084F58; Thu, 1 Jul 2021 21:53:11 +0000 (UTC) Received: from greed.delorie.com (ovpn-113-234.rdu2.redhat.com [10.10.113.234]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 146692C175; Thu, 1 Jul 2021 21:53:03 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.15.2/8.15.2) with ESMTP id 161Lr26Z607471; Thu, 1 Jul 2021 17:53:02 -0400 From: DJ Delorie To: Siddhesh Poyarekar Cc: libc-alpha@sourceware.org, carlos@redhat.com, fweimer@redhat.com Subject: Re: [PATCH v2 01/10] mtrace: Deprecate mallwatch and tr_break In-Reply-To: <20210630151921.2752628-2-siddhesh@sourceware.org> Date: Thu, 01 Jul 2021 17:53:02 -0400 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Jul 2021 21:53:14 -0000 Siddhesh Poyarekar writes: > The variable and function pair appear to provide a way for users to > set conditional breakpoints in mtrace when a specific address is > returned by the allocator. This can be achieved by using conditional > breakpoints in gdb so it is redundant. There is no documentation of > this interface in the manual either, so it appears to have been a hack > that got added to debug malloc. Deprecate these symbols and do not > call tr_break anymore. > --- > NEWS | 4 ++++ > malloc/mtrace.c | 57 +++++++++++++++++-------------------------------- > 2 files changed, 24 insertions(+), 37 deletions(-) > > diff --git a/NEWS b/NEWS > index 60933bd975..8e72946c3f 100644 > --- a/NEWS > +++ b/NEWS > @@ -93,6 +93,10 @@ Deprecated and removed features, and other changes affecting compatibility: > package managers that delete removed files late during the package > upgrade or downgrade process. > > +* The symbols mallwatch and tr_break are now deprecated and no longer used in > + mtrace. Similar functionality can be achieved by using conditional > + breakpoints within mtrace functions from within gdb. Ok. > diff --git a/malloc/mtrace.c b/malloc/mtrace.c > index b65b21a933..6c2c58b706 100644 > --- a/malloc/mtrace.c > +++ b/malloc/mtrace.c > @@ -50,8 +50,25 @@ static char *malloc_trace_buffer; > > __libc_lock_define_initialized (static, lock); > > -/* Address to breakpoint on accesses to... */ > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_34) > +/* Compatibility symbols that were introduced to help break at allocation sites > + for specific memory allocations. This is unusable with ASLR, although gdb > + may allow predictable allocation addresses. Even then, gdb has watchpoint > + and conditional breakpoint support which should provide the same > + functionality without having this kludge. These symbols are preserved in > + case some applications ended up linking against them but they don't actually > + do anything anymore; not that they did much before anyway. */ > + > void *mallwatch; > +compat_symbol (libc, mallwatch, mallwatch, GLIBC_2_0); > + > +void > +tr_break (void) > +{ > +} > +compat_symbol (libc, tr_break, tr_break, GLIBC_2_0); > +#endif > + Ok. > @@ -61,19 +78,6 @@ static void *(*tr_old_realloc_hook) (void *ptr, size_t size, > static void *(*tr_old_memalign_hook) (size_t __alignment, size_t __size, > const void *); > > -/* This function is called when the block being alloc'd, realloc'd, or > - freed has an address matching the variable "mallwatch". In a debugger, > - set "mallwatch" to the address of interest, then put a breakpoint on > - tr_break. */ > - > -extern void tr_break (void) __THROW; > -libc_hidden_proto (tr_break) > -void > -tr_break (void) > -{ > -} > -libc_hidden_def (tr_break) > - Ok. > @@ -167,12 +171,6 @@ tr_freehook (void *ptr, const void *caller) > tr_where (caller, info); > /* Be sure to print it first. */ > fprintf (mallstream, "- %p\n", ptr); > - if (ptr == mallwatch) > - { > - __libc_lock_unlock (lock); > - tr_break (); > - __libc_lock_lock (lock); > - } Ok. > @@ -203,9 +201,6 @@ tr_mallochook (size_t size, const void *caller) > > __libc_lock_unlock (lock); > > - if (hdr == mallwatch) > - tr_break (); > - Ok. > @@ -214,9 +209,6 @@ tr_reallochook (void *ptr, size_t size, const void *caller) > { > void *hdr; > > - if (ptr == mallwatch) > - tr_break (); > - Ok. > @@ -247,9 +239,6 @@ tr_reallochook (void *ptr, size_t size, const void *caller) > > __libc_lock_unlock (lock); > > - if (hdr == mallwatch) > - tr_break (); > - Ok. > - if (hdr == mallwatch) > - tr_break (); > - Ok. > -/* We enable tracing if either the environment variable MALLOC_TRACE > - is set, or if the variable mallwatch has been patched to an address > - that the debugging user wants us to stop on. When patching mallwatch, > - don't forget to set a breakpoint on tr_break! */ > +/* We enable tracing if the environment variable MALLOC_TRACE is set. */ > > void > mtrace (void) > @@ -321,7 +304,7 @@ mtrace (void) > #else > mallfile = getenv (mallenv); > #endif > - if (mallfile != NULL || mallwatch != NULL) > + if (mallfile != NULL) > { > char *mtb = malloc (TRACE_BUFFER_SIZE); > if (mtb == NULL) Ok. LGTM. Reviewed-by: DJ Delorie