----- Original Message ----- > Notify the user if there is more than one symbol of the given target. > > For example, after this patch: > > - The 'dis' command now reports that more than one symbol of "watchdog" > exists, to allow the user to be more specific > > crash> dis watchdog > duplicate symbols found: watchdog > ffffffff810e6100 (t) watchdog > /usr/src/debug/kernel-2.6.32-431.1.2.el6/linux-2.6.32-431.1.2.el6.x86_64/kernel/hung_task.c: > 199 > ffffffff810e6990 (t) watchdog > /usr/src/debug/kernel-2.6.32-431.1.2.el6/linux-2.6.32-431.1.2.el6.x86_64/kernel/watchdog.c: > 332 > ffffffff81eb3fc8 (b) watchdog

> > - Choose to disassemble the watchdog() function defined in > kernel/watchdog.c by using the address of the first function > > crash> dis ffffffff810e6990 > 0xffffffff810e6990 <watchdog>: push %rbp > 0xffffffff810e6991 <watchdog+1>: mov %rsp,%rbp > 0xffffffff810e6994 <watchdog+4>: push %r13 > 0xffffffff810e699d <watchdog+13>: nopl 0x0(%rax,%rax,1) > ... > > Signed-off-by: Aaron Tomlin <atomlin(a)redhat.com&gt; Hi Aaron, Good plan. But is there any particular reason for adding "show_flags"? Since the symbol passed to show_symbol() will never have an offset, it doesn't make any difference to save and use the incoming radix, as opposed to the -x|d override, or for that matter, nothing. Dave > --- > defs.h | 1 + > kernel.c | 28 ++++++++++++++++++++++++---- > symbols.c | 3 +-- > 3 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/defs.h b/defs.h > index f21137d..26aa963 100644 > --- a/defs.h > +++ b/defs.h > @@ -4585,6 +4585,7 @@ void show_symbol(struct syment *, ulong, ulong); > #define SHOW_DEC_OFFS (0x8) > #define SHOW_RADIX() (*gdb_output_radix == 16 ? SHOW_HEX_OFFS : > SHOW_DEC_OFFS) > #define SHOW_MODULE (0x10) > +int symbol_name_count(char *); > int symbol_query(char *, char *, struct syment **); > struct syment *next_symbol(char *, struct syment *); > struct syment *prev_symbol(char *, struct syment *); > diff --git a/kernel.c b/kernel.c > index 628779c..b87d9ec 100644 > --- a/kernel.c > +++ b/kernel.c > @@ -1339,6 +1339,7 @@ cmd_dis(void) > ulong revtarget; > ulong count; > ulong offset; > + ulong show_flags; > struct syment *sp; > struct gnu_request *req; > char *savename; > @@ -1362,6 +1363,7 @@ cmd_dis(void) > sp = NULL; > unfiltered = user_mode = do_machdep_filter = do_load_module_filter = 0; > radix = 0; > + show_flags = SHOW_LINENUM | SHOW_RADIX(); > > req = (struct gnu_request *)getbuf(sizeof(struct gnu_request)); > req->buf = GETBUF(BUFSIZE); > @@ -1425,7 +1427,28 @@ cmd_dis(void) > radix = pc->output_radix; > > if (args[optind]) { > - if (can_eval(args[optind])) > + if ((sp = symbol_search(args[optind])) && > + ((symbol_name_count(sp->name)) > 1)) { > + fprintf(fp, "duplicate symbols found: %s\n", > + args[optind]); > + > + do { > + if (module_symbol(sp->value, NULL, NULL, > + NULL, 0)) > + show_symbol(sp, 0, show_flags|SHOW_MODULE); > + else > + show_symbol(sp, 0, show_flags); > + > + } while ((sp = symbol_search_next(sp->name, sp))); > + > + FREEBUF(req->buf); > + FREEBUF(req); > + return; > + } > + if (sp) { > + req->addr = sp->value; > + req->flags |= GNU_FUNCTION_ONLY; > + } else if (can_eval(args[optind])) > req->addr = eval(args[optind], FAULT_ON_ERROR, > NULL); > else if (hexadecimal(args[optind], 0)) { > req->addr = htol(args[optind], FAULT_ON_ERROR, > NULL); > @@ -1438,9 +1461,6 @@ cmd_dis(void) > } > if (!offset) > req->flags |= GNU_FUNCTION_ONLY; > - } else if ((sp = symbol_search(args[optind]))) { > - req->addr = sp->value; > - req->flags |= GNU_FUNCTION_ONLY; > } else { > fprintf(fp, "symbol not found: %s\n", > args[optind]); > fprintf(fp, "possible alternatives:\n"); > diff --git a/symbols.c b/symbols.c > index 48d9f58..958181a 100644 > --- a/symbols.c > +++ b/symbols.c > @@ -37,7 +37,6 @@ static asection *get_kernel_section(char *); > static char * get_section(ulong vaddr, char *buf); > static void symbol_dump(ulong, char *); > static void check_for_dups(struct load_module *); > -static int symbol_name_count(char *); > static struct syment *kallsyms_module_symbol(struct load_module *, > symbol_info *); > static void store_load_module_symbols \ > (bfd *, int, void *, long, uint, ulong, char *); > @@ -4156,7 +4155,7 @@ symbol_search(char *s) > /* > * Count the number of instances of a symbol name. > */ > -static int > +int > symbol_name_count(char *s) > { > int i; > -- > 2.4.3 > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility

