[Crash-utility] [PATCH 2/2] dis: Report duplicate symbols

[Dave Anderson]

----- Original Message -----
> 
> 
> ----- 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

Also, is it worth worrying about non-text symbols like the "(b) watchdog" above?
Maybe the patch should ignore symbols whose sp->type is not (t) or (T)?

Dave


> > 
> > - 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 at redhat.com>
> 
> 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 at redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> > 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>