From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11044 Path: news.gmane.org!.POSTED!not-for-mail From: Szabolcs Nagy Newsgroups: gmane.linux.lib.musl.general Subject: Re: Reviving planned ldso changes Date: Thu, 16 Feb 2017 02:58:24 +0100 Message-ID: <20170216015824.GG12395@port70.net> References: <20170103054351.GA8459@brightrain.aerifal.cx> <20170104105108.GD6695@port70.net> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="uAKRQypu60I7Lcqm" X-Trace: blaine.gmane.org 1487210321 19699 195.159.176.226 (16 Feb 2017 01:58:41 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 16 Feb 2017 01:58:41 +0000 (UTC) User-Agent: Mutt/1.6.0 (2016-04-01) To: musl@lists.openwall.com Original-X-From: musl-return-11059-gllmg-musl=m.gmane.org@lists.openwall.com Thu Feb 16 02:58:36 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1ceBL7-0004My-EA for gllmg-musl@m.gmane.org; Thu, 16 Feb 2017 02:58:33 +0100 Original-Received: (qmail 8007 invoked by uid 550); 16 Feb 2017 01:58:36 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 7986 invoked from network); 16 Feb 2017 01:58:36 -0000 Mail-Followup-To: musl@lists.openwall.com Content-Disposition: inline In-Reply-To: <20170104105108.GD6695@port70.net> Xref: news.gmane.org gmane.linux.lib.musl.general:11044 Archived-At: --uAKRQypu60I7Lcqm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline * Szabolcs Nagy [2017-01-04 11:51:09 +0100]: > * Rich Felker [2017-01-03 00:43:51 -0500]: > > One item that's been on the agenda for a long time (maybe 1.5 years > > now?) is planned dynamic linker improvements. The thread "Further > > dynamic linker optimizations" from summer 2015 covered parts but maybe > > not all of what I had in mind. ... > - symbol lookup logic is duplicated between do_dlsym and find_sym, > it would be nice to do that with common code path if possible. i looked into this and found some issues i attached my current patches for comments, but the bugs may be fixed separately from the refactoring. --uAKRQypu60I7Lcqm Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-treat-STB_WEAK-and-STB_GNU_UNIQUE-like-STB_GLOBAL-in.patch" >From 5dd896dcdf53e50fc2d7c1957d62df7b8930ff01 Mon Sep 17 00:00:00 2001 From: Szabolcs Nagy Date: Sat, 3 Dec 2016 20:52:43 +0000 Subject: [PATCH 1/2] treat STB_WEAK and STB_GNU_UNIQUE like STB_GLOBAL in find_sym A weak symbol definition is not special during dynamic linking, so don't let a strong definition in a later module override it. (glibc dynamic linker allows overriding weak definitions if LD_DYNAMIC_WEAK is set, musl does not.) STB_GNU_UNIQUE means that the symbol is global, even if it is in a module that's loaded with RTLD_LOCAL, and all references resolve to the same definition. This semantics is only relevant for c++ plugin systems and even there it's often not what the user wants (so it can be turned off in g++ by -fno-gnu-unique when the c++ shared lib is compiled). In musl just treat it like STB_GLOBAL. --- ldso/dynlink.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ldso/dynlink.c b/ldso/dynlink.c index c6890845..476cd6a8 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -286,11 +286,9 @@ static struct symdef find_sym(struct dso *dso, const char *s, int need_def) continue; if (!(1<<(sym->st_info&0xf) & OK_TYPES)) continue; if (!(1<<(sym->st_info>>4) & OK_BINDS)) continue; - - if (def.sym && sym->st_info>>4 == STB_WEAK) continue; def.sym = sym; def.dso = dso; - if (sym->st_info>>4 == STB_GLOBAL) break; + break; } return def; } -- 2.11.0 --uAKRQypu60I7Lcqm Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-make-relocation-time-symbol-lookup-and-dlsym-consist.patch" >From b5e20dbf30e67a12bc0931f12126553d036bbd31 Mon Sep 17 00:00:00 2001 From: Szabolcs Nagy Date: Sat, 3 Dec 2016 23:08:44 +0000 Subject: [PATCH 2/2] make relocation time symbol lookup and dlsym consistent Using common code path for all symbol lookups fixes three dlsym issues: - st_shndx of STT_TLS symbols were not checked and thus an undefined tls symbol reference could be incorrectly treated as a definition (the sysv hash lookup returns undefined symbols, gnu does not, so should be rare in practice). - symbol binding was not checked so a hidden symbol may be returned (in principle STB_LOCAL symbols may appear in the dynamic symbol table for hidden symbols, but linkers most likely don't produce it). - mips specific behaviour was not applied (ARCH_SYM_REJECT_UND). TODO: not benchmarked, common code should be inlined probably. --- ldso/dynlink.c | 83 +++++++++++++++++++--------------------------------------- 1 file changed, 27 insertions(+), 56 deletions(-) diff --git a/ldso/dynlink.c b/ldso/dynlink.c index 476cd6a8..e5a382b0 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -256,14 +256,15 @@ static Sym *gnu_lookup_filtered(uint32_t h1, uint32_t *hashtab, struct dso *dso, #define ARCH_SYM_REJECT_UND(s) 0 #endif -static struct symdef find_sym(struct dso *dso, const char *s, int need_def) +static struct symdef find_sym(struct dso *dso, const char *s, int need_def, int use_deps) { uint32_t h = 0, gh, gho, *ght; size_t ghm = 0; struct symdef def = {0}; - for (; dso; dso=dso->next) { + struct dso **deps = use_deps ? dso->deps : 0; + for (; dso; dso=use_deps ? *deps++ : dso->next) { Sym *sym; - if (!dso->global) continue; + if (!dso->global && !use_deps) continue; if ((ght = dso->ghashtab)) { if (!ghm) { gh = gnu_hash(s); @@ -332,7 +333,7 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri ctx = type==REL_COPY ? head->next : head; def = (sym->st_info&0xf) == STT_SECTION ? (struct symdef){ .dso = dso, .sym = sym } - : find_sym(ctx, name, type==REL_PLT); + : find_sym(ctx, name, type==REL_PLT, 0); if (!def.sym && (sym->st_shndx != SHN_UNDEF || sym->st_info>>4 != STB_WEAK)) { error("Error relocating %s: %s: symbol not found", @@ -1377,7 +1378,7 @@ void __dls2(unsigned char *base, size_t *sp) /* Call dynamic linker stage-3, __dls3, looking it up * symbolically as a barrier against moving the address * load across the above relocation processing. */ - struct symdef dls3_def = find_sym(&ldso, "__dls3", 0); + struct symdef dls3_def = find_sym(&ldso, "__dls3", 0, 0); if (DL_FDPIC) ((stage3_func)&ldso.funcdescs[dls3_def.sym-ldso.syms])(sp); else ((stage3_func)laddr(&ldso, dls3_def.sym->st_value))(sp); } @@ -1770,58 +1771,28 @@ void *__tls_get_addr(size_t *); static void *do_dlsym(struct dso *p, const char *s, void *ra) { - size_t i; - uint32_t h = 0, gh = 0, *ght; - Sym *sym; - if (p == head || p == RTLD_DEFAULT || p == RTLD_NEXT) { - if (p == RTLD_DEFAULT) { - p = head; - } else if (p == RTLD_NEXT) { - p = addr2dso((size_t)ra); - if (!p) p=head; - p = p->next; - } - struct symdef def = find_sym(p, s, 0); - if (!def.sym) goto failed; - if ((def.sym->st_info&0xf) == STT_TLS) - return __tls_get_addr((size_t []){def.dso->tls_id, def.sym->st_value}); - if (DL_FDPIC && (def.sym->st_info&0xf) == STT_FUNC) - return def.dso->funcdescs + (def.sym - def.dso->syms); - return laddr(def.dso, def.sym->st_value); - } - if (__dl_invalid_handle(p)) + struct symdef def; + int use_deps = 0; + if (p == head || p == RTLD_DEFAULT) { + p = head; + } else if (p == RTLD_NEXT) { + p = addr2dso((size_t)ra); + if (!p) p=head; + p = p->next; + } else if (__dl_invalid_handle(p)) { return 0; - if ((ght = p->ghashtab)) { - gh = gnu_hash(s); - sym = gnu_lookup(gh, ght, p, s); - } else { - h = sysv_hash(s); - sym = sysv_lookup(s, h, p); - } - if (sym && (sym->st_info&0xf) == STT_TLS) - return __tls_get_addr((size_t []){p->tls_id, sym->st_value}); - if (DL_FDPIC && sym && sym->st_shndx && (sym->st_info&0xf) == STT_FUNC) - return p->funcdescs + (sym - p->syms); - if (sym && sym->st_value && (1<<(sym->st_info&0xf) & OK_TYPES)) - return laddr(p, sym->st_value); - if (p->deps) for (i=0; p->deps[i]; i++) { - if ((ght = p->deps[i]->ghashtab)) { - if (!gh) gh = gnu_hash(s); - sym = gnu_lookup(gh, ght, p->deps[i], s); - } else { - if (!h) h = sysv_hash(s); - sym = sysv_lookup(s, h, p->deps[i]); - } - if (sym && (sym->st_info&0xf) == STT_TLS) - return __tls_get_addr((size_t []){p->deps[i]->tls_id, sym->st_value}); - if (DL_FDPIC && sym && sym->st_shndx && (sym->st_info&0xf) == STT_FUNC) - return p->deps[i]->funcdescs + (sym - p->deps[i]->syms); - if (sym && sym->st_value && (1<<(sym->st_info&0xf) & OK_TYPES)) - return laddr(p->deps[i], sym->st_value); - } -failed: - error("Symbol not found: %s", s); - return 0; + } else + use_deps = 1; + def = find_sym(p, s, 0, use_deps); + if (!def.sym) { + error("Symbol not found: %s", s); + return 0; + } + if ((def.sym->st_info&0xf) == STT_TLS) + return __tls_get_addr((size_t []){def.dso->tls_id, def.sym->st_value}); + if (DL_FDPIC && (def.sym->st_info&0xf) == STT_FUNC) + return def.dso->funcdescs + (def.sym - def.dso->syms); + return laddr(def.dso, def.sym->st_value); } int dladdr(const void *addr, Dl_info *info) -- 2.11.0 --uAKRQypu60I7Lcqm--