Re: [PATCH] make relocation time symbol lookup and dlsym consistent

[Rich Felker <dalias@libc.org>]

On Tue, Aug 13, 2019 at 10:38:44AM +0200, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@port70.net> [2019-08-11 01:18:11 +0200]:
> >  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((tls_mod_off_t []){def.dso->tls_id, def.sym->st_value-DTP_OFFSET});
> > -		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))
> > +	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((tls_mod_off_t []){p->tls_id, sym->st_value-DTP_OFFSET});
> > -	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);
> > -	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((tls_mod_off_t []){p->deps[i]->tls_id, sym->st_value-DTP_OFFSET});
> > -		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;
> > +	struct symdef def = find_sym2(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((tls_mod_off_t []){def.dso->tls_id, def.sym->st_value-DTP_OFFSET});
> > +	if (DL_FDPIC && (def.sym->st_info&0xf) == STT_FUNC)
> > +		return def.dso->funcdescs + (def.sym - def.dso->syms);
> 
> there is another behaviour change i did not notice before:
> 
> st_shndx is no longer checked in DL_FDPIC case here, i assumed
> find_sym did that, but there is no fdpic specific logic there.
> 
> the old code was inconsistent in the RTLD_DEFAULT vs shared lib
> dlsym case.
> 
> i dont know if this is relevant for fdpic, i didnt test that case.

Thanks for catching. I'll take a look at this.

Rich