Re: [PATCH, doc RFA] Make second arg to "mt print *symbols" a regexp

[Doug Evans <dje at google dot com>]

On Tue, Sep 29, 2015 at 4:32 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> This patch was helpful while debugging a gdb crash from the gdb-generated
> .gdb_index addrmap. It just changes the optional second arg of
> "mt print {,p,m}symbols" to be a regexp of a filename instead of the
> filename itself - the regexp is just easier to get right the first time.
>
> Also, I noticed "mt print msymbols" was looping over pspaces.
> The others don't, and for myself I would never want the current
> behaviour. And if I ever did I'd rather have a composable command
> that applied a specific command over all pspaces than building
> this functionality into the command itself!
>
> This patch also does a bit of misc cleanup.
> E.g., DEV_TTY is supposed to be the default output file, but these commands
> do not take a default output file.
>
> 2015-09-29  Doug Evans  <dje@google.com>
>
>         * psymtab.c (DEV_TTY): Delete.
>         (maintenance_print_psymbols): Rewrite argument processing.
>         Treat optional second parameter as a regexp.
>         * symmisc.c (DEV_TTY): Delete.
>         (maintenance_print_symbols): Rewrite argument processing.
>         Treat optional second parameter as a regexp.
>         (maintenance_print_msymbols): Ditto.
>
>         doc/
>         * gdb.texinfo (Symbols): Update docs of mt print symbols, psymbols,
>         and msymbols.
>
> diff --git a/gdb/psymtab.c b/gdb/psymtab.c
> index d565c67..1cce5c4 100644
> --- a/gdb/psymtab.c
> +++ b/gdb/psymtab.c
> @@ -36,10 +36,6 @@
>  #include "cp-support.h"
>  #include "gdbcmd.h"
>
> -#ifndef DEV_TTY
> -#define DEV_TTY "/dev/tty"
> -#endif
> -
>  struct psymbol_bcache
>  {
>    struct bcache *bcache;
> @@ -1890,31 +1886,28 @@ maintenance_print_psymbols (char *args, int
> from_tty)
>    char **argv;
>    struct ui_file *outfile;
>    struct cleanup *cleanups;
> -  char *symname = NULL;
> -  char *filename = DEV_TTY;
> +  char *filename = NULL;
> +  char *print_only = NULL;
>    struct objfile *objfile;
>    struct partial_symtab *ps;
>
>    dont_repeat ();
>
> -  if (args == NULL)
> -    {
> -      error (_("\
> -print-psymbols takes an output file name and optional symbol file name"));
> -    }
>    argv = gdb_buildargv (args);
>    cleanups = make_cleanup_freeargv (argv);
> -
> -  if (argv[0] != NULL)
> +  /* Note: gdb_buildargv cannot currently return { NULL },
> +     but we play it safe just in case that changes.  */
> +  if (argv == NULL || argv[0] == NULL)
>      {
> -      filename = argv[0];
> -      /* If a second arg is supplied, it is a source file name to match on.
> */
> -      if (argv[1] != NULL)
> -       {
> -         symname = argv[1];
> -       }
> +      error (_("\"print psymbols\" takes an output file name"
> +              " and an optional symbol file name"));
>      }
>
> +  filename = argv[0];
> +  /* If a second arg is supplied, it is a source file name to match on.  */
> +  if (argv[1] != NULL)
> +    print_only = argv[1];
> +
>    filename = tilde_expand (filename);
>    make_cleanup (xfree, filename);
>
> @@ -1923,10 +1916,13 @@ print-psymbols takes an output file name and
> optional symbol file name"));
>      perror_with_name (filename);
>    make_cleanup_ui_file_delete (outfile);
>
> +  if (print_only != NULL)
> +    re_comp (print_only);
> +
>    ALL_PSYMTABS (objfile, ps)
>      {
>        QUIT;
> -      if (symname == NULL || filename_cmp (symname, ps->filename) == 0)
> +      if (print_only == NULL || re_exec (ps->filename))
>         dump_psymtab (objfile, ps, outfile);
>      }
>    do_cleanups (cleanups);
> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index 53d8c03..810b60c 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -37,13 +37,8 @@
>  #include "gdbcmd.h"
>  #include "source.h"
>  #include "readline/readline.h"
> -
>  #include "psymtab.h"
>
> -#ifndef DEV_TTY
> -#define DEV_TTY "/dev/tty"
> -#endif
> -
>  /* Unfortunately for debugging, stderr is usually a macro.  This is painful
>     when calling functions that take FILE *'s from the debugger.
>     So we make a variable which has the same value and which is accessible
> when
> @@ -410,45 +405,47 @@ maintenance_print_symbols (char *args, int from_tty)
>    char **argv;
>    struct ui_file *outfile;
>    struct cleanup *cleanups;
> -  char *symname = NULL;
> -  char *filename = DEV_TTY;
> +  char *filename = NULL;
> +  char *print_only = NULL;
>    struct objfile *objfile;
>    struct compunit_symtab *cu;
>    struct symtab *s;
>
>    dont_repeat ();
>
> -  if (args == NULL)
> -    {
> -      error (_("Arguments missing: an output file name "
> -              "and an optional symbol file name"));
> -    }
>    argv = gdb_buildargv (args);
>    cleanups = make_cleanup_freeargv (argv);
> -
> -  if (argv[0] != NULL)
> +  /* Note: gdb_buildargv cannot currently return { NULL },
> +     but we play it safe just in case that changes.  */
> +  if (argv == NULL || argv[0] == NULL)
>      {
> -      filename = argv[0];
> -      /* If a second arg is supplied, it is a source file name to match on.
> */
> -      if (argv[1] != NULL)
> -       {
> -         symname = argv[1];
> -       }
> +      error (_("\"print symbols\" takes an output file name"
> +              " and an optional symbol file name"));
>      }
>
> +  filename = argv[0];
> +  /* If a second arg is supplied, it is a source file name to match on.  */
> +  if (argv[1] != NULL)
> +    print_only = argv[1];
> +
>    filename = tilde_expand (filename);
>    make_cleanup (xfree, filename);
>
>    outfile = gdb_fopen (filename, FOPEN_WT);
> -  if (outfile == 0)
> +  if (outfile == NULL)
>      perror_with_name (filename);
>    make_cleanup_ui_file_delete (outfile);
>
> +  if (print_only != NULL)
> +    re_comp (print_only);
> +
>    ALL_FILETABS (objfile, cu, s)
>      {
>        QUIT;
> -      if (symname == NULL
> -         || filename_cmp (symname, symtab_to_filename_for_display (s)) ==
> 0)
> +      /* Heads up: While we're looping more symtabs can get expanded (e.g.,
> to
> +        process check_typedef).  We'll just ignore them since they're added
> to
> +        the front of the list, which is ok.  */
> +      if (print_only == NULL || re_exec (symtab_to_filename_for_display
> (s)))
>         dump_symtab (objfile, s, outfile);
>      }
>    do_cleanups (cleanups);
> @@ -642,54 +639,44 @@ maintenance_print_msymbols (char *args, int from_tty)
>    char **argv;
>    struct ui_file *outfile;
>    struct cleanup *cleanups;
> -  char *filename = DEV_TTY;
> -  char *symname = NULL;
> -  struct program_space *pspace;
> +  char *filename = NULL;
> +  char *print_only = NULL;
>    struct objfile *objfile;
>
> -  struct stat sym_st, obj_st;
> -
>    dont_repeat ();
>
> -  if (args == NULL)
> -    {
> -      error (_("print-msymbols takes an output file "
> -              "name and optional symbol file name"));
> -    }
>    argv = gdb_buildargv (args);
>    cleanups = make_cleanup_freeargv (argv);
> -
> -  if (argv[0] != NULL)
> +  /* Note: gdb_buildargv cannot currently return { NULL },
> +     but we play it safe just in case that changes.  */
> +  if (argv == NULL || argv[0] == NULL)
>      {
> -      filename = argv[0];
> -      /* If a second arg is supplied, it is a source file name to match on.
> */
> -      if (argv[1] != NULL)
> -       {
> -         symname = gdb_realpath (argv[1]);
> -         make_cleanup (xfree, symname);
> -         if (symname && stat (symname, &sym_st))
> -           perror_with_name (symname);
> -       }
> +      error (_("\"print msymbols\" takes an output file name"
> +              " and an optional objfile name"));
>      }
>
> +  filename = argv[0];
> +  /* If a second arg is supplied, it is a source file name to match on.  */
> +  if (argv[1] != NULL)
> +    print_only = argv[1];
> +
>    filename = tilde_expand (filename);
>    make_cleanup (xfree, filename);
>
>    outfile = gdb_fopen (filename, FOPEN_WT);
> -  if (outfile == 0)
> +  if (outfile == NULL)
>      perror_with_name (filename);
>    make_cleanup_ui_file_delete (outfile);
>
> -  ALL_PSPACES (pspace)
> -    ALL_PSPACE_OBJFILES (pspace, objfile)
> -      {
> -       QUIT;
> -       if (symname == NULL || (!stat (objfile_name (objfile), &obj_st)
> -                               && sym_st.st_dev == obj_st.st_dev
> -                               && sym_st.st_ino == obj_st.st_ino))
> -         dump_msymbols (objfile, outfile);
> -      }
> -  fprintf_filtered (outfile, "\n\n");
> +  if (print_only != NULL)
> +    re_comp (print_only);
> +
> +  ALL_OBJFILES (objfile)
> +  {
> +    QUIT;
> +    if (print_only == NULL || re_exec (objfile_name (objfile)))
> +      dump_msymbols (objfile, outfile);
> +  }
>    do_cleanups (cleanups);
>  }
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f298172..96171b3 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -16809,10 +16809,12 @@ entered from the keyboard causes symbol
> information to be loaded.
>  @cindex partial symbol dump
>  @kindex maint print msymbols
>  @cindex minimal symbol dump
> -@item maint print symbols @var{filename}
> -@itemx maint print psymbols @var{filename}
> -@itemx maint print msymbols @var{filename}
> +@item maint print symbols @var{filename} @r{[} @var{regexp} @r{]}
> +@itemx maint print psymbols @var{filename} @r{[} @var{regexp} @r{]}
> +@itemx maint print msymbols @var{filename} @r{[} @var{regexp} @r{]}
>  Write a dump of debugging symbol data into the file @var{filename}.
> +If @var{regexp} is given then only print symbols from files that match
> +the regular expression.
>  These commands are used to debug the @value{GDBN} symbol-reading code.
> Only
>  symbols with debugging data are included.  If you use @samp{maint print
>  symbols}, @value{GDBN} includes all the symbols for which it has already

Ping on the doc part.