From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by sourceware.org (Postfix) with ESMTPS id 5233A3858C51 for ; Mon, 2 May 2022 21:20:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5233A3858C51 Received: by mail-pj1-x1034.google.com with SMTP id j8-20020a17090a060800b001cd4fb60dccso434249pjj.2 for ; Mon, 02 May 2022 14:20:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=IDWlO+Ff+tkcDpjC4ez7uYLw3cup7mmaEbynfrxJJZI=; b=NvgVSp6b8cn1GegaYt+PIYPSF1XFvNEA0GbfeMArEFV0M8o+i5flojnOUJ/NRxrl24 TK+01zpfFAYw3r3Sxkt9e9TMYrUl4cP8X247YLWFkBylhs0eSYUPJK8UMxYwvzgnT9TM SR7HBpLtoq4iwreIag9ILmPgRGnDI4NhH/4hffFq535QxjLC+pBZvVQc/GybqX5FA01I pH6ayfGyYwR7r4a/wu2HDEIq7SqLMiVSfGu4PjvFEi7o6ytbB1jKx45oe/9MTFgMiSjK Ztw7ydKmzP68N9HeZF5JaKCFxk8ZUKJMqV5Qj3/rWNe79zt9pqOWQHeyEdMb3NZ4j7Gf 70oQ== X-Gm-Message-State: AOAM533sx/+FTRDpG9pML/hzM8l0+Ra39l5q804NH8HzAGnWzJe4fmNE ZZ0d8X7ZuVT/Qx8Bv8hLIu77QHWStYtWYQ== X-Google-Smtp-Source: ABdhPJzawfGdHT0LMWdzg2pMbKRnxFPn9Us1LZ+9oBCqFENxb29pSqeFwfl9kKB9z3xKvt2PsXLhzQ== X-Received: by 2002:a17:90a:5b09:b0:1cd:b3d3:a3f3 with SMTP id o9-20020a17090a5b0900b001cdb3d3a3f3mr1190846pji.9.1651526440686; Mon, 02 May 2022 14:20:40 -0700 (PDT) Received: from google.com ([2620:15c:2ce:200:9b50:a399:e048:3563]) by smtp.gmail.com with ESMTPSA id z19-20020a17090abd9300b001cd4989ff4asm160380pjr.17.2022.05.02.14.20.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 May 2022 14:20:39 -0700 (PDT) Date: Mon, 2 May 2022 14:20:35 -0700 From: Fangrui Song To: Nicholas Guriev Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] elf: Rewrite long RESOLVE_MAP macro to a debug friendly function Message-ID: <20220502212035.45lvk22rjui65pvq@google.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-27.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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, 02 May 2022 21:20:44 -0000 On 2022-05-02, Nicholas Guriev wrote: >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. I think this is useful. Does the size of dl-reloc.os code increase or decrease with the change? >Signed-off-by: Nicholas Guriev >--- > >I was playing with the DT_AUXILIARY flag and run into a crash in the >RESOLVE_MAP macro. Alas, GDB is unable to see what is going on in a >multi-line macro, so I turned it to a static function. Hope this is useful. > > elf/dl-reloc.c | 55 ++++++++++++++++++++++++++++++-------------------- > 1 file changed, 33 insertions(+), 22 deletions(-) > >diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c >index 771a34bd14..88d7fee041 100644 >--- a/elf/dl-reloc.c >+++ b/elf/dl-reloc.c >@@ -162,29 +162,40 @@ _dl_nothread_init_static_tls (struct link_map *map) > } > #endif /* !PTHREAD_IN_LIBC */ > >+static lookup_t >+_dl_resolve_map (lookup_t l, struct r_scope_elem *scope[], const ElfW(Sym) **ref, >+ const struct r_found_version *version, unsigned long 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 >+ { >+ 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); Use `lookup_t _lr = ` ? >+ 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 _dl_resolve_map > > #include "dynamic-link.h" > >-- >2.32.0 >