Re: [Patch v3] Add -file-list-shared-libraries MI command

["Marc-André Laperle" <marc-andre.laperle@ericsson.com>]

This patch should address all previous comments.

Regards,
Marc-André

On 2017-02-28 05:07 PM, Marc-Andre Laperle wrote:
> This change adds the MI equivalent for the "info sharedlibrary"
> command. The command was already partially documented but ignored as
> it was not implemented. The new MI command works similarly to the CLI
> command, taking an optional regular expression as an argument and
> outputting the library information.
>
> I included a test for the new command in mi-solib.exp.
>
> gdb/doc/ChangeLog:
>
>          * gdb.texinfo (gdb/mi Symbol Query Commands): Document new MI
>          command file-list-shared-libraries
>          (GDB/MI Async Records): Update documentation of library-loaded with new
>          field.
>
> gdb/ChangeLog:
>
>          * NEWS: Add an entry about new '-file-list-shared-libraries' command.
>          * mi/mi-cmd-file.c (mi_cmd_file_list_shared_libraries):
>          New function definition.
>          * mi/mi-cmds.c (mi_cmds): Add -file-list-shared-libraries command.
>          * mi/mi-cmds.h (mi_cmd_file_list_shared_libraries):
>          New function declaration.
>          * mi/mi-interp.c (mi_output_solib_attribs): New Function.
>          * mi/mi-interp.h: New file.
>          * solib.c (info_sharedlibrary_command): Replace for loop with
>          ALL_SO_LIBS macro
>          * solib.h (update_solib_list): New function declaration.
>          (so_list_head): Move macro.
>          * solist.h (ALL_SO_LIBS): New macro.
>
> gdb/testsuite/ChangeLog:
>
>          * gdb.mi/mi-solib.exp (test_file_list_shared_libraries):
>          New procedure.
>
> Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
> ---
>   gdb/NEWS                          |  4 +++
>   gdb/doc/gdb.texinfo               | 34 ++++++++++++++++++-----
>   gdb/mi/mi-cmd-file.c              | 57 +++++++++++++++++++++++++++++++++++++++
>   gdb/mi/mi-cmds.c                  |  2 ++
>   gdb/mi/mi-cmds.h                  |  1 +
>   gdb/mi/mi-interp.c                | 36 +++++++++++++++++++------
>   gdb/mi/mi-interp.h                | 27 +++++++++++++++++++
>   gdb/solib.c                       | 21 ++-------------
>   gdb/solib.h                       | 21 +++++++++++++++
>   gdb/solist.h                      | 10 ++++++-
>   gdb/testsuite/gdb.mi/mi-solib.exp | 49 ++++++++++++++++++++++-----------
>   11 files changed, 212 insertions(+), 50 deletions(-)
>   create mode 100644 gdb/mi/mi-interp.h
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 21e8cd3..a5c5e0d 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -68,6 +68,10 @@ flash-erase
>     Erases all the flash memory regions reported by the target.  This is
>     equivalent to the CLI command flash-erase.
>   
> +-file-list-shared-libraries
> +  List the shared libraries in the program.  This is
> +  equivalent to the CLI command "info shared".
> +
>   *** Changes in GDB 7.12
>   
>   * GDB and GDBserver now build with a C++ compiler by default.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b9b4c82..097a608 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26545,8 +26545,8 @@ that thread.
>   
>   @item =library-loaded,...
>   Reports that a new library file was loaded by the program.  This
> -notification has 4 fields---@var{id}, @var{target-name},
> -@var{host-name}, and @var{symbols-loaded}.  The @var{id} field is an
> +notification has 5 fields---@var{id}, @var{target-name},
> +@var{host-name}, @var{symbols-loaded} and @var{ranges}.  The @var{id} field is an
>   opaque identifier of the library.  For remote debugging case,
>   @var{target-name} and @var{host-name} fields give the name of the
>   library file on the target, and on the host respectively.  For native
> @@ -26556,7 +26556,8 @@ and should not be relied on to convey any useful information.  The
>   @var{thread-group} field, if present, specifies the id of the thread
>   group in whose context the library was loaded.  If the field is
>   absent, it means the library was loaded in the context of all present
> -thread groups.
> +thread groups. The @var{ranges} field specifies the ranges of addresses belonging
> +to this library.
>   
>   @item =library-unloaded,...
>   Reports that a library was unloaded by the program.  This notification
> @@ -31477,26 +31478,45 @@ The @value{GDBN} equivalent is @samp{info sources}.
>   (gdb)
>   @end smallexample
>   
> -@ignore
>   @subheading The @code{-file-list-shared-libraries} Command
>   @findex -file-list-shared-libraries
>   
>   @subsubheading Synopsis
>   
>   @smallexample
> - -file-list-shared-libraries
> + -file-list-shared-libraries [ @var{regexp} ]
>   @end smallexample
>   
>   List the shared libraries in the program.
> +With a regular expression @var{regexp}, only those libraries whose
> +names match @var{regexp} are listed.
>   
>   @subsubheading @value{GDBN} Command
>   
> -The corresponding @value{GDBN} command is @samp{info shared}.
> +The corresponding @value{GDBN} command is @samp{info shared}. The fields
> +have a similar meaning than the @code{=library-loaded} notification.
> +The @code{ranges} field specifies the muliple segments belonging to this
> +library.  Each range has the following fields:
> +
> +@table @samp
> +@item from
> +The address defining the inclusive lower bound of the segment.
> +@item to
> +The address defining the exclusive upper bound of the segment.
> +@end table
>   
>   @subsubheading Example
> -N.A.
> +@smallexample
> +(gdb)
> +-file-list-exec-source-files
> +^done,shared-libraries=[
> +@{id="/lib/libfoo.so",target-name="/lib/libfoo.so",host-name="/lib/libfoo.so",symbols-loaded="1",thread-group="i1",ranges=[@{from="0x72815989",to="0x728162c0"@}]@},
> +@{id="/lib/libbar.so",target-name="/lib/libbar.so",host-name="/lib/libbar.so",symbols-loaded="1",thread-group="i1",ranges=[@{from="0x76ee48c0",to="0x76ee9160"@}]@}]
> +(gdb)
> +@end smallexample
>   
>   
> +@ignore
>   @subheading The @code{-file-list-symbol-files} Command
>   @findex -file-list-symbol-files
>   
> diff --git a/gdb/mi/mi-cmd-file.c b/gdb/mi/mi-cmd-file.c
> index 71e2845..a2ad392 100644
> --- a/gdb/mi/mi-cmd-file.c
> +++ b/gdb/mi/mi-cmd-file.c
> @@ -20,11 +20,15 @@
>   #include "defs.h"
>   #include "mi-cmds.h"
>   #include "mi-getopt.h"
> +#include "mi-interp.h"
>   #include "ui-out.h"
>   #include "symtab.h"
>   #include "source.h"
>   #include "objfiles.h"
>   #include "psymtab.h"
> +#include "solib.h"
> +#include "solist.h"
> +#include "gdb_regex.h"
>   
>   /* Return to the client the absolute path and line number of the
>      current file being executed.  */
> @@ -106,3 +110,56 @@ mi_cmd_file_list_exec_source_files (char *command, char **argv, int argc)
>   
>     uiout->end (ui_out_type_list);
>   }
> +
> +/* See mi-cmds.h.  */
> +
> +void
> +mi_cmd_file_list_shared_libraries (char *command, char **argv, int argc)
> +{
> +  struct ui_out *uiout = current_uiout;
> +  const char *pattern;
> +  struct so_list *so = NULL;
> +  struct gdbarch *gdbarch = target_gdbarch ();
> +
> +  switch (argc)
> +    {
> +    case 0:
> +      pattern = NULL;
> +      break;
> +    case 1:
> +      pattern = argv[0];
> +      break;
> +    default:
> +      error (_("Usage: -file-list-shared-libraries [REGEXP]"));
> +    }
> +
> +  if (pattern != NULL)
> +    {
> +      const char *re_err = re_comp (pattern);
> +
> +      if (re_err != NULL)
> +	error (_("Invalid regexp: %s"), re_err);
> +    }
> +
> +  update_solib_list (1);
> +
> +  /* Print the table header.  */
> +  struct cleanup *cleanup
> +    = make_cleanup_ui_out_list_begin_end (uiout, "shared-libraries");
> +
> +  ALL_SO_LIBS (so)
> +    {
> +      if (so->so_name[0] == '\0')
> +	continue;
> +      if (pattern != NULL && !re_exec (so->so_name))
> +	continue;
> +
> +      struct cleanup *tuple_clean_up
> +        = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
> +      mi_output_solib_attribs (uiout, so);
> +
> +      do_cleanups (tuple_clean_up);
> +    }
> +
> +  do_cleanups (cleanup);
> +}
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index abb70bd..b7494ce 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -115,6 +115,8 @@ static struct mi_cmd mi_cmds[] =
>   		 mi_cmd_file_list_exec_source_file),
>     DEF_MI_CMD_MI ("file-list-exec-source-files",
>   		 mi_cmd_file_list_exec_source_files),
> +  DEF_MI_CMD_MI ("file-list-shared-libraries",
> +		 mi_cmd_file_list_shared_libraries),
>     DEF_MI_CMD_CLI ("file-symbol-file", "symbol-file", 1),
>     DEF_MI_CMD_MI ("gdb-exit", mi_cmd_gdb_exit),
>     DEF_MI_CMD_CLI_1 ("gdb-set", "set", 1,
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index d0906e6..fcadfff 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -70,6 +70,7 @@ extern mi_cmd_argv_ftype mi_cmd_exec_step;
>   extern mi_cmd_argv_ftype mi_cmd_exec_step_instruction;
>   extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_file;
>   extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_files;
> +extern mi_cmd_argv_ftype mi_cmd_file_list_shared_libraries;
>   extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
>   extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
>   extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index aa76989..1a26818 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -1119,6 +1119,33 @@ mi_on_resume (ptid_t ptid)
>       }
>   }
>   
> +/* See mi-interp.h.  */
> +
> +void
> +mi_output_solib_attribs (ui_out *uiout, struct so_list *solib)
> +{
> +  struct gdbarch *gdbarch = target_gdbarch ();
> +
> +  uiout->field_string ("id", solib->so_original_name);
> +  uiout->field_string ("target-name", solib->so_original_name);
> +  uiout->field_string ("host-name", solib->so_name);
> +  uiout->field_int ("symbols-loaded", solib->symbols_loaded);
> +  if (!gdbarch_has_global_solist (target_gdbarch ()))
> +      uiout->field_fmt ("thread-group", "i%d", current_inferior ()->num);
> +
> +  struct cleanup *cleanup
> +    = make_cleanup_ui_out_list_begin_end (uiout, "ranges");
> +  struct cleanup *tuple_clean_up
> +    = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
> +  if (solib->addr_high != 0)
> +    {
> +      uiout->field_core_addr ("from", gdbarch, solib->addr_low);
> +      uiout->field_core_addr ("to", gdbarch, solib->addr_high);
> +    }
> +  do_cleanups (tuple_clean_up);
> +  do_cleanups (cleanup);
> +}
> +
>   static void
>   mi_solib_loaded (struct so_list *solib)
>   {
> @@ -1140,14 +1167,7 @@ mi_solib_loaded (struct so_list *solib)
>   
>         uiout->redirect (mi->event_channel);
>   
> -      uiout->field_string ("id", solib->so_original_name);
> -      uiout->field_string ("target-name", solib->so_original_name);
> -      uiout->field_string ("host-name", solib->so_name);
> -      uiout->field_int ("symbols-loaded", solib->symbols_loaded);
> -      if (!gdbarch_has_global_solist (target_gdbarch ()))
> -	{
> -	  uiout->field_fmt ("thread-group", "i%d", current_inferior ()->num);
> -	}
> +      mi_output_solib_attribs (uiout, solib);
>   
>         uiout->redirect (NULL);
>   
> diff --git a/gdb/mi/mi-interp.h b/gdb/mi/mi-interp.h
> new file mode 100644
> index 0000000..5b7b9f2
> --- /dev/null
> +++ b/gdb/mi/mi-interp.h
> @@ -0,0 +1,27 @@
> +/* MI Interpreter Definitions and Commands for GDB, the GNU debugger.
> +
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef MI_INTERP_H
> +#define MI_INTERP_H
> +
> +/* Output the shared object attributes to UIOUT.  */
> +
> +void mi_output_solib_attribs (ui_out *uiout, struct so_list *solib);
> +
> +#endif
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 38737b6..af94383 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -89,9 +89,6 @@ set_solib_ops (struct gdbarch *gdbarch, const struct target_so_ops *new_ops)
>      configuration needs to call set_solib_ops.  */
>   struct target_so_ops *current_target_so_ops;
>   
> -/* List of known shared objects */
> -#define so_list_head current_program_space->so_list
> -
>   /* Local function prototypes */
>   
>   /* If non-empty, this is a search path for loading non-absolute shared library
> @@ -749,21 +746,7 @@ solib_used (const struct so_list *const known)
>     return 0;
>   }
>   
> -/* Synchronize GDB's shared object list with inferior's.
> -
> -   Extract the list of currently loaded shared objects from the
> -   inferior, and compare it with the list of shared objects currently
> -   in GDB's so_list_head list.  Edit so_list_head to bring it in sync
> -   with the inferior's new list.
> -
> -   If we notice that the inferior has unloaded some shared objects,
> -   free any symbolic info GDB had read about those shared objects.
> -
> -   Don't load symbolic info for any new shared objects; just add them
> -   to the list, and leave their symbols_loaded flag clear.
> -
> -   If FROM_TTY is non-null, feel free to print messages about what
> -   we're doing.  */
> +/* See solib.h.  */
>   
>   void
>   update_solib_list (int from_tty)
> @@ -1105,7 +1088,7 @@ info_sharedlibrary_command (char *pattern, int from_tty)
>   
>     uiout->table_body ();
>   
> -  for (so = so_list_head; so; so = so->next)
> +  ALL_SO_LIBS (so)
>       {
>         struct cleanup *lib_cleanup;
>   
> diff --git a/gdb/solib.h b/gdb/solib.h
> index 1b46849..e91fb75 100644
> --- a/gdb/solib.h
> +++ b/gdb/solib.h
> @@ -28,6 +28,9 @@ struct program_space;
>   
>   #include "symfile-add-flags.h"
>   
> +/* List of known shared objects */
> +#define so_list_head current_program_space->so_list
> +
>   /* Called when we free all symtabs, to free the shared library information
>      as well.  */
>   
> @@ -75,6 +78,24 @@ extern void no_shared_libraries (char *ignored, int from_tty);
>   extern void set_solib_ops (struct gdbarch *gdbarch,
>   			   const struct target_so_ops *new_ops);
>   
> +/* Synchronize GDB's shared object list with inferior's.
> +
> +   Extract the list of currently loaded shared objects from the
> +   inferior, and compare it with the list of shared objects currently
> +   in GDB's so_list_head list.  Edit so_list_head to bring it in sync
> +   with the inferior's new list.
> +
> +   If we notice that the inferior has unloaded some shared objects,
> +   free any symbolic info GDB had read about those shared objects.
> +
> +   Don't load symbolic info for any new shared objects; just add them
> +   to the list, and leave their symbols_loaded flag clear.
> +
> +   If FROM_TTY is non-null, feel free to print messages about what
> +   we're doing.  */
> +
> +extern void update_solib_list (int from_tty);
> +
>   /* Return non-zero if NAME is the libpthread shared library.  */
>   
>   extern int libpthread_name_p (const char *name);
> diff --git a/gdb/solist.h b/gdb/solist.h
> index 9c7e965..6e2ab66 100644
> --- a/gdb/solist.h
> +++ b/gdb/solist.h
> @@ -24,6 +24,11 @@
>   #include "symtab.h"
>   #include "gdb_bfd.h"
>   
> +#define ALL_SO_LIBS(so) \
> +    for (so = so_list_head; \
> +	 so != NULL; \
> +	 so = so->next)
> +
>   /* Forward declaration for target specific link map information.  This
>      struct is opaque to all but the target specific file.  */
>   struct lm_info;
> @@ -74,7 +79,10 @@ struct so_list
>   
>       /* Record the range of addresses belonging to this shared library.
>          There may not be just one (e.g. if two segments are relocated
> -       differently); but this is only used for "info sharedlibrary".  */
> +       differently).  This is used for "info sharedlibrary" and
> +       the MI command "-file-list-shared-libraries".  The later has a format
> +       that supports outputting multiple segments once the related code
> +       supports them.  */
>       CORE_ADDR addr_low, addr_high;
>     };
>   
> diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
> index 575cb97..ff933e7 100644
> --- a/gdb/testsuite/gdb.mi/mi-solib.exp
> +++ b/gdb/testsuite/gdb.mi/mi-solib.exp
> @@ -48,27 +48,46 @@ if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != ""
>   
>   mi_delete_breakpoints
>   mi_gdb_reinitialize_dir $srcdir/$subdir
> -mi_gdb_reinitialize_dir $srcdir/$subdir
>   mi_gdb_load ${binfile}
>   
>   mi_load_shlibs $binfile_lib
>   
> -mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
> -    "set stop-on-solib-events"
> +proc_with_prefix test_stop_on_solib_events {} {
> +    mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
> +	"set stop-on-solib-events"
>   
> -# We use "run" rather than "-exec-run" here in order to test that CLI
> -# commands still cause the correct MI output to be generated.
> -mi_run_with_cli
> +    # We use "run" rather than "-exec-run" here in order to test that CLI
> +    # commands still cause the correct MI output to be generated.
> +    mi_run_with_cli
>   
> -# Also test that the CLI solib event note is output.
> -set test "CLI prints solib event"
> -gdb_expect {
> -    -re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
> -	pass "$test"
> -    }
> -    timeout {
> -	fail "$test (timeout)"
> +    # Also test that the CLI solib event note is output.
> +    set test "CLI prints solib event"
> +    gdb_expect {
> +	-re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
> +	    pass "$test"
> +	}
> +	timeout {
> +	    fail "$test (timeout)"
> +	}
>       }
> +
> +    mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
> +
> +    # Unset solib events to avoid interfering with other tests.
> +    mi_gdb_test "778-gdb-set stop-on-solib-events 0" "778\\^done" \
> +	"unset stop-on-solib-events"
> +}
> +
> +proc_with_prefix test_file_list_shared_libraries {} {
> +    global libname
> +    global binfile
> +
> +    mi_continue_to main
> +
> +    mi_gdb_test "222-file-list-shared-libraries" \
> +	"222\\^done,shared-libraries=\\\[\{id=\".*${libname}.so\",target-name=\".*${libname}.so\",host-name=\".*${libname}.so\",symbols-loaded=\"1\",thread-group=\".*\",ranges=\\\[\{from=\".*\",to=\".*\"\}]\}]" \
> +	"get the list of shared libraries"
>   }
>   
> -mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
> +test_stop_on_solib_events
> +test_file_list_shared_libraries