From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x30.google.com (mail-oa1-x30.google.com [IPv6:2001:4860:4864:20::30]) by sourceware.org (Postfix) with ESMTPS id EA2063858D1E for ; Wed, 20 Apr 2022 18:33:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EA2063858D1E Received: by mail-oa1-x30.google.com with SMTP id 586e51a60fabf-d6e29fb3d7so2874246fac.7 for ; Wed, 20 Apr 2022 11:33:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=pN0zl3+LNNlpLu0Hah4lQyIVptWmjeVSqN4GwXeWLsw=; b=4v2QXH/KoQEZIDIw4AB3ktrr4PjWJr2NJGSHIrahUWyP512+uoARd4dY0lVqqYwAt4 zWlTJh9A2VKjevDi5nFifpLKp1en2P8whXw+A1c0G9s3CEUtmhw/JRbGvM8GD0mprX13 xULDKxZtCmmLZ0Wte/+Qhi0kiLOHyMOV92yrLadl5djKcHgyGH3e8szbwhEr9fE3O+Ry DYy0xp4ORa0Qn5LLbMvJWMdpRdj+54dfoRJoQ7Cb6DZ30fmBEtuxPYwaVe2DZ6vIWweH lqUtXRWwc6Eq8GBqmOWSCorOPTsIrcbpQqrAnCzgtWP9zLfbS0jUHYgkrKn0Fn1Yj31i 4U9g== X-Gm-Message-State: AOAM5318eDgx0ReQDxm6Cc1UMfNq+CV30p0RDEO1bqQwKnjm3eO9hxei 9ExsQK1SJJLXlhhPYz+Qh5JrZ8BPPTql9w== X-Google-Smtp-Source: ABdhPJy1cGhn/gSWGPVuKDm/h/H49redhrx/cUxgqJ9YMbRKZKOoyJhA7WUmccpDHpC0b9rydnOypQ== X-Received: by 2002:a05:6870:3046:b0:e5:a6e5:7193 with SMTP id u6-20020a056870304600b000e5a6e57193mr2315596oau.193.1650479624338; Wed, 20 Apr 2022 11:33:44 -0700 (PDT) Received: from ?IPV6:2804:431:c7ca:c9d0:24b1:bd98:2ef4:714c? ([2804:431:c7ca:c9d0:24b1:bd98:2ef4:714c]) by smtp.gmail.com with ESMTPSA id f24-20020a9d5e98000000b006055789d903sm1978172otl.16.2022.04.20.11.33.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Apr 2022 11:33:43 -0700 (PDT) Message-ID: <9a11cca2-9914-7434-42f5-2c0a47009463@linaro.org> Date: Wed, 20 Apr 2022 15:33:41 -0300 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] elf: Move elf_dynamic_do_Rel RTLD_BOOTSTRAP branches outside Content-Language: en-US To: Fangrui Song , libc-alpha@sourceware.org References: <20220418073711.2471534-1-maskray@google.com> From: Adhemerval Zanella In-Reply-To: <20220418073711.2471534-1-maskray@google.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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: Wed, 20 Apr 2022 18:33:47 -0000 On 18/04/2022 04:37, Fangrui Song via Libc-alpha wrote: > elf_dynamic_do_Rel checks RTLD_BOOTSTRAP in several #ifdef branches. > Create an outside RTLD_BOOTSTRAP branch to simplify reasoning about the > function at the cost of a few duplicate lines. > > Since dl_naudit is zero in RTLD_BOOTSTRAP code, the RTLD_BOOTSTRAP > branch can avoid _dl_audit_symbind calls to save code size (stripped > x86-64 ld.so is 672 bytes smaller). Thanks, it does looks clear. I am seeing a code decrease on all archers, although not a large as you are seeing (x86_64 went from 191222 to 191094). Reviewed-by: Adhemerval Zanella > --- > elf/do-rel.h | 49 ++++++++++++++++++++++++++++--------------------- > 1 file changed, 28 insertions(+), 21 deletions(-) > > diff --git a/elf/do-rel.h b/elf/do-rel.h > index d3e022267b..27b9b9f427 100644 > --- a/elf/do-rel.h > +++ b/elf/do-rel.h > @@ -45,15 +45,35 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > __typeof (((ElfW(Dyn) *) 0)->d_un.d_val) nrelative, > int lazy, int skip_ifunc) > { > - const ElfW(Rel) *r = (const void *) reladdr; > + const ElfW(Rel) *relative = (const void *) reladdr; > + const ElfW(Rel) *r = relative + nrelative; > const ElfW(Rel) *end = (const void *) (reladdr + relsize); > ElfW(Addr) l_addr = map->l_addr; > -# if defined ELF_MACHINE_IRELATIVE && !defined RTLD_BOOTSTRAP > + const ElfW(Sym) *const symtab > + = (const void *) D_PTR (map, l_info[DT_SYMTAB]); > + > +#ifdef RTLD_BOOTSTRAP > + for (; relative < r; ++relative) > + DO_ELF_MACHINE_REL_RELATIVE (map, l_addr, relative); > + > + const ElfW (Half) *const version > + = (const void *) D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > + for (; r < end; ++r) > + { > + ElfW (Half) ndx = version[ELFW (R_SYM) (r->r_info)] & 0x7fff; > + const ElfW (Sym) *sym = &symtab[ELFW (R_SYM) (r->r_info)]; > + void *const r_addr_arg = (void *) (l_addr + r->r_offset); > + const struct r_found_version *rversion = &map->l_versions[ndx]; > + > + elf_machine_rel (map, scope, r, sym, rversion, r_addr_arg, skip_ifunc); > + } > +#else /* !RTLD_BOOTSTRAP */ > +# if defined ELF_MACHINE_IRELATIVE > const ElfW(Rel) *r2 = NULL; > const ElfW(Rel) *end2 = NULL; > # endif > > -#if (!defined DO_RELA || !defined ELF_MACHINE_PLT_REL) && !defined RTLD_BOOTSTRAP > +#if !defined DO_RELA || !defined ELF_MACHINE_PLT_REL > /* We never bind lazily during ld.so bootstrap. Unfortunately gcc is > not clever enough to see through all the function calls to realize > that. */ > @@ -82,12 +102,6 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > else > #endif > { > - const ElfW(Sym) *const symtab = > - (const void *) D_PTR (map, l_info[DT_SYMTAB]); > - const ElfW(Rel) *relative = r; > - r += nrelative; > - > -#ifndef RTLD_BOOTSTRAP > /* This is defined in rtld.c, but nowhere in the static libc.a; make > the reference weak so static programs can still link. This > declaration cannot be done when compiling rtld.c (i.e. #ifdef > @@ -106,16 +120,10 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > memory_loc += l_addr... */ > if (l_addr != 0) > # endif > -#endif > for (; relative < r; ++relative) > DO_ELF_MACHINE_REL_RELATIVE (map, l_addr, relative); > > -#ifdef RTLD_BOOTSTRAP > - /* The dynamic linker always uses versioning. */ > - assert (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL); > -#else > if (map->l_info[VERSYMIDX (DT_VERSYM)]) > -#endif > { > const ElfW(Half) *const version = > (const void *) D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > @@ -126,7 +134,7 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > const ElfW(Sym) *sym = &symtab[ELFW(R_SYM) (r->r_info)]; > void *const r_addr_arg = (void *) (l_addr + r->r_offset); > const struct r_found_version *rversion = &map->l_versions[ndx]; > -#if defined ELF_MACHINE_IRELATIVE && !defined RTLD_BOOTSTRAP > +#if defined ELF_MACHINE_IRELATIVE > if (ELFW(R_TYPE) (r->r_info) == ELF_MACHINE_IRELATIVE) > { > if (r2 == NULL) > @@ -138,7 +146,7 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > > elf_machine_rel (map, scope, r, sym, rversion, r_addr_arg, > skip_ifunc); > -#if defined SHARED && !defined RTLD_BOOTSTRAP > +#if defined SHARED > if (ELFW(R_TYPE) (r->r_info) == ELF_MACHINE_JMP_SLOT > && GLRO(dl_naudit) > 0) > { > @@ -151,7 +159,7 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > #endif > } > > -#if defined ELF_MACHINE_IRELATIVE && !defined RTLD_BOOTSTRAP > +#if defined ELF_MACHINE_IRELATIVE > if (r2 != NULL) > for (; r2 <= end2; ++r2) > if (ELFW(R_TYPE) (r2->r_info) == ELF_MACHINE_IRELATIVE) > @@ -166,7 +174,6 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > } > #endif > } > -#ifndef RTLD_BOOTSTRAP > else > { > for (; r < end; ++r) > @@ -184,7 +191,7 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > # endif > elf_machine_rel (map, scope, r, sym, NULL, r_addr_arg, > skip_ifunc); > -# if defined SHARED && !defined RTLD_BOOTSTRAP > +# if defined SHARED > if (ELFW(R_TYPE) (r->r_info) == ELF_MACHINE_JMP_SLOT > && GLRO(dl_naudit) > 0) > { > @@ -207,8 +214,8 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[], > skip_ifunc); > # endif > } > -#endif > } > +#endif /* !RTLD_BOOTSTRAP */ > } > > #undef elf_dynamic_do_Rel