From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from poodle.tulip.relay.mailchannels.net (poodle.tulip.relay.mailchannels.net [23.83.218.249]) by sourceware.org (Postfix) with ESMTPS id 8670E389942E for ; Mon, 9 May 2022 13:38:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8670E389942E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id D200D801A45; Mon, 9 May 2022 13:38:24 +0000 (UTC) Received: from pdx1-sub0-mail-a305.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id CBB19800851; Mon, 9 May 2022 13:38:23 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1652103503; a=rsa-sha256; cv=none; b=z5sEyDZmYqh7YEKIrvtc89ixMpW5Mh8uquYm4Yk/DPI0py7AUkPiDAHW3BRsUAIDSL80Xs H/64YB0MwTvGSr5tT/jfJyaDvWj7KsFxMktLvoYfOkQcGDJDyU4prmca5YNDMddYmXqu8/ 4zmLhxLOAuqkUqqpJKcU3LfsplyCa/khBii/10r2ykJ6oYyYj7OuCTE9lkRLQe++o2cUUY 34AdHyqHjf4q5cE2cO5/TltHTvG1U8JbHzTVR6NDt3e7LYR8ONqq6Se/X/Ta9L8nRdrNVK xfN+ul66VQHq2mPZl22y5kjTKU7RsWa/jaT1GN1zytQUJUQ/K+wOCygJQAa2Aw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1652103503; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HAMbouMse13NsXoHyHjpyoTA3gQ/67S61APj/myjylk=; b=rR8yBM6BHxoTzydA5vWML4UALgJKZ8Yrxli4tn6wzq3s1z+r2pZs7upv5GamCLP2AKQqWq RdAU3vMtyDFUYoWaFGk8NAnU3L+oEuOOoAsVTL2kD7WQmNA98RQglSrTZ/irWsGV9Hwr/w aGdSnNRaPEGJa5NWN6y5IAHJkB/M7ATBta3aBEeYJJt25QzDi6dJzHT+lMfdmERXS14x2v fbxuoWiF20WYAz98NEN5xbu3exjNQH8eB5QfNKLpvP6RYxLTpxyr3+jWReX0KmJP/tyH6r 8/j5ISaBeweQUGj2ybTfc0yhSVQht0E5Dyr3k2IwrHHyrMDbme1u2EeZs0aA0Q== ARC-Authentication-Results: i=1; rspamd-847cd55849-twkll; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Plucky-Thoughtful: 3576344a5f0e7992_1652103504346_654703810 X-MC-Loop-Signature: 1652103504345:1085890883 X-MC-Ingress-Time: 1652103504345 Received: from pdx1-sub0-mail-a305.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.112.55.199 (trex/6.7.1); Mon, 09 May 2022 13:38:24 +0000 Received: from [192.168.1.174] (unknown [1.186.121.104]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a305.dreamhost.com (Postfix) with ESMTPSA id 4Kxj1B1bzwz1K3; Mon, 9 May 2022 06:38:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1652103503; bh=HAMbouMse13NsXoHyHjpyoTA3gQ/67S61APj/myjylk=; h=Date:Subject:To:Cc:From:Content-Type:Content-Transfer-Encoding; b=GcXJMw1/V04XbAJjdcqer0pa2JU8mCMgnbKlnJfp0qpZ0R5nwnBxuu1G57EFLWVB3 6vtbF/0Db4uxlyHPlnvrSkXkbGoA/RcDire6V7lsj1iyCisRZNDQdRS6wLR+tW/FZO jgUJNgS+fLeutYJIEDutmo7fi7C3XPI5jnDs0IsqcaiG9LbX9bwC2rLzK8Gip8wKMu n1F5ElelUrY4ylRKrJr6gxvi5m6CdzqzAltC+HrAeQ21A8g+qTHO0P3Hz69R7vDc8A 1fOSmYr3V6H7X7ZRibh9sJZf/DAYkh9b7WYlKjM2g5oA8EkTmr1u4pUCA08BPDMAmY phs4NGUW4YXXQ== Message-ID: Date: Mon, 9 May 2022 19:08:16 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH v3] elf: Rewrite long RESOLVE_MAP macro to a debug friendly function Content-Language: en-US To: Nicholas Guriev , =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Cc: libc-alpha@sourceware.org References: From: Siddhesh Poyarekar In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3038.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_SBL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Mon, 09 May 2022 13:38:30 -0000 On 07/05/2022 19:03, Nicholas Guriev wrote: > On Вт, 2022-05-03 at 13:12 -0700, Fāng-ruì Sòng wrote: >> The function has internal linkage (static) and it seems that the >> prevailing style doesn't use `_dl_` prefix for such functions. > > Ok, I renamed the function by removing the prefix. I also added keyword > `inline` so that GCC could really inline this new function. GCC relies > on `--param max-inline-insns-auto` (which defaults to 15) when the > compiler decides to inline ordinary function, and `--param max-inline- > insns-single` (70 by default) is used for functions with the keyword. (Not a full review, leaving that for someone else; just a nit I noticed) Please use __always_inline instead, which adds the inline keyword as well as __attribute__ ((inline)). That way gcc ensures that the function is *always* inlined. Thanks, Siddhesh > -- >8 -- > > A static function that may be inlined is way better to find where exactly a > crash happens. So one can step into the function with GDB. The macro still > remains for compatibility with other code. > > Signed-off-by: Nicholas Guriev > --- > elf/dl-reloc.c | 56 ++++++++++++++++++++++++++++++-------------------- > 1 file changed, 34 insertions(+), 22 deletions(-) > > diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c > index 771a34bd14..10affc4d48 100644 > --- a/elf/dl-reloc.c > +++ b/elf/dl-reloc.c > @@ -162,29 +162,41 @@ _dl_nothread_init_static_tls (struct link_map *map) > } > #endif /* !PTHREAD_IN_LIBC */ > > +static inline lookup_t > +resolve_map (lookup_t l, struct r_scope_elem *scope[], const ElfW(Sym) **ref, > + const struct r_found_version *version, unsigned long int r_type) > +{ > + if (ELFW(ST_BIND) ((*ref)->st_info) == STB_LOCAL > + || __glibc_unlikely (dl_symbol_visibility_binds_local_p (*ref))) > + return l; > + > + if (__glibc_unlikely (*ref == l->l_lookup_cache.sym) > + && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class) > + { > + bump_num_cache_relocations (); > + *ref = l->l_lookup_cache.ret; > + } > + else > + { > + int tc = elf_machine_type_class (r_type); > + l->l_lookup_cache.type_class = tc; > + l->l_lookup_cache.sym = *ref; > + const char *undef_name > + = (const char *) D_PTR (l, l_info[DT_STRTAB]) + (*ref)->st_name; > + const struct r_found_version *v = NULL; > + if (version != NULL && version->hash != 0) > + v = version; > + lookup_t lr = _dl_lookup_symbol_x (undef_name, l, ref, scope, v, tc, > + DL_LOOKUP_ADD_DEPENDENCY > + | DL_LOOKUP_FOR_RELOCATE, NULL); > + l->l_lookup_cache.ret = *ref; > + l->l_lookup_cache.value = lr; > + } > + return l->l_lookup_cache.value; > +} > + > /* This macro is used as a callback from the ELF_DYNAMIC_RELOCATE code. */ > -#define RESOLVE_MAP(l, scope, ref, version, r_type) \ > - ((ELFW(ST_BIND) ((*ref)->st_info) != STB_LOCAL \ > - && __glibc_likely (!dl_symbol_visibility_binds_local_p (*ref))) \ > - ? ((__glibc_unlikely ((*ref) == l->l_lookup_cache.sym) \ > - && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class) \ > - ? (bump_num_cache_relocations (), \ > - (*ref) = l->l_lookup_cache.ret, \ > - l->l_lookup_cache.value) \ > - : ({ lookup_t _lr; \ > - int _tc = elf_machine_type_class (r_type); \ > - l->l_lookup_cache.type_class = _tc; \ > - l->l_lookup_cache.sym = (*ref); \ > - const struct r_found_version *v = NULL; \ > - if ((version) != NULL && (version)->hash != 0) \ > - v = (version); \ > - _lr = _dl_lookup_symbol_x ((const char *) D_PTR (l, l_info[DT_STRTAB]) + (*ref)->st_name, \ > - l, (ref), scope, v, _tc, \ > - DL_LOOKUP_ADD_DEPENDENCY \ > - | DL_LOOKUP_FOR_RELOCATE, NULL); \ > - l->l_lookup_cache.ret = (*ref); \ > - l->l_lookup_cache.value = _lr; })) \ > - : l) > +#define RESOLVE_MAP resolve_map > > #include "dynamic-link.h" >