Discussion:
DDB is elf
Martin Pieuchot
2016-01-25 15:10:52 UTC
Permalink
Removes the abstraction layer to support multiple executable binaries.
These days all our architectures are ELF and this would allows us to
reuse the ELF parsing code more easily.

Diff below includes the db_sifting() removal.

ok?

Index: db_elf.c
===================================================================
RCS file: /cvs/src/sys/ddb/db_elf.c,v
retrieving revision 1.15
diff -u -p -r1.15 db_elf.c
--- db_elf.c 25 Jan 2016 14:56:03 -0000 1.15
+++ db_elf.c 25 Jan 2016 15:02:05 -0000
@@ -53,30 +53,6 @@ static char *db_elf_find_linetab(db_symt
#define STAB_TO_EHDR(stab) ((Elf_Ehdr *)((stab)->private))
#define STAB_TO_SHDR(stab, e) ((Elf_Shdr *)((stab)->private + (e)->e_shoff))

-boolean_t db_elf_sym_init(int, void *, void *, const char *);
-db_sym_t db_elf_lookup(db_symtab_t *, char *);
-db_sym_t db_elf_search_symbol(db_symtab_t *, db_addr_t,
- db_strategy_t, db_expr_t *);
-void db_elf_symbol_values(db_symtab_t *, db_sym_t,
- char **, db_expr_t *);
-boolean_t db_elf_line_at_pc(db_symtab_t *, db_sym_t,
- char **, int *, db_expr_t);
-boolean_t db_elf_sym_numargs(db_symtab_t *, db_sym_t, int *,
- char **);
-void db_elf_forall(db_symtab_t *,
- db_forall_func_t db_forall_func, void *);
-
-db_symformat_t db_symformat_elf = {
- "ELF",
- db_elf_sym_init,
- db_elf_lookup,
- db_elf_search_symbol,
- db_elf_symbol_values,
- db_elf_line_at_pc,
- db_elf_sym_numargs,
- db_elf_forall
-};
-
/*
* Find the symbol table and strings; tell ddb about them.
*
@@ -261,7 +237,7 @@ db_elf_find_linetab(db_symtab_t *stab, s
* Lookup the symbol with the given name.
*/
db_sym_t
-db_elf_lookup(db_symtab_t *stab, char *symstr)
+db_elf_sym_lookup(db_symtab_t *stab, char *symstr)
{
Elf_Sym *symp, *symtab_start, *symtab_end;
char *strtab;
@@ -287,7 +263,7 @@ db_elf_lookup(db_symtab_t *stab, char *s
* provided threshold).
*/
db_sym_t
-db_elf_search_symbol(db_symtab_t *symtab, db_addr_t off, db_strategy_t strategy,
+db_elf_sym_search(db_symtab_t *symtab, db_addr_t off, db_strategy_t strategy,
db_expr_t *diffp)
{
Elf_Sym *rsymp, *symp, *symtab_start, *symtab_end;
@@ -350,7 +326,7 @@ db_elf_search_symbol(db_symtab_t *symtab
* Return the name and value for a symbol.
*/
void
-db_elf_symbol_values(db_symtab_t *symtab, db_sym_t sym, char **namep,
+db_elf_sym_values(db_symtab_t *symtab, db_sym_t sym, char **namep,
db_expr_t *valuep)
{
Elf_Sym *symp = (Elf_Sym *)sym;
@@ -396,23 +372,8 @@ db_elf_line_at_pc(db_symtab_t *symtab, d
return (TRUE);
}

-/*
- * Returns the number of arguments to a function and their
- * names if we can find the appropriate debugging symbol.
- */
-boolean_t
-db_elf_sym_numargs(db_symtab_t *symtab, db_sym_t cursym, int *nargp,
- char **argnamep)
-{
-
- /*
- * XXX We don't support this (yet).
- */
- return (FALSE);
-}
-
void
-db_elf_forall(db_symtab_t *stab, db_forall_func_t db_forall_func, void *arg)
+db_elf_sym_forall(db_symtab_t *stab, db_forall_func_t db_forall_func, void *arg)
{
char *strtab;
static char suffix[2];
Index: db_hangman.c
===================================================================
RCS file: /cvs/src/sys/ddb/db_hangman.c,v
retrieving revision 1.32
diff -u -p -r1.32 db_hangman.c
--- db_hangman.c 14 Mar 2015 03:38:46 -0000 1.32
+++ db_hangman.c 25 Jan 2016 15:02:05 -0000
@@ -103,14 +103,14 @@ db_randomsym(size_t *lenp)
stab = &db_symtabs[arc4random_uniform(nsymtabs)];

dfa.cnt = 0;
- X_db_forall(stab, db_hang_forall, &dfa);
+ db_elf_sym_forall(stab, db_hang_forall, &dfa);
nsyms = -dfa.cnt;

if (nsyms == 0)
return (NULL);

dfa.cnt = arc4random_uniform(nsyms);
- X_db_forall(stab, db_hang_forall, &dfa);
+ db_elf_sym_forall(stab, db_hang_forall, &dfa);

q = db_qualify(dfa.sym, stab->name);

Index: db_sym.c
===================================================================
RCS file: /cvs/src/sys/ddb/db_sym.c,v
retrieving revision 1.41
diff -u -p -r1.41 db_sym.c
--- db_sym.c 25 Jan 2016 14:50:13 -0000 1.41
+++ db_sym.c 25 Jan 2016 15:02:05 -0000
@@ -52,30 +52,10 @@ db_symtab_t db_symtabs[MAXNOSYMTABS] = {

db_symtab_t *db_last_symtab;

-static db_forall_func_t db_sift;
-
extern char end[];

-/*
- * Put the most picky symbol table formats at the top!
- */
-const db_symformat_t *db_symformats[] = {
- &db_symformat_elf,
- NULL,
-};
-
-const db_symformat_t *db_symformat;
-
-boolean_t X_db_sym_init(int, void *, void *, const char *);
-db_sym_t X_db_lookup(db_symtab_t *, char *);
-db_sym_t X_db_search_symbol(db_symtab_t *, db_addr_t,
- db_strategy_t, db_expr_t *);
-void X_db_symbol_values(db_symtab_t *, db_sym_t, char **,
- db_expr_t *);
boolean_t X_db_line_at_pc(db_symtab_t *, db_sym_t, char **,
int *, db_expr_t);
-int X_db_sym_numargs(db_symtab_t *, db_sym_t, int *,
- char **);

/*
* Initialize the kernel debugger by initializing the master symbol
@@ -85,7 +65,6 @@ int X_db_sym_numargs(db_symtab_t *, db_
void
ddb_init(void)
{
- const db_symformat_t **symf;
const char *name = "bsd";
extern char *esym;
#if defined(__sparc64__) || defined(__mips__) || defined(__amd64__) || \
@@ -111,15 +90,12 @@ ddb_init(void)
return;
}

- if (xesym != NULL && xesym != xssym)
- for (symf = db_symformats; *symf != NULL; symf++) {
- db_symformat = *symf;
- if (X_db_sym_init((vaddr_t)xesym - (vaddr_t)xssym,
- xssym, xesym, name) == TRUE)
+ if (xesym != NULL && xesym != xssym) {
+ if (db_elf_sym_init((vaddr_t)xesym - (vaddr_t)xssym, xssym,
+ xesym, name) == TRUE)
return;
- }
+ }

- db_symformat = NULL;
printf("[ no symbol table formats found ]\n");
}

@@ -263,7 +239,7 @@ db_lookup(char *symstr)
*/
for (i = symtab_start; i < symtab_end; i++) {
if (db_symtabs[i].name &&
- (sp = X_db_lookup(&db_symtabs[i], symstr))) {
+ (sp = db_elf_sym_lookup(&db_symtabs[i], symstr))) {
db_last_symtab = &db_symtabs[i];
return sp;
}
@@ -271,106 +247,6 @@ db_lookup(char *symstr)
return 0;
}

-/* Private structure for passing args to db_sift() from db_sifting(). */
-struct db_sift_args {
- char *symstr;
- int mode;
-};
-
-/*
- * Does the work of db_sifting(), called once for each
- * symbol via X_db_forall(), prints out symbols matching
- * criteria.
- */
-static void
-db_sift(db_symtab_t *stab, db_sym_t sym, char *name, char *suffix, int prefix,
- void *arg)
-{
- char c, sc;
- char *find, *p;
- size_t len;
- struct db_sift_args *dsa;
-
- dsa = (struct db_sift_args*)arg;
-
- find = dsa->symstr; /* String we're looking for. */
- p = name; /* String we're searching within. */
-
- /* Matching algorithm cribbed from strstr(), which is not
- in the kernel. */
- if ((c = *find++) != 0) {
- len = strlen(find);
- do {
- do {
- if ((sc = *p++) == 0)
- return;
- } while (sc != c);
- } while (strncmp(p, find, len) != 0);
- }
- if (dsa->mode=='F') /* ala ls -F */
- db_printf("%s%s ", name, suffix);
- else
- db_printf("%s ", name);
-}
-
-/*
- * "Sift" for a partial symbol.
- * Named for the Sun OpenPROM command ("sifting").
- * If the symbol has a qualifier (e.g., ux:vm_map),
- * then only the specified symbol table will be searched;
- * otherwise, all symbol tables will be searched..
- *
- * "mode" is how-to-display, set from modifiers.
- */
-void
-db_sifting(char *symstr, int mode)
-{
- char *cp;
- int i;
- int symtab_start = 0;
- int symtab_end = MAXNOSYMTABS;
- struct db_sift_args dsa;
-
- /*
- * Look for, remove, and remember any symbol table specifier.
- */
- for (cp = symstr; *cp; cp++) {
- if (*cp == ':') {
- *cp = '\0';
- for (i = 0; i < MAXNOSYMTABS; i++) {
- if (db_symtabs[i].name &&
- ! strcmp(symstr, db_symtabs[i].name)) {
- symtab_start = i;
- symtab_end = i + 1;
- break;
- }
- }
- *cp = ':';
- if (i == MAXNOSYMTABS) {
- db_error("invalid symbol table name");
- /*NOTREACHED*/
- }
- symstr = cp+1;
- }
- }
-
- /* Pass args to db_sift(). */
- dsa.symstr = symstr;
- dsa.mode = mode;
-
- /*
- * Look in the specified set of symbol tables.
- */
- for (i = symtab_start; i < symtab_end; i++)
- if (db_symtabs[i].name) {
- db_printf("Sifting table %s:\n", db_symtabs[i].name);
- X_db_forall(&db_symtabs[i], db_sift, &dsa);
- }
-
- return;
-}
-
-
/*
* Does this symbol name appear in more than one symbol table?
* Used by db_symbol_values to decide whether to qualify a symbol.
@@ -390,7 +266,7 @@ db_symbol_is_ambiguous(db_sym_t sym)
db_symbol_values(sym, &sym_name, 0);
for (i = 0; i < MAXNOSYMTABS; i++) {
if (db_symtabs[i].name &&
- X_db_lookup(&db_symtabs[i], sym_name)) {
+ db_elf_sym_lookup(&db_symtabs[i], sym_name)) {
if (found_once)
return TRUE;
found_once = TRUE;
@@ -416,7 +292,7 @@ db_search_symbol(db_addr_t val, db_strat
for (i = 0; i < MAXNOSYMTABS; i++) {
if (!db_symtabs[i].name)
continue;
- sym = X_db_search_symbol(&db_symtabs[i], val, strategy, &newdiff);
+ sym = db_elf_sym_search(&db_symtabs[i], val, strategy, &newdiff);
if (newdiff < diff) {
db_last_symtab = &db_symtabs[i];
diff = newdiff;
@@ -440,7 +316,7 @@ db_symbol_values(db_sym_t sym, char **na
return;
}

- X_db_symbol_values(db_last_symtab, sym, namep, &value);
+ db_elf_sym_values(db_last_symtab, sym, namep, &value);

if (db_symbol_is_ambiguous(sym))
*namep = db_qualify(sym, db_last_symtab->name);
@@ -492,8 +368,9 @@ db_printsym(db_expr_t off, db_strategy_t
(*pr)("+%s", db_format(buf, sizeof(buf),
d, DB_FORMAT_R, 1, 0));
}
- if (strategy == DB_STGY_PROC) {
- if (db_line_at_pc(cursym, &filename, &linenum, off))
+ if (strategy != DB_STGY_PROC) {
+ if (db_elf_line_at_pc(db_last_symtab, cursym,
+ &filename, &linenum, off))
(*pr)(" [%s:%d]", filename, linenum);
}
return;
@@ -502,84 +379,4 @@ db_printsym(db_expr_t off, db_strategy_t

(*pr)("%s", db_format(buf, sizeof(buf), off, DB_FORMAT_N, 1, 0));
return;
-}
-
-
-boolean_t
-db_line_at_pc(db_sym_t sym, char **filename, int *linenum, db_expr_t pc)
-{
- return X_db_line_at_pc(db_last_symtab, sym, filename, linenum, pc);
-}
-
-int
-db_sym_numargs(db_sym_t sym, int *nargp, char **argnames)
-{
- return X_db_sym_numargs(db_last_symtab, sym, nargp, argnames);
-}
-
-boolean_t
-X_db_sym_init(int symsize, void *vss, void *vse, const char *name)
-{
-
- if (db_symformat != NULL)
- return ((*db_symformat->sym_init)(symsize, vss, vse, name));
- return (FALSE);
-}
-
-db_sym_t
-X_db_lookup(db_symtab_t *stab, char *symstr)
-{
-
- if (db_symformat != NULL)
- return ((*db_symformat->sym_lookup)(stab, symstr));
- return ((db_sym_t)0);
-}
-
-db_sym_t
-X_db_search_symbol(db_symtab_t *stab, db_addr_t off, db_strategy_t strategy,
- db_expr_t *diffp)
-{
-
- if (db_symformat != NULL)
- return ((*db_symformat->sym_search)(stab, off, strategy,
- diffp));
- return ((db_sym_t)0);
-}
-
-void
-X_db_symbol_values(db_symtab_t *stab, db_sym_t sym, char **namep,
- db_expr_t *valuep)
-{
-
- if (db_symformat != NULL)
- (*db_symformat->sym_value)(stab, sym, namep, valuep);
-}
-
-boolean_t
-X_db_line_at_pc(db_symtab_t *stab, db_sym_t cursym, char **filename,
- int *linenum, db_expr_t off)
-{
-
- if (db_symformat != NULL)
- return ((*db_symformat->sym_line_at_pc)(stab, cursym,
- filename, linenum, off));
- return (FALSE);
-}
-
-boolean_t
-X_db_sym_numargs(db_symtab_t *stab, db_sym_t cursym, int *nargp,
- char **argnamep)
-{
-
- if (db_symformat != NULL)
- return ((*db_symformat->sym_numargs)(stab, cursym, nargp,
- argnamep));
- return (FALSE);
-}
-
-void
-X_db_forall(db_symtab_t *stab, db_forall_func_t db_forall_func, void *arg)
-{
- if (db_symformat != NULL)
- (*db_symformat->sym_forall)(stab, db_forall_func, arg);
}
Index: db_sym.h
===================================================================
RCS file: /cvs/src/sys/ddb/db_sym.h,v
retrieving revision 1.18
diff -u -p -r1.18 db_sym.h
--- db_sym.h 25 Jan 2016 14:30:30 -0000 1.18
+++ db_sym.h 25 Jan 2016 15:02:05 -0000
@@ -74,29 +74,6 @@ typedef int db_strategy_t; /* search st
*/
typedef void (db_forall_func_t)(db_symtab_t *, db_sym_t, char *, char *, int, void *);

-void X_db_forall(db_symtab_t *,
- db_forall_func_t db_forall_func, void *);
-
-/*
- * A symbol table may be in one of many formats. All symbol tables
- * must be of the same format as the master kernel symbol table.
- */
-typedef struct {
- const char *sym_format;
- boolean_t (*sym_init)(int, void *, void *, const char *);
- db_sym_t (*sym_lookup)(db_symtab_t *, char *);
- db_sym_t (*sym_search)(db_symtab_t *, db_addr_t, db_strategy_t,
- db_expr_t *);
- void (*sym_value)(db_symtab_t *, db_sym_t, char **,
- db_expr_t *);
- boolean_t (*sym_line_at_pc)(db_symtab_t *, db_sym_t,
- char **, int *, db_expr_t);
- boolean_t (*sym_numargs)(db_symtab_t *, db_sym_t, int *,
- char **);
- void (*sym_forall)(db_symtab_t *,
- db_forall_func_t *db_forall_func, void *);
-} db_symformat_t;
-
extern boolean_t db_qualify_ambiguous_names;
/* if TRUE, check across symbol tables
* for multiple occurrences of a name.
@@ -120,9 +97,6 @@ int db_value_of_name(char *, db_expr_t *

db_sym_t db_lookup(char *);

-void db_sifting(char *, int);
- /* print partially matching symbol names */
-
boolean_t db_symbol_is_ambiguous(db_sym_t);

db_sym_t db_search_symbol(db_addr_t, db_strategy_t, db_expr_t *);
@@ -142,9 +116,13 @@ void db_symbol_values(db_sym_t, char **,
void db_printsym(db_expr_t, db_strategy_t, int (*)(const char *, ...));
/* print closest symbol to a value */

-boolean_t db_line_at_pc(db_sym_t, char **, int *, db_expr_t);
+#define db_sym_numargs(sym, nargp, argnames) (FALSE)

-int db_sym_numargs(db_sym_t, int *, char **);
char *db_qualify(db_sym_t, const char *);

-extern db_symformat_t db_symformat_elf;
+boolean_t db_elf_sym_init(int, void *, void *, const char *);
+db_sym_t db_elf_sym_lookup(db_symtab_t *, char *);
+void db_elf_sym_values(db_symtab_t *, db_sym_t, char **, db_expr_t *);
+db_sym_t db_elf_sym_search(db_symtab_t *, db_addr_t, db_strategy_t, db_expr_t *);
+boolean_t db_elf_line_at_pc(db_symtab_t *, db_sym_t, char **, int *, db_expr_t);
+void db_elf_sym_forall(db_symtab_t *, db_forall_func_t db_forall_func, void *);
Theo Buehler
2016-01-27 08:38:24 UTC
Permalink
Post by Martin Pieuchot
Removes the abstraction layer to support multiple executable binaries.
These days all our architectures are ELF and this would allows us to
reuse the ELF parsing code more easily.
Diff below includes the db_sifting() removal.
ok?
It would be great if others could help mpi@ move forward with this.
The patch is pretty straightforward and looks ok to me, just one
comment below.
Post by Martin Pieuchot
Index: db_elf.c
===================================================================
Index: db_hangman.c
===================================================================
ok
Post by Martin Pieuchot
Index: db_sym.c
===================================================================
RCS file: /cvs/src/sys/ddb/db_sym.c,v
retrieving revision 1.41
diff -u -p -r1.41 db_sym.c
--- db_sym.c 25 Jan 2016 14:50:13 -0000 1.41
+++ db_sym.c 25 Jan 2016 15:02:05 -0000
@@ -52,30 +52,10 @@ db_symtab_t db_symtabs[MAXNOSYMTABS] = {
db_symtab_t *db_last_symtab;
-static db_forall_func_t db_sift;
-
extern char end[];
-/*
- * Put the most picky symbol table formats at the top!
- */
-const db_symformat_t *db_symformats[] = {
- &db_symformat_elf,
- NULL,
-};
-
-const db_symformat_t *db_symformat;
-
-boolean_t X_db_sym_init(int, void *, void *, const char *);
-db_sym_t X_db_lookup(db_symtab_t *, char *);
-db_sym_t X_db_search_symbol(db_symtab_t *, db_addr_t,
- db_strategy_t, db_expr_t *);
-void X_db_symbol_values(db_symtab_t *, db_sym_t, char **,
- db_expr_t *);
boolean_t X_db_line_at_pc(db_symtab_t *, db_sym_t, char **,
int *, db_expr_t);
-int X_db_sym_numargs(db_symtab_t *, db_sym_t, int *,
- char **);
The X_db_line_at_pc() prototype should also be removed. Otherwise ok
Post by Martin Pieuchot
Index: db_sym.h
===================================================================
ok

Loading...