Re: ldso : dladdr support

[musl <b.brezillon.musl@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2933 bytes --]

Hi,

This patch fixes a bug in dladdr: sym var was not incremented across gnu hash chain iteration).
I also reworked the dladdr implem to share more code between sysv and gnu hash.
I still haven't found a better way to get the symbol table size. Do you?

This patch uses the new decode_vec function, but as I told you in my previous mail, I'm not sure this the way to go.
Could you tell me what you think?

On 17/08/2012 07:39, Rich Felker wrote:
> On Thu, Aug 16, 2012 at 12:41:48AM +0200, boris brezillon wrote:
>>> Sorry for taking a while to get back to you. I haven't had as much
>>> time to work on musl the past couple weeks and some other topics (like
>>> mips dynamic linking) had priority, but I hope to have more again for
>>> a while now. Here's a quick review of some things that will hopefully
>>> turn into a discussion for improving/simplifying the code.
>> No problem.
>> Thanks for the review.
>> Do you want to discuss it on irc or keep going on the mailing list?
> List is best I think, but I don't mind having some specific
> discussions that need more real-time feedback on IRC.
>
>>> BTW, while I _want_ it to be safe, it's possible that early switches
>>> (early meaning prior to the comment in __dynlink that says libc is now
>>> fully functional) will actually fail to work/crash on some archs... So
>>> this needs consideration too.
>>>
>> I didn't knew that. Could explain me why?
> The compiler may compile a switch to something like:
>
> jmp *local_jmptable@GOTOFF(%ebx,%ecx,4)
>
> where the local_jmptable needs to contain absolute jump addresses.
> Prior to relocation, it will obly have the load-address-relative
> addresses.
>
>>> I'm not seeing why this function needs hash tables at all. It's not
>>> looking up symbols, just iterating over the entire symbol table, no?
>>> Please explain if I'm mistaken.
>> I don't see any other way to know the sym table size except reading
>> the .dynsym section header.
> Indeed, I missed the fact that there's no DT_* entry reporting the
> symbol table size. I checked how readelf reports the number of
> entries, and it's using the section headers. I'm under the impression
> that using them would not be valid for the dynamic linker; reportedly
> ELF files are supposed to be valid even with the section headers
> stripped.
>
>> That's why I'm iterating over the hash table.
>> For sysv hash the nchain (second entry of hash table) gives the sym table size.
>> For gnu hash It's a little bit more complicated (see
>> https://blogs.oracle.com/ali/entry/gnu_hash_elf_sections).
> Wow, leave it to the GNU folks to take something simple and make it
> difficult...
>
>> Should we parse the .dynsym section header and store the sym table
>> size in dso struct?
>> Do you see any other way to get the dynsym table size or at least
>> iterate over the dynsym table (specific pattern for last element ?).
> I've been looking but I don't see any way yet.
>
> Rich


