From 9485f4235684b94daa95ac58b794cb1a1d53ebf3 Mon Sep 17 00:00:00 2001 From: ad Date: Sun, 16 Nov 2008 15:13:35 +0000 Subject: [PATCH] - Local symbols could shadow globals in some instances. Fix it. - mutex_enter() from ksyms_getval() could panic due to a change made in revision 1.40. Fix it. - Replace the p-tree with a binary search of global symbols. Saves about 250kB of wired memory on i386 and allows for faster lookups within module symbol tables. --- sys/kern/kern_ksyms.c | 276 +++++++++++++++--------------------------- sys/sys/ksyms.h | 3 +- 2 files changed, 102 insertions(+), 177 deletions(-) diff --git a/sys/kern/kern_ksyms.c b/sys/kern/kern_ksyms.c index b6e09a9f47ec..ce4c63bc1b02 100644 --- a/sys/kern/kern_ksyms.c +++ b/sys/kern/kern_ksyms.c @@ -1,4 +1,4 @@ -/* $NetBSD: kern_ksyms.c,v 1.42 2008/11/12 12:36:16 ad Exp $ */ +/* $NetBSD: kern_ksyms.c,v 1.43 2008/11/16 15:13:35 ad Exp $ */ /*- * Copyright (c) 2008 The NetBSD Foundation, Inc. @@ -67,14 +67,11 @@ /* * TODO: * - * Consider replacing patricia tree with simpler binary search - * for symbol tables. - * * Add support for mmap, poll. */ #include -__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.42 2008/11/12 12:36:16 ad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.43 2008/11/16 15:13:35 ad Exp $"); #ifdef _KERNEL #include "opt_ddb.h" @@ -134,21 +131,6 @@ TAILQ_HEAD(, ksyms_symtab) ksyms_symtabs = TAILQ_HEAD_INITIALIZER(ksyms_symtabs); static struct ksyms_symtab kernel_symtab; -/* - * Patricia-tree-based lookup structure for the in-kernel global symbols. - * Based on a design by Mikael Sundstrom, msm@sm.luth.se. - */ -struct ptree { - int16_t bitno; - int16_t lr[2]; -} *symb; -static int16_t baseidx; -static int treex = 1; - -#define P_BIT(key, bit) ((key[bit >> 3] >> (bit & 7)) & 1) -#define STRING(idx) (kernel_symtab.sd_symstart[idx].st_name + \ - kernel_symtab.sd_strstart) - static int ksyms_verify(void *symstart, void *strstart) { @@ -166,146 +148,54 @@ ksyms_verify(void *symstart, void *strstart) return 1; } -/* - * Walk down the tree until a terminal node is found. - */ -static int -symbol_traverse(const char *key) -{ - int16_t nb, rbit = baseidx; - - while (rbit > 0) { - nb = symb[rbit].bitno; - rbit = symb[rbit].lr[P_BIT(key, nb)]; - } - return -rbit; -} - -static int -ptree_add(char *key, int val) -{ - int idx; - int nix, cix, bit, rbit, sb, lastrbit, svbit = 0, ix; - char *m, *k; - - if (baseidx == 0) { - baseidx = -val; - return 0; /* First element */ - } - - /* Get string to match against */ - idx = symbol_traverse(key); - - /* Find first mismatching bit */ - m = STRING(idx); - k = key; - if (strcmp(m, k) == 0) - return 1; - - for (cix = 0; *m && *k && *m == *k; m++, k++, cix += 8) - ; - ix = ffs((int)*m ^ (int)*k) - 1; - cix += ix; - - /* Create new node */ - nix = treex++; - bit = P_BIT(key, cix); - symb[nix].bitno = cix; - symb[nix].lr[bit] = -val; - - /* Find where to insert node */ - rbit = baseidx; - lastrbit = 0; - for (;;) { - if (rbit < 0) - break; - sb = symb[rbit].bitno; - if (sb > cix) - break; - if (sb == cix) - printf("symb[rbit].bitno == cix!!!\n"); - lastrbit = rbit; - svbit = P_BIT(key, sb); - rbit = symb[rbit].lr[svbit]; - } - - /* Do the actual insertion */ - if (lastrbit == 0) { - /* first element */ - symb[nix].lr[!bit] = baseidx; - baseidx = nix; - } else { - symb[nix].lr[!bit] = rbit; - symb[lastrbit].lr[svbit] = nix; - } - return 0; -} - -static int -ptree_find(const char *key) -{ - int idx; - - if (baseidx == 0) - return 0; - idx = symbol_traverse(key); - - if (strcmp(key, STRING(idx)) == 0) - return idx; - return 0; -} - -static void -ptree_gen(char *off, struct ksyms_symtab *tab) -{ - Elf_Sym *sym; - int i, nsym; - - if (off != NULL) - symb = (struct ptree *)ALIGN(off); - else - symb = malloc((tab->sd_symsize/sizeof(Elf_Sym)) * - sizeof(struct ptree), M_DEVBUF, M_WAITOK); - symb--; /* sym index won't be 0 */ - - sym = tab->sd_symstart; - if ((nsym = tab->sd_symsize/sizeof(Elf_Sym)) > INT16_MAX) { - printf("Too many symbols for tree, skipping %d symbols\n", - nsym-INT16_MAX); - nsym = INT16_MAX; - } - for (i = 1; i < nsym; i++) { - if (ELF_ST_BIND(sym[i].st_info) != STB_GLOBAL) - continue; - ptree_add(tab->sd_strstart+sym[i].st_name, i); - if (tab->sd_minsym == NULL || - sym[i].st_value < tab->sd_minsym->st_value) - tab->sd_minsym = &sym[i]; - if (tab->sd_maxsym == NULL || - sym[i].st_value > tab->sd_maxsym->st_value) - tab->sd_maxsym = &sym[i]; - } -} - /* * Finds a certain symbol name in a certain symbol table. */ static Elf_Sym * -findsym(const char *name, struct ksyms_symtab *table) +findsym(const char *name, struct ksyms_symtab *table, int type) { - Elf_Sym *start = table->sd_symstart; - int i, sz = table->sd_symsize/sizeof(Elf_Sym); - char *np; - char *realstart = table->sd_strstart - table->sd_usroffset; + Elf_Sym *sym, *maxsym; + int low, mid, high, nglob; + char *str, *cmp; - if (table == &kernel_symtab && (i = ptree_find(name)) != 0) - return &start[i]; + sym = table->sd_symstart; + str = table->sd_strstart - table->sd_usroffset; + nglob = table->sd_nglob; + low = 0; + high = nglob; - for (i = 0; i < sz; i++) { - np = realstart + start[i].st_name; - if (name[0] == np[0] && name[1] == np[1] && - strcmp(name, np) == 0) - return &start[i]; + /* + * Start with a binary search of all global symbols in this table. + * Global symbols must have unique names. + */ + while (low < high) { + mid = (low + high) >> 1; + cmp = sym[mid].st_name + str; + if (cmp[0] < name[0] || strcmp(cmp, name) < 0) { + low = mid + 1; + } else { + high = mid; + } + } + KASSERT(low == high); + if (__predict_true(low < nglob && + strcmp(sym[low].st_name + str, name) == 0)) { + KASSERT(ELF_ST_BIND(sym[low].st_info) == STB_GLOBAL); + return &sym[low]; + } + + /* + * Perform a linear search of local symbols (rare). Many local + * symbols with the same name can exist so are not included in + * the binary search. + */ + if (type != KSYMS_EXTERN) { + maxsym = sym + table->sd_symsize / sizeof(Elf_Sym); + for (sym += nglob; sym < maxsym; sym++) { + if (strcmp(name, sym->st_name + str) == 0) { + return sym; + } + } } return NULL; } @@ -317,8 +207,6 @@ void ksymsattach(int arg) { - if (baseidx == 0) - ptree_gen(0, &kernel_symtab); } /* @@ -334,13 +222,41 @@ ksymsattach(int arg) * newstart - Address to which the symbol table has to be copied during * shrinking. If NULL, it is not moved. */ +static const char *addsymtab_strstart; + +static int +addsymtab_compar(const void *a, const void *b) +{ + const Elf_Sym *sa, *sb; + + sa = a; + sb = b; + + /* + * Split the symbol table into two, with globals at the start + * and locals at the end. + */ + if (ELF_ST_BIND(sa->st_info) != ELF_ST_BIND(sb->st_info)) { + if (ELF_ST_BIND(sa->st_info) == STB_GLOBAL) { + return -1; + } + if (ELF_ST_BIND(sb->st_info) == STB_GLOBAL) { + return 1; + } + } + + /* Within each band, sort by name. */ + return strcmp(sa->st_name + addsymtab_strstart, + sb->st_name + addsymtab_strstart); +} + static void addsymtab(const char *name, void *symstart, size_t symsize, void *strstart, size_t strsize, struct ksyms_symtab *tab, void *newstart) { Elf_Sym *sym, *nsym; - int i, j, n; + int i, j, n, nglob; char *str; tab->sd_symstart = symstart; @@ -362,6 +278,7 @@ addsymtab(const char *name, void *symstart, size_t symsize, sym = tab->sd_symstart; nsym = (Elf_Sym *)newstart; str = tab->sd_strstart; + nglob = 0; for (i = n = 0; i < tab->sd_symsize/sizeof(Elf_Sym); i++) { /* * Remove useless symbols. @@ -384,14 +301,30 @@ addsymtab(const char *name, void *symstart, size_t symsize, /* Save symbol. Set it as an absolute offset */ nsym[n] = sym[i]; nsym[n].st_shndx = SHN_ABS; - j = strlen(nsym[n].st_name + tab->sd_strstart) + 1; + j = strlen(nsym[n].st_name + str) + 1; if (j > ksyms_maxlen) ksyms_maxlen = j; - n++; + nglob += (ELF_ST_BIND(nsym[n].st_info) == STB_GLOBAL); + /* Compute min and max symbols. */ + if (tab->sd_minsym == NULL || + tab->sd_minsym->st_value > nsym[n].st_value) { + tab->sd_minsym = &nsym[n]; + } + if (tab->sd_maxsym == NULL || + tab->sd_maxsym->st_value < nsym[n].st_value) { + tab->sd_maxsym = &nsym[n]; + } + n++; } + + /* Fill the rest of the record, and sort the symbols. */ tab->sd_symstart = nsym; tab->sd_symsize = n * sizeof(Elf_Sym); + tab->sd_nglob = nglob; + addsymtab_strstart = str; + qsort(nsym, n, sizeof(Elf_Sym), addsymtab_compar); + /* ksymsread() is unlocked, so membar. */ membar_producer(); TAILQ_INSERT_TAIL(&ksyms_symtabs, tab, sd_queue); @@ -514,9 +447,6 @@ ksyms_getval_unlocked(const char *mod, const char *sym, unsigned long *val, struct ksyms_symtab *st; Elf_Sym *es; - if (!ksyms_initted) - return ENOENT; - #ifdef KSYMS_DEBUG if (ksyms_debug & FOLLOW_CALLS) printf("ksyms_getval_unlocked: mod %s sym %s valp %p\n", @@ -524,23 +454,14 @@ ksyms_getval_unlocked(const char *mod, const char *sym, unsigned long *val, #endif TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) { - if (st->sd_gone) + if (__predict_false(st->sd_gone)) continue; - if (mod && strcmp(st->sd_name, mod)) + if (mod != NULL && strcmp(st->sd_name, mod)) continue; - if ((es = findsym(sym, st)) == NULL) - continue; - if (es->st_shndx == SHN_UNDEF) - continue; - - /* Skip if bad binding */ - if (type == KSYMS_EXTERN && - ELF_ST_BIND(es->st_info) != STB_GLOBAL) - continue; - - if (val) + if ((es = findsym(sym, st, type)) != NULL) { *val = es->st_value; - return 0; + return 0; + } } return ENOENT; } @@ -550,6 +471,9 @@ ksyms_getval(const char *mod, const char *sym, unsigned long *val, int type) { int rc; + if (!ksyms_initted) + return ENOENT; + mutex_enter(&ksyms_lock); rc = ksyms_getval_unlocked(mod, sym, val, type); mutex_exit(&ksyms_lock); @@ -962,7 +886,7 @@ ksymsioctl(dev_t dev, u_long cmd, void *data, int fflag, struct lwp *l) TAILQ_FOREACH(st, &ksyms_symtabs, sd_queue) { if (st->sd_gone) continue; - if ((sym = findsym(str, st)) == NULL) + if ((sym = findsym(str, st, KSYMS_ANY)) == NULL) continue; #ifdef notdef /* Skip if bad binding */ diff --git a/sys/sys/ksyms.h b/sys/sys/ksyms.h index 4542e303d90e..32d61a2c74e9 100644 --- a/sys/sys/ksyms.h +++ b/sys/sys/ksyms.h @@ -1,4 +1,4 @@ -/* $NetBSD: ksyms.h,v 1.18 2008/11/12 12:36:28 ad Exp $ */ +/* $NetBSD: ksyms.h,v 1.19 2008/11/16 15:13:35 ad Exp $ */ /* * Copyright (c) 2001, 2003 Anders Magnusson (ragge@ludd.luth.se). @@ -45,6 +45,7 @@ struct ksyms_symtab { int sd_usroffset; /* Real address for userspace */ int sd_symsize; /* Size in bytes of symbol table */ int sd_strsize; /* Size of string table */ + int sd_nglob; /* Number of global symbols */ bool sd_gone; /* dead but around for open() */ };