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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 8B8C23857C65 for ; Thu, 17 Dec 2020 04:18:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8B8C23857C65 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-146-KnXBe02zNdu8fhpc4EuN5A-1; Wed, 16 Dec 2020 23:18:41 -0500 X-MC-Unique: KnXBe02zNdu8fhpc4EuN5A-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A5DC1107ACE3; Thu, 17 Dec 2020 04:18:40 +0000 (UTC) Received: from greed.delorie.com (ovpn-113-151.rdu2.redhat.com [10.10.113.151]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7BC1C5D9C0; Thu, 17 Dec 2020 04:18:40 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.14.7/8.14.7) with ESMTP id 0BH4IdZE009594; Wed, 16 Dec 2020 23:18:39 -0500 From: DJ Delorie To: Adder Cc: libc-alpha@sourceware.org, fweimer@redhat.com Subject: Re: [PATCH] malloc tcache: Debugger now sees the address of the corrupted chunk. In-Reply-To: (message from Adder on Thu, 17 Dec 2020 04:08:30 +0200) Date: Wed, 16 Dec 2020 23:18:39 -0500 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 17 Dec 2020 04:18:44 -0000 Adder writes: > But perhaps the speed cost is lower than it appears. > I have always been bad at evaluating time. > I am going to think about a way to benchmark this. > Suggestions are warmly welcome. https://developers.redhat.com/blog/2016/03/11/practical-micro-benchmarking-with-ltrace-and-sched/ > In the meanwhile, could we also consider the following alternative ? > > if (e_next) > { > if (__glibc_unlikely (e_next->key != tcache)) > please_crash_in_a_way_which_allows_debugger_to_print_e (); > } > > Advantages (YMMV): > > - no (volatile T *), no (void), no other cast; > - easier to understand (even without "corrupted pointer" in mind); > - only loads, no stores; > - probability of e_next accidentally being valid and having a good key is low. Note that it's checking the next ptr in one chunk, and the key in a *different* chunk. I think that's OK, corruption can happen anywhere, except that the please_crash would need to decide which 'e' is the relevent one for the message it's printing. But I think we need to ask the larger glibc group if there's a preferred idiom for "make this variable available to the debugger" > Disadvantages: > > - conditional jump; That's what __glibc_unlikely() is for - it tells gcc to optimize the conditional for the common path, often negating the costs of the jump completely. >> This is where malloc_printerr should be called. Even if the data is >> corrupt, this is what we do elsewhere in these cases. > > Purpose is to give "e" to the debugger (including the human debugger). > In my testing (on x86_64), adding malloc_printerr here loses "e". > > For clarity and consistence with usage of malloc_printerr elsewhere, > I wish to suggest adding a function malloc_printerr_4 > which is given the pointer to the string and 4 additional uint64_t args. I was going to suggest passing a chunk_ptr to malloc_printerr as that's what we usually have (or pass NULL if we don't). We have macros to convert tcache pointers to chunk pointers.