From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 2213E3857830 for ; Tue, 13 Apr 2021 14:03:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2213E3857830 Received: by mail-ot1-x329.google.com with SMTP id p6-20020a9d69460000b029028bb7c6ff64so87113oto.10 for ; Tue, 13 Apr 2021 07:03:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tlW3VLR9/ZmZ1Pw9w/Ta+NrXX3tAbIK6b6ZXmGVY+Ng=; b=Xe7hzbGu+sosxBH6Rag2TTO759A/DIJLGJp4P/+ZTYralDheUnLz1KB5OoZlJYNrkn pgVY97Rt5YDXcN1WGcrM6PbtLIJ7Hbu11Do3B6nJM4kxDKlaF9YJgFy1xZENcMc1eLJH DAblap31Ie7J8hSKud6uqImobe1vbZaJNziZCifahiWyo5SNbZZII/BTfNlrmzlBrlT4 4tZ81DtiHHZXyBhbpXcvodkUEuHTFeDmFVmP16gmNiskPmpkGUBf/ZuAH9vLeB7zTG2k Vb13p53lPRsBo1dF3PpMzg6yjmR2/OrIP9bDjWRXRMt6I6Uc4JLcPecUerMDIVjLB6Qg V3gw== X-Gm-Message-State: AOAM531L1GrY61j+KIjkiuGs7T2doAJFo9vk2xF0xhRw+qSrqADw9LOZ i5SCazdwfPXpjszNUG+8G2CZr8uguf1oRgVm8osXlTNWSAA= X-Google-Smtp-Source: ABdhPJxV/mEzGmzYlfBXEhME9HqTkn0+Oan5c8kH7QrIPcq/rzf0wIfeo2s6YqZ77GTlzge3dkbLqrv46PTmx36ZRWw= X-Received: by 2002:a05:6830:1515:: with SMTP id k21mr28183639otp.269.1618322637323; Tue, 13 Apr 2021 07:03:57 -0700 (PDT) MIME-Version: 1.0 References: <5419c82f25996f3887c541352e26a09446e66a3d.1618301209.git.szabolcs.nagy@arm.com> In-Reply-To: <5419c82f25996f3887c541352e26a09446e66a3d.1618301209.git.szabolcs.nagy@arm.com> From: "H.J. Lu" Date: Tue, 13 Apr 2021 07:03:21 -0700 Message-ID: Subject: Re: [PATCH v2 11/14] x86_64: Remove lazy tlsdesc relocation related code To: Szabolcs Nagy Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3034.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, 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: Tue, 13 Apr 2021 14:04:00 -0000 On Tue, Apr 13, 2021 at 2:33 AM Szabolcs Nagy via Libc-alpha wrote: > > _dl_tlsdesc_resolve_rela and _dl_tlsdesc_resolve_hold are only used for > lazy tlsdesc relocation processing which is no longer supported. > > -- > v2: > - fix elf_machine_runtime_setup: remove tlsdesc got setup. > --- > sysdeps/x86_64/dl-machine.h | 4 -- > sysdeps/x86_64/dl-tlsdesc.S | 104 ---------------------------------- > sysdeps/x86_64/dl-tlsdesc.h | 4 +- > sysdeps/x86_64/tlsdesc.c | 109 +----------------------------------- > 4 files changed, 2 insertions(+), 219 deletions(-) > > diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h > index 9a876a371e..a8596aa3fa 100644 > --- a/sysdeps/x86_64/dl-machine.h > +++ b/sysdeps/x86_64/dl-machine.h > @@ -127,10 +127,6 @@ elf_machine_runtime_setup (struct link_map *l, int lazy, int profile) > } > } > > - if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy) > - *(ElfW(Addr)*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) + l->l_addr) > - = (ElfW(Addr)) &_dl_tlsdesc_resolve_rela; > - > return lazy; > } > > diff --git a/sysdeps/x86_64/dl-tlsdesc.S b/sysdeps/x86_64/dl-tlsdesc.S > index 1d055adadd..ca9236bed8 100644 > --- a/sysdeps/x86_64/dl-tlsdesc.S > +++ b/sysdeps/x86_64/dl-tlsdesc.S > @@ -144,107 +144,3 @@ _dl_tlsdesc_dynamic: > cfi_endproc > .size _dl_tlsdesc_dynamic, .-_dl_tlsdesc_dynamic > #endif /* SHARED */ > - > - /* This function is a wrapper for a lazy resolver for TLS_DESC > - RELA relocations. The incoming 0(%rsp) points to the caller's > - link map, pushed by the dynamic object's internal lazy TLS > - resolver front-end before tail-calling us. We need to pop it > - ourselves. %rax points to a TLS descriptor, such that 0(%rax) > - holds the address of the internal resolver front-end (unless > - some other thread beat us to resolving it) and 8(%rax) holds a > - pointer to the relocation. > - > - When the actual resolver returns, it will have adjusted the > - TLS descriptor such that we can tail-call it for it to return > - the TP offset of the symbol. */ > - > - .hidden _dl_tlsdesc_resolve_rela > - .global _dl_tlsdesc_resolve_rela > - .type _dl_tlsdesc_resolve_rela,@function > - cfi_startproc > - .align 16 > - /* The PLT entry will have pushed the link_map pointer. */ > -_dl_tlsdesc_resolve_rela: > - _CET_ENDBR > - cfi_adjust_cfa_offset (8) > - /* Save all call-clobbered registers. Add 8 bytes for push in > - the PLT entry to align the stack. */ > - subq $80, %rsp > - cfi_adjust_cfa_offset (80) > - movq %rax, (%rsp) > - movq %rdi, 8(%rsp) > - movq %rax, %rdi /* Pass tlsdesc* in %rdi. */ > - movq %rsi, 16(%rsp) > - movq 80(%rsp), %rsi /* Pass link_map* in %rsi. */ > - movq %r8, 24(%rsp) > - movq %r9, 32(%rsp) > - movq %r10, 40(%rsp) > - movq %r11, 48(%rsp) > - movq %rdx, 56(%rsp) > - movq %rcx, 64(%rsp) > - call _dl_tlsdesc_resolve_rela_fixup > - movq (%rsp), %rax > - movq 8(%rsp), %rdi > - movq 16(%rsp), %rsi > - movq 24(%rsp), %r8 > - movq 32(%rsp), %r9 > - movq 40(%rsp), %r10 > - movq 48(%rsp), %r11 > - movq 56(%rsp), %rdx > - movq 64(%rsp), %rcx > - addq $88, %rsp > - cfi_adjust_cfa_offset (-88) > - jmp *(%rax) > - cfi_endproc > - .size _dl_tlsdesc_resolve_rela, .-_dl_tlsdesc_resolve_rela > - > - /* This function is a placeholder for lazy resolving of TLS > - relocations. Once some thread starts resolving a TLS > - relocation, it sets up the TLS descriptor to use this > - resolver, such that other threads that would attempt to > - resolve it concurrently may skip the call to the original lazy > - resolver and go straight to a condition wait. > - > - When the actual resolver returns, it will have adjusted the > - TLS descriptor such that we can tail-call it for it to return > - the TP offset of the symbol. */ > - > - .hidden _dl_tlsdesc_resolve_hold > - .global _dl_tlsdesc_resolve_hold > - .type _dl_tlsdesc_resolve_hold,@function > - cfi_startproc > - .align 16 > -_dl_tlsdesc_resolve_hold: > -0: > - _CET_ENDBR > - /* Save all call-clobbered registers. */ > - subq $72, %rsp > - cfi_adjust_cfa_offset (72) > - movq %rax, (%rsp) > - movq %rdi, 8(%rsp) > - movq %rax, %rdi /* Pass tlsdesc* in %rdi. */ > - movq %rsi, 16(%rsp) > - /* Pass _dl_tlsdesc_resolve_hold's address in %rsi. */ > - leaq . - _dl_tlsdesc_resolve_hold(%rip), %rsi > - movq %r8, 24(%rsp) > - movq %r9, 32(%rsp) > - movq %r10, 40(%rsp) > - movq %r11, 48(%rsp) > - movq %rdx, 56(%rsp) > - movq %rcx, 64(%rsp) > - call _dl_tlsdesc_resolve_hold_fixup > -1: > - movq (%rsp), %rax > - movq 8(%rsp), %rdi > - movq 16(%rsp), %rsi > - movq 24(%rsp), %r8 > - movq 32(%rsp), %r9 > - movq 40(%rsp), %r10 > - movq 48(%rsp), %r11 > - movq 56(%rsp), %rdx > - movq 64(%rsp), %rcx > - addq $72, %rsp > - cfi_adjust_cfa_offset (-72) > - jmp *(%rax) > - cfi_endproc > - .size _dl_tlsdesc_resolve_hold, .-_dl_tlsdesc_resolve_hold > diff --git a/sysdeps/x86_64/dl-tlsdesc.h b/sysdeps/x86_64/dl-tlsdesc.h > index d134b3f4db..03d5ac7a54 100644 > --- a/sysdeps/x86_64/dl-tlsdesc.h > +++ b/sysdeps/x86_64/dl-tlsdesc.h > @@ -55,9 +55,7 @@ struct tlsdesc_dynamic_arg > > extern ptrdiff_t attribute_hidden > _dl_tlsdesc_return(struct tlsdesc *on_rax), > - _dl_tlsdesc_undefweak(struct tlsdesc *on_rax), > - _dl_tlsdesc_resolve_rela(struct tlsdesc *on_rax), > - _dl_tlsdesc_resolve_hold(struct tlsdesc *on_rax); > + _dl_tlsdesc_undefweak(struct tlsdesc *on_rax); > > # ifdef SHARED > extern void *_dl_make_tlsdesc_dynamic (struct link_map *map, > diff --git a/sysdeps/x86_64/tlsdesc.c b/sysdeps/x86_64/tlsdesc.c > index 4083849f22..ecf864d6ee 100644 > --- a/sysdeps/x86_64/tlsdesc.c > +++ b/sysdeps/x86_64/tlsdesc.c > @@ -16,120 +16,13 @@ > License along with the GNU C Library; if not, see > . */ > > -#include > #include > -#include > #include > #include > #include > +#define _dl_tlsdesc_resolve_hold 0 > #include > > -/* The following 2 functions take a caller argument, that contains the > - address expected to be in the TLS descriptor. If it's changed, we > - want to return immediately. */ > - > -/* This function is used to lazily resolve TLS_DESC RELA relocations. > - The argument location is used to hold a pointer to the relocation. */ > - > -void > -attribute_hidden > -_dl_tlsdesc_resolve_rela_fixup (struct tlsdesc volatile *td, > - struct link_map *l) > -{ > - const ElfW(Rela) *reloc = td->arg; > - > - if (_dl_tlsdesc_resolve_early_return_p > - (td, (void*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_PLT)]) + l->l_addr))) > - return; > - > - /* The code below was borrowed from _dl_fixup(). */ > - const ElfW(Sym) *const symtab > - = (const void *) D_PTR (l, l_info[DT_SYMTAB]); > - const char *strtab = (const void *) D_PTR (l, l_info[DT_STRTAB]); > - const ElfW(Sym) *sym = &symtab[ELFW(R_SYM) (reloc->r_info)]; > - lookup_t result; > - > - /* Look up the target symbol. If the normal lookup rules are not > - used don't look in the global scope. */ > - if (ELFW(ST_BIND) (sym->st_info) != STB_LOCAL > - && __builtin_expect (ELFW(ST_VISIBILITY) (sym->st_other), 0) == 0) > - { > - const struct r_found_version *version = NULL; > - > - if (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > - { > - const ElfW(Half) *vernum = > - (const void *) D_PTR (l, l_info[VERSYMIDX (DT_VERSYM)]); > - ElfW(Half) ndx = vernum[ELFW(R_SYM) (reloc->r_info)] & 0x7fff; > - version = &l->l_versions[ndx]; > - if (version->hash == 0) > - version = NULL; > - } > - > - result = _dl_lookup_symbol_x (strtab + sym->st_name, l, &sym, > - l->l_scope, version, ELF_RTYPE_CLASS_PLT, > - DL_LOOKUP_ADD_DEPENDENCY, NULL); > - } > - else > - { > - /* We already found the symbol. The module (and therefore its load > - address) is also known. */ > - result = l; > - } > - > - if (! sym) > - { > - td->arg = (void*)reloc->r_addend; > - td->entry = _dl_tlsdesc_undefweak; > - } > - else > - { > -# ifndef SHARED > - CHECK_STATIC_TLS (l, result); > -# else > - if (!TRY_STATIC_TLS (l, result)) > - { > - td->arg = _dl_make_tlsdesc_dynamic (result, sym->st_value > - + reloc->r_addend); > - td->entry = _dl_tlsdesc_dynamic; > - } > - else > -# endif > - { > - td->arg = (void*)(sym->st_value - result->l_tls_offset > - + reloc->r_addend); > - td->entry = _dl_tlsdesc_return; > - } > - } > - > - _dl_tlsdesc_wake_up_held_fixups (); > -} > - > -/* This function is used to avoid busy waiting for other threads to > - complete the lazy relocation. Once another thread wins the race to > - relocate a TLS descriptor, it sets the descriptor up such that this > - function is called to wait until the resolver releases the > - lock. */ > - > -void > -attribute_hidden > -_dl_tlsdesc_resolve_hold_fixup (struct tlsdesc volatile *td, > - void *caller) > -{ > - /* Maybe we're lucky and can return early. */ > - if (caller != td->entry) > - return; > - > - /* Locking here will stop execution until the running resolver runs > - _dl_tlsdesc_wake_up_held_fixups(), releasing the lock. > - > - FIXME: We'd be better off waiting on a condition variable, such > - that we didn't have to hold the lock throughout the relocation > - processing. */ > - __rtld_lock_lock_recursive (GL(dl_load_lock)); > - __rtld_lock_unlock_recursive (GL(dl_load_lock)); > -} > - > /* Unmap the dynamic object, but also release its TLS descriptor table > if there is one. */ > > -- > 2.17.1 > LGTM. Thanks. -- H.J.