[-- Attachment #2: dladdr-gnu-hash-v3.patch --]
[-- Type: text/x-patch, Size: 14726 bytes --]

diff --git a/include/dlfcn.h b/include/dlfcn.h
index dea74c7..8524e0b 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -18,6 +18,17 @@ char  *dlerror(void);
 void  *dlopen(const char *, int);
 void  *dlsym(void *, const char *);
 
+#ifdef _GNU_SOURCE
+typedef struct {
+	const char *dli_fname;
+	void *dli_fbase;
+	const char *dli_sname;
+	void *dli_saddr;
+} Dl_info;
+
+int dladdr (void *addr, Dl_info *info);
+#endif
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
index f55c6f1..bf1ec6b 100644
--- a/src/ldso/dynlink.c
+++ b/src/ldso/dynlink.c
@@ -1,3 +1,4 @@
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -28,12 +29,14 @@ typedef Elf32_Phdr Phdr;
 typedef Elf32_Sym Sym;
 #define R_TYPE(x) ((x)&255)
 #define R_SYM(x) ((x)>>8)
+#define ELF_ST_TYPE ELF32_ST_TYPE
 #else
 typedef Elf64_Ehdr Ehdr;
 typedef Elf64_Phdr Phdr;
 typedef Elf64_Sym Sym;
 #define R_TYPE(x) ((x)&0xffffffff)
 #define R_SYM(x) ((x)>>32)
+#define ELF_ST_TYPE ELF64_ST_TYPE
 #endif
 
 struct debug {
@@ -53,6 +56,7 @@ struct dso {
 	int refcnt;
 	Sym *syms;
 	uint32_t *hashtab;
+	uint32_t *ghashtab;
 	char *strings;
 	unsigned char *map;
 	size_t map_len;
@@ -82,19 +86,27 @@ static struct debug debug;
 
 struct debug *_dl_debug_addr = &debug;
 
-#define AUX_CNT 24
-#define DYN_CNT 34
-
-static void decode_vec(size_t *v, size_t *a, size_t cnt)
+static void decode_vec(size_t *v, const size_t *def, size_t *storage, size_t *found, size_t size)
 {
-	memset(a, 0, cnt*sizeof(size_t));
-	for (; v[0]; v+=2) if (v[0]<cnt) {
-		a[0] |= 1ULL<<v[0];
-		a[v[0]] = v[1];
+	size_t i;
+	memset(storage, 0, size*sizeof(size_t));
+	memset(found, 0, (size+sizeof(size_t)*8-1)/8);
+	for (; v[0]; v+=2) {
+		if (v[0] < def[0] || v[0] > def[size-1])
+			continue;
+		for (i = 0; i < size; ++i) {
+			if (v[0] < def[i])
+				break;
+			else if (v[0] == def[i]) {
+				storage[i] = v[1];
+				found[i/(sizeof(size_t)*8)] |= (1 << (i%(sizeof(size_t)*8)));
+				break;
+			}
+		}
 	}
 }
 
-static uint32_t hash(const char *s0)
+static uint32_t sysv_hash(const char *s0)
 {
 	const unsigned char *s = (void *)s0;
 	uint_fast32_t h = 0;
@@ -105,7 +117,16 @@ static uint32_t hash(const char *s0)
 	return h & 0xfffffff;
 }
 
-static Sym *lookup(const char *s, uint32_t h, struct dso *dso)
+static uint32_t gnu_hash (const char *s0)
+{
+	const unsigned char *s = (void *)s0;
+	uint_fast32_t h = 5381;
+	for (; *s; s++)
+		h = h*33 + *s;
+	return h & 0xffffffff;
+}
+
+static Sym *sysv_lookup(const char *s, uint32_t h, struct dso *dso)
 {
 	size_t i;
 	Sym *syms = dso->syms;
@@ -118,20 +139,86 @@ static Sym *lookup(const char *s, uint32_t h, struct dso *dso)
 	return 0;
 }
 
+static Sym *gnu_lookup(const char *s, uint32_t h1, struct dso *dso)
+{
+	size_t i;
+	Sym *sym;
+	char *strings = dso->strings;
+	uint32_t *hashtab = dso->ghashtab;
+	uint32_t nbuckets = hashtab[0];
+	size_t *maskwords = (size_t *)(hashtab + 4);
+	uint32_t *buckets = hashtab + 4 + (hashtab[2]*(sizeof(size_t)/sizeof(uint32_t)));
+	uint32_t symndx = hashtab[1];
+	Sym *syms = dso->syms;
+	uint32_t shift2 = hashtab[3];
+	uint32_t h2 = h1 >> shift2;
+	uint32_t *hashvals = buckets + nbuckets;
+	uint32_t *hashval;
+	size_t c = sizeof(size_t) * 8;
+	size_t n = (h1/c) & (hashtab[2]-1);
+	size_t bitmask = (1 << (h1%c)) | (1 << (h2%c));
+
+	if ((maskwords[n] & bitmask) != bitmask)
+		return 0;
+
+	n = buckets[h1 % nbuckets];
+	if (!n)
+		return 0;
+
+	sym = syms + n;
+	hashval = hashvals + n - symndx;
+
+	for (h1 &= (uint32_t)-2;; sym++) {
+		h2 = *hashval++;
+		if ((h1 == (h2 & ~1)) && !strcmp(s, strings + sym->st_name))
+			return sym;
+
+		if (h2 & 1)
+			break;
+	}
+
+	return 0;
+}
+
 #define OK_TYPES (1<<STT_NOTYPE | 1<<STT_OBJECT | 1<<STT_FUNC | 1<<STT_COMMON)
 #define OK_BINDS (1<<STB_GLOBAL | 1<<STB_WEAK)
 
 static void *find_sym(struct dso *dso, const char *s, int need_def)
 {
-	uint32_t h = hash(s);
 	void *def = 0;
-	if (h==0x6b366be && !strcmp(s, "dlopen")) rtld_used = 1;
-	if (h==0x6b3afd && !strcmp(s, "dlsym")) rtld_used = 1;
-	if (h==0x595a4cc && !strcmp(s, "__stack_chk_fail")) ssp_used = 1;
+	static uint32_t precomp[2][3] = {
+		{0x6b366be, 0x6b3afd, 0x595a4cc},
+		{0xf9040207, 0xf4dc4ae, 0x1f4039c9},
+	};
+	uint32_t *precomptab;
+	uint32_t h = 0, gh = 0;
+	if (dso->hashtab) {
+		h = sysv_hash(s);
+		precomptab = precomp[0];
+	} else {
+		gh = gnu_hash(s);
+		precomptab = precomp[0];
+	}
+
+	if (h == precomptab[0] && !strcmp(s, "dlopen")) rtld_used = 1;
+	if (h == precomptab[1] && !strcmp(s, "dlsym")) rtld_used = 1;
+	if (h == precomptab[2] && !strcmp(s, "__stack_chk_fail")) ssp_used = 1;
+
 	for (; dso; dso=dso->next) {
 		Sym *sym;
+
 		if (!dso->global) continue;
-		sym = lookup(s, h, dso);
+
+		if (dso->hashtab && (h || !dso->ghashtab)) {
+			if (!h)
+				h = sysv_hash(s);
+			sym = sysv_lookup(s, h, dso);
+		} else {
+			if (!gh)
+				gh = gnu_hash(s);
+			sym = gnu_lookup(s, gh, dso);
+		}
+
 		if (sym && (!need_def || sym->st_shndx) && sym->st_value
 		 && (1<<(sym->st_info&0xf) & OK_TYPES)
 		 && (1<<(sym->st_info>>4) & OK_BINDS)) {
@@ -320,11 +407,16 @@ static int path_open(const char *name, const char *search, char *buf, size_t buf
 
 static void decode_dyn(struct dso *p)
 {
-	size_t dyn[DYN_CNT] = {0};
-	decode_vec(p->dynv, dyn, DYN_CNT);
-	p->syms = (void *)(p->base + dyn[DT_SYMTAB]);
-	p->hashtab = (void *)(p->base + dyn[DT_HASH]);
-	p->strings = (void *)(p->base + dyn[DT_STRTAB]);
+	static const size_t def[] = {DT_HASH, DT_STRTAB, DT_SYMTAB, DT_GNU_HASH};
+	size_t dyn[4];
+	size_t found;
+	decode_vec (p->dynv, def, dyn, &found, 4);
+	p->syms = (void *)(p->base + dyn[2]);
+	p->strings = (void *)(p->base + dyn[1]);
+	if (found&(1<<0))
+		p->hashtab = (void *)(p->base + dyn[0]);
+	if (found&(1<<3))
+		p->ghashtab = (void *)(p->base + dyn[3]);
 }
 
 static struct dso *load_library(const char *name)
@@ -485,17 +577,22 @@ static void make_global(struct dso *p)
 
 static void reloc_all(struct dso *p)
 {
-	size_t dyn[DYN_CNT] = {0};
+	size_t dyn[7];
+	size_t found;
+	static const size_t def[] = {
+		DT_PLTRELSZ, DT_RELA, DT_RELASZ, DT_REL, 
+		DT_RELSZ, DT_PLTREL, DT_JMPREL
+	};
 	for (; p; p=p->next) {
 		if (p->relocated) continue;
-		decode_vec(p->dynv, dyn, DYN_CNT);
+		decode_vec(p->dynv, def, dyn, &found, 7);
 #ifdef NEED_ARCH_RELOCS
 		do_arch_relocs(p, head);
 #endif
-		do_relocs(p, (void *)(p->base+dyn[DT_JMPREL]), dyn[DT_PLTRELSZ],
-			2+(dyn[DT_PLTREL]==DT_RELA));
-		do_relocs(p, (void *)(p->base+dyn[DT_REL]), dyn[DT_RELSZ], 2);
-		do_relocs(p, (void *)(p->base+dyn[DT_RELA]), dyn[DT_RELASZ], 3);
+		do_relocs(p, (void *)(p->base+dyn[6]), dyn[0],
+			2+(dyn[5]==DT_RELA));
+		do_relocs(p, (void *)(p->base+dyn[3]), dyn[4], 2);
+		do_relocs(p, (void *)(p->base+dyn[1]), dyn[2], 3);
 		p->relocated = 1;
 	}
 }
@@ -520,14 +617,16 @@ static size_t find_dyn(Phdr *ph, size_t cnt, size_t stride)
 
 static void do_init_fini(struct dso *p)
 {
-	size_t dyn[DYN_CNT] = {0};
+	size_t dyn[2];
+	static const size_t def[] = {DT_INIT, DT_FINI};
+	size_t found;
 	for (; p; p=p->prev) {
 		if (p->constructed) return;
-		decode_vec(p->dynv, dyn, DYN_CNT);
-		if (dyn[0] & (1<<DT_FINI))
-			atexit((void (*)(void))(p->base + dyn[DT_FINI]));
-		if (dyn[0] & (1<<DT_INIT))
-			((void (*)(void))(p->base + dyn[DT_INIT]))();
+		decode_vec(p->dynv, def, dyn, &found, 2);
+		if (found & (1<<1))
+			atexit((void (*)(void))(p->base + dyn[1]));
+		if (found & (1<<0))
+			((void (*)(void))(p->base + dyn[0]))();
 		p->constructed = 1;
 	}
 }
@@ -538,7 +637,14 @@ void _dl_debug_state(void)
 
 void *__dynlink(int argc, char **argv)
 {
-	size_t *auxv, aux[AUX_CNT] = {0};
+	size_t *auxv;
+	static const size_t def[] = {
+		AT_PHDR, AT_PHENT, AT_PHNUM, AT_BASE, AT_ENTRY,
+		AT_UID,	AT_EUID, AT_GID, AT_EGID, AT_SECURE,
+		AT_SYSINFO_EHDR
+	};
+	size_t aux[11];
+	size_t found;
 	size_t i;
 	Phdr *phdr;
 	Ehdr *ehdr;
@@ -556,11 +662,11 @@ void *__dynlink(int argc, char **argv)
 			env_preload = argv[i]+11;
 	auxv = (void *)(argv+i+1);
 
-	decode_vec(auxv, aux, AUX_CNT);
+	decode_vec(auxv, def, aux, &found, 11);
 
 	/* Only trust user/env if kernel says we're not suid/sgid */
-	if ((aux[0]&0x7800)!=0x7800 || aux[AT_UID]!=aux[AT_EUID]
-	  || aux[AT_GID]!=aux[AT_EGID] || aux[AT_SECURE]) {
+	if ((found&0x1e0)!=0x1e0 || aux[5]!=aux[6]
+	  || aux[7]!=aux[8] || aux[9]) {
 		env_path = 0;
 		env_preload = 0;
 	}
@@ -569,36 +675,35 @@ void *__dynlink(int argc, char **argv)
 	 * will not be set. In that case, we assume the base address is
 	 * the start of the page containing the PHDRs; I don't know any
 	 * better approach... */
-	if (!aux[AT_BASE]) {
-		aux[AT_BASE] = aux[AT_PHDR] & -PAGE_SIZE;
-		aux[AT_PHDR] = aux[AT_PHENT] = aux[AT_PHNUM] = 0;
+	if (!aux[3]) {
+		aux[3] = aux[0] & -PAGE_SIZE;
+		aux[0] = aux[1] = aux[2] = 0;
 	}
 
 	/* The dynamic linker load address is passed by the kernel
 	 * in the AUX vector, so this is easy. */
-	lib->base = (void *)aux[AT_BASE];
+	lib->base = (void *)aux[3];
 	lib->name = lib->shortname = "libc.so";
 	lib->global = 1;
 	ehdr = (void *)lib->base;
 	lib->dynv = (void *)(lib->base + find_dyn(
-		(void *)(aux[AT_BASE]+ehdr->e_phoff),
+		(void *)(aux[3]+ehdr->e_phoff),
 		ehdr->e_phnum, ehdr->e_phentsize));
 	decode_dyn(lib);
-
-	if (aux[AT_PHDR]) {
+	if (aux[0]) {
 		size_t interp_off = 0;
 		/* Find load address of the main program, via AT_PHDR vs PT_PHDR. */
-		phdr = (void *)aux[AT_PHDR];
-		for (i=aux[AT_PHNUM]; i; i--, phdr=(void *)((char *)phdr + aux[AT_PHENT])) {
+		phdr = (void *)aux[0];
+		for (i=aux[2]; i; i--, phdr=(void *)((char *)phdr + aux[1])) {
 			if (phdr->p_type == PT_PHDR)
-				app->base = (void *)(aux[AT_PHDR] - phdr->p_vaddr);
+				app->base = (void *)(aux[0] - phdr->p_vaddr);
 			else if (phdr->p_type == PT_INTERP)
 				interp_off = (size_t)phdr->p_vaddr;
 		}
 		if (interp_off) lib->name = (char *)app->base + interp_off;
 		app->name = argv[0];
 		app->dynv = (void *)(app->base + find_dyn(
-			(void *)aux[AT_PHDR], aux[AT_PHNUM], aux[AT_PHENT]));
+			(void *)aux[0], aux[2], aux[1]));
 	} else {
 		int fd;
 		char *ldname = argv[0];
@@ -628,16 +733,15 @@ void *__dynlink(int argc, char **argv)
 		lib->name = ldname;
 		app->name = argv[0];
 		app->dynv = (void *)(app->base + dyno);
-		aux[AT_ENTRY] = ehdr->e_entry;
+		aux[4] = ehdr->e_entry;
 	}
 	app->global = 1;
 	app->constructed = 1;
 	decode_dyn(app);
 
 	/* Attach to vdso, if provided by the kernel */
-	for (i=0; auxv[i]; i+=2) {
-		size_t vdso_base = auxv[i+1];
-		if (auxv[i] != AT_SYSINFO_EHDR) continue;
+	if (found&(1<<10)) {
+		size_t vdso_base = aux[10];
 		ehdr = (void *)vdso_base;
 		phdr = (void *)(vdso_base + ehdr->e_phoff);
 		for (i=ehdr->e_phnum; i; i--, phdr=(void *)((char *)phdr + ehdr->e_phentsize)) {
@@ -651,7 +755,6 @@ void *__dynlink(int argc, char **argv)
 		decode_dyn(vdso);
 		vdso->prev = lib;
 		lib->next = vdso;
-		break;
 	}
 
 	/* Initial dso chain consists only of the app. We temporarily
@@ -667,7 +770,7 @@ void *__dynlink(int argc, char **argv)
 	/* PAST THIS POINT, ALL LIBC INTERFACES ARE FULLY USABLE. */
 
 	/* Donate unused parts of app and library mapping to malloc */
-	reclaim_gaps(app->base, (void *)aux[AT_PHDR], aux[AT_PHENT], aux[AT_PHNUM]);
+	reclaim_gaps(app->base, (void *)aux[0], aux[1], aux[2]);
 	ehdr = (void *)lib->base;
 	reclaim_gaps(lib->base, (void *)(lib->base+ehdr->e_phoff),
 		ehdr->e_phentsize, ehdr->e_phnum);
@@ -713,7 +816,7 @@ void *__dynlink(int argc, char **argv)
 	}
 
 	errno = 0;
-	return (void *)aux[AT_ENTRY];
+	return (void *)aux[4];
 }
 
 void *dlopen(const char *file, int mode)
@@ -784,7 +887,7 @@ end:
 static void *do_dlsym(struct dso *p, const char *s, void *ra)
 {
 	size_t i;
-	uint32_t h;
+	uint32_t h = 0, gh = 0;
 	Sym *sym;
 	if (p == RTLD_NEXT) {
 		for (p=head; p && (unsigned char *)ra-p->map>p->map_len; p=p->next);
@@ -798,12 +901,28 @@ static void *do_dlsym(struct dso *p, const char *s, void *ra)
 		if (!res) goto failed;
 		return res;
 	}
-	h = hash(s);
-	sym = lookup(s, h, p);
+	
+	if (p->hashtab) {
+		h = sysv_hash(s);
+		sym = sysv_lookup(s, h, p);
+	} else {
+		gh = gnu_hash(s);
+		sym = gnu_lookup(s, gh, p);
+	}
+
 	if (sym && sym->st_value && (1<<(sym->st_info&0xf) & OK_TYPES))
 		return p->base + sym->st_value;
 	if (p->deps) for (i=0; p->deps[i]; i++) {
-		sym = lookup(s, h, p->deps[i]);
+		if (p->deps[i]->hashtab && (h || !p->deps[i]->ghashtab)) {
+			if (!h)
+				h = sysv_hash(s);
+			sym = sysv_lookup(s, h, p->deps[i]);
+		} else {
+			if (!gh)
+				gh = gnu_hash(s);
+			sym = gnu_lookup(s, h, p->deps[i]);
+		}
+
 		if (sym && sym->st_value && (1<<(sym->st_info&0xf) & OK_TYPES))
 			return p->deps[i]->base + sym->st_value;
 	}
@@ -813,6 +932,96 @@ failed:
 	return 0;
 }
 
+struct sym_search {
+	void *addr;
+	Dl_info *info;
+};
+
+static int find_closest_sym (struct dso *dso, Sym *sym, struct sym_search *search)
+{
+	void *symaddr = dso->base + sym->st_value;
+	char *strings = dso->strings;
+	Dl_info *info = search->info;
+	void *addr = search->addr;
+	void *prevaddr = info->dli_saddr;
+
+	if (sym->st_value == 0 && sym->st_shndx == SHN_UNDEF)
+		return 1;
+
+	if (ELF_ST_TYPE(sym->st_info) == STT_TLS)
+		return 1;
+
+	if (addr < symaddr)
+		return 1;
+
+	if (prevaddr && (addr - symaddr) > (addr - prevaddr))
+		return 1;
+
+	info->dli_saddr = symaddr;
+	info->dli_sname = strings + sym->st_name;
+
+	if (addr == symaddr)
+		return 0;
+
+	return 1;
+
+}
+
+static int do_dladdr (void *addr, Dl_info *info)
+{
+	struct sym_search search;
+	struct dso *p;
+	memset (info, 0, sizeof (*info));
+	search.info = info;
+	search.addr = addr;
+	for (p=head; p; p=p->next) {
+		if ((unsigned char *)addr >= p->map && (unsigned char *)addr < p->map + p->map_len) {
+			Sym *syms = p->syms;
+			uint32_t nsym = 0;
+			size_t i;
+			info->dli_fname = p->name;
+			info->dli_fbase = p->base;
+			if (p->hashtab)
+				nsym = p->hashtab[1];
+			else {
+				uint32_t *buckets;
+				buckets = p->ghashtab + 4 + (p->ghashtab[2] * (sizeof(size_t)/sizeof(uint32_t)));
+				syms += p->ghashtab[1];
+				for (i = 0; i < p->ghashtab[0]; ++i) {
+					if (buckets[i] > nsym)
+						nsym = buckets[i];
+				}
+
+				if (nsym) {
+					nsym -= p->ghashtab[1];
+					uint32_t *hashval = buckets + p->ghashtab[0] + nsym;
+					do {
+						nsym++;
+					}while (!(*hashval++ & 1));
+				}
+
+			}
+
+			for (i = 0; i < nsym; i++) {
+				if (!find_closest_sym (p, syms + i, &search))
+					return 1;
+			}
+
+			return 1;
+		}
+	}
+	return 0;
+}
+
+int dladdr (void *addr, Dl_info *info)
+{
+	int res;
+	pthread_rwlock_rdlock(&lock);
+	res = do_dladdr (addr, info);
+	pthread_rwlock_unlock(&lock);
+	return res;
+}
+
 void *__dlsym(void *p, const char *s, void *ra)
 {
 	void *res;