----- Original Message ----- > Notify the user if there is more than one symbol of the given target. > > For example, after this patch: > > - The 'dis' command now reports that more than one symbol of "watchdog" > exists, to allow the user to be more specific > > crash> dis watchdog > duplicate symbols found: watchdog > ffffffff810e6100 (t) watchdog > /usr/src/debug/kernel-2.6.32-431.1.2.el6/linux-2.6.32-431.1.2.el6.x86_64/kernel/hung_task.c: > 199 > ffffffff810e6990 (t) watchdog > /usr/src/debug/kernel-2.6.32-431.1.2.el6/linux-2.6.32-431.1.2.el6.x86_64/kernel/watchdog.c: > 332 > ffffffff81eb3fc8 (b) watchdog > > - Choose to disassemble the watchdog() function defined in > kernel/watchdog.c by using the address of the first function > > crash> dis ffffffff810e6990 > 0xffffffff810e6990 <watchdog>: push %rbp > 0xffffffff810e6991 <watchdog+1>: mov %rsp,%rbp > 0xffffffff810e6994 <watchdog+4>: push %r13 > 0xffffffff810e699d <watchdog+13>: nopl 0x0(%rax,%rax,1) > ... > > Signed-off-by: Aaron Tomlin <atomlin(a)redhat.com&gt; Hi Aaron, Good plan. But is there any particular reason for adding "show_flags"? Since the symbol passed to show_symbol() will never have an offset, it doesn't make any difference to save and use the incoming radix, as opposed to the -x|d override, or for that matter, nothing.

On Wed 2015-07-08 17:16 -0400, Dave Anderson wrote: > > Hi Aaron, > > I was revisiting your v1 patch, and realized that the duplicate symbol check > also needs to apply to the "can_eval()" section as well, because a user could > enter "dis symbol+<offset>". > > So I extracted your do-while loop into a discrete function, and beefed it > up to handle the entry of symbols with multiple text and data symbols, as > well as some other possibilities. In so doing, things got a bit murkier > than your original patch. I've attached what I've come up with so far. > Tell me what you think. > > Thanks, > Dave Hi Dave, The changes seem great - thanks for this! I'd like to propose the following on top of what you've done so far: diff --git a/defs.h b/defs.h index ac17455..cc63de8 100644 --- a/defs.h +++ b/defs.h @@ -4542,6 +4542,7 @@ void symtab_init(void); char *check_specified_kernel_debug_file(void); void no_debugging_data(int); void get_text_init_space(void); +int is_symbol_text(struct syment *); int is_kernel_text(ulong); int is_kernel_data(ulong); int is_init_data(ulong value); diff --git a/kernel.c b/kernel.c index 7331a5b..9a3f14d 100644 --- a/kernel.c +++ b/kernel.c @@ -1347,7 +1347,7 @@ resolve_text_symbol(char *arg, struct syment *sp_in, struct gnu_request *req, in sp = sp_orig; do { - if ((sp->type == 't') || (sp->type == 'T')) { + if (is_symbol_text(sp)) { if (!first_text) first_text = sp; text_symbols++; @@ -1383,7 +1383,7 @@ resolve_text_symbol(char *arg, struct syment *sp_in, struct gnu_request *req, in count = 0; sp = sp_orig; do { - if ((sp->type == 't') || (sp->type == 'T')) { + if (is_symbol_text(sp)) { if (++count == 1) fprintf(fp, "duplicate text symbols found: %s\n", diff --git a/symbols.c b/symbols.c index b83b8b5..e190f24 100644 --- a/symbols.c +++ b/symbols.c @@ -2672,6 +2672,14 @@ compare_mods(const void *v1, const void *v2) lm1->mod_base == lm2->mod_base ? 0 : 1); } +int +is_symbol_text(struct syment *sp) +{ + if ((sp->type == 'T') || (sp->type == 't')) + return TRUE; + else + return FALSE; +} /* * Check whether a value falls into a text-type (SEC_CODE) section.

If no count is not entered, and the target address is the starting address of a function, disassemble the entire function. However this is only true, if the given target is symbolic.

This patch simply removes that restriction when the target is a hexadecimal address: For example, each of the following commands will return the same result. - Where 0xffffffff80137e9d is assumed to be the address of the first instruction in do_fork() crash> dis 0xffffffff80137e9d crash> dis do_fork Signed-off-by: Aaron Tomlin <atomlin(a)redhat.com&gt; --- kernel.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel.c b/kernel.c index 4932282..628779c 100644 --- a/kernel.c +++ b/kernel.c @@ -1436,6 +1436,8 @@ cmd_dis(void) req->addr); unfiltered = TRUE; } + if (!offset) + req->flags |= GNU_FUNCTION_ONLY; } else if ((sp = symbol_search(args[optind]))) { req->addr = sp->value; req->flags |= GNU_FUNCTION_ONLY; -- 2.4.3 -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility