public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 01/10] Standardize darwin's lm_info
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (2 preceding siblings ...)
  2017-04-26 22:47 ` [PATCH 04/10] Class-ify lm_info_dsbt Simon Marchi
@ 2017-04-26 22:47 ` Simon Marchi
  2017-04-28 15:57   ` Pedro Alves
  2017-04-26 22:52 ` [PATCH 05/10] Fix indentation of lm_info_frv Simon Marchi
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Darwin's lm_info structure is used a little bit differently than the
other solib implementations.  The other implementations first allocate
an so_list object, then instanciate their specific lm_info structure,
and assign it to so_list::lm_info.

The Darwin implementation allocates both at the same time
(darwin_so_list).  This patch changes it to be like the others, so that
we'll be able to do some generalizations later.

gdb/ChangeLog:

	* solib-darwin.c (struct darwin_so_list): Remove.
	(darwin_current_sos): Allocate an so_list object instead of a
	darwin_so_list, separately allocate an lm_info object.
	(darwin_free_so): Free lm_info.
---
 gdb/solib-darwin.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 0b975569e2..c507e13caf 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -159,14 +159,6 @@ struct lm_info
   CORE_ADDR lm_addr;
 };
 
-struct darwin_so_list
-{
-  /* Common field.  */
-  struct so_list sl;
-  /* Darwin specific data.  */
-  struct lm_info li;
-};
-
 /* Lookup the value for a specific symbol.  */
 
 static CORE_ADDR
@@ -271,7 +263,6 @@ darwin_current_sos (void)
       unsigned long hdr_val;
       char *file_path;
       int errcode;
-      struct darwin_so_list *dnew;
       struct so_list *newobj;
       struct cleanup *old_chain;
 
@@ -302,11 +293,10 @@ darwin_current_sos (void)
 	break;
 
       /* Create and fill the new so_list element.  */
-      dnew = XCNEW (struct darwin_so_list);
-      newobj = &dnew->sl;
-      old_chain = make_cleanup (xfree, dnew);
+      newobj = XCNEW (struct so_list);
+      old_chain = make_cleanup (xfree, newobj);
 
-      newobj->lm_info = &dnew->li;
+      newobj->lm_info = XCNEW (struct lm_info);
 
       strncpy (newobj->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
       newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
@@ -587,6 +577,7 @@ darwin_clear_solib (void)
 static void
 darwin_free_so (struct so_list *so)
 {
+  xfree (so->lm_info);
 }
 
 /* The section table is built from bfd sections using bfd VMAs.
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 04/10] Class-ify lm_info_dsbt
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
  2017-04-26 22:47 ` [PATCH 02/10] Make various lm_info implementations inherit from a base class Simon Marchi
  2017-04-26 22:47 ` [PATCH 03/10] Class-ify lm_info_aix Simon Marchi
@ 2017-04-26 22:47 ` Simon Marchi
  2017-04-28 15:59   ` Pedro Alves
  2017-04-26 22:47 ` [PATCH 01/10] Standardize darwin's lm_info Simon Marchi
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes lm_info_dsbt a "real" class.  It introduces a
destructor, initializes the field and replaces XCNEW/xfree with
new/delete.

gdb/ChangeLog:

	* solib-dsbt.c (struct lm_info_dsbt): Add destructor, initialize
	map field.
	(dsbt_current_sos): Allocate lm_info_dsbt with new.
	(dsbt_relocate_main_executable): Free lm_info_dsbt with delete
	and allocate with new.
	(dsbt_clear_solib, dsbt_free_so): Free lm_info_dsbt with delete.
---
 gdb/solib-dsbt.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 6d410ac90b..f8de53af5d 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -125,8 +125,13 @@ struct ext_link_map
 
 struct lm_info_dsbt : public lm_info_base
 {
+  ~lm_info_dsbt ()
+  {
+    xfree (this->map);
+  }
+
   /* The loadmap, digested into an easier to use form.  */
-  struct int_elf32_dsbt_loadmap *map;
+  int_elf32_dsbt_loadmap *map = NULL;
 };
 
 /* Per pspace dsbt specific data.  */
@@ -711,7 +716,7 @@ dsbt_current_sos (void)
 	    }
 
 	  sop = XCNEW (struct so_list);
-	  lm_info_dsbt *li = XCNEW (lm_info_dsbt);
+	  lm_info_dsbt *li = new lm_info_dsbt;
 	  sop->lm_info = li;
 	  li->map = loadmap;
 	  /* Fetch the name.  */
@@ -930,8 +935,9 @@ dsbt_relocate_main_executable (void)
   dsbt_get_initial_loadmaps ();
   ldm = info->exec_loadmap;
 
-  xfree (info->main_executable_lm_info);
-  info->main_executable_lm_info = XCNEW (lm_info_dsbt);
+  if (info->main_executable_lm_info != NULL)
+    delete info->main_executable_lm_info;
+  info->main_executable_lm_info = new lm_info_dsbt;
   info->main_executable_lm_info->map = ldm;
 
   new_offsets = XCNEWVEC (struct section_offsets,
@@ -1006,11 +1012,10 @@ dsbt_clear_solib (void)
 
   info->lm_base_cache = 0;
   info->main_lm_addr = 0;
-  if (info->main_executable_lm_info != 0)
+  if (info->main_executable_lm_info != NULL)
     {
-      xfree (info->main_executable_lm_info->map);
-      xfree (info->main_executable_lm_info);
-      info->main_executable_lm_info = 0;
+      delete info->main_executable_lm_info;
+      info->main_executable_lm_info = NULL;
     }
 }
 
@@ -1019,8 +1024,7 @@ dsbt_free_so (struct so_list *so)
 {
   lm_info_dsbt *li = (lm_info_dsbt *) so->lm_info;
 
-  xfree (li->map);
-  xfree (li);
+  delete li;
 }
 
 static void
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 02/10] Make various lm_info implementations inherit from a base class
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
@ 2017-04-26 22:47 ` Simon Marchi
  2017-04-28 15:57   ` Pedro Alves
  2017-04-26 22:47 ` [PATCH 03/10] Class-ify lm_info_aix Simon Marchi
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

The lm_info structure is used to store target specific information about
mapped libraries.  It is currently defined as an opaque type in solist.h
and a pointer to it is included in solist, the target-agnostic object
representing a loaded shared library.  Multiple targets define their own
implementation of lm_info.

In anticipation of using C++ stuff (e.g. vector) in the lm_info objects,
we first need to avoid different definitions of classes with the same
name (which violates the one definition rule).  This patch does it by
having a base class (lm_info_base) from which all the specific lm_info
derive.  Each implementation is renamed to something that makes sense
(e.g. lm_info_aix for AIX).  The next logical step would probably be to
derive directly from so_list, but it's not really obvious, so I'll keep
that for another day.

One special case is the Neutrino (nto) support.  It uses SVR4-style
libraries, but overrides some methods.  To do that, it needed to have
its own copy of SVR4's lm_info structure in nto-tdep.c, because it was
just not possible to put it in solib-svr4.h and include that file.  Over
time, that copy got out of sync, which is still the case today.  I can
only assume that the lm_addr function in nto-tdep.c is broken right now.
The first field of the old lm_info was a pointer (gdb_byte *), whereas
in the new lm_info it's an address in the inferior (CORE_ADDR).  Trying
to use that field today probably results in a crash.  With this
refactor, it's now possible to put lm_info_svr4 in solib-svr4.h and just
include it.  I have adapted the code in nto-tdep.c to that it builds,
but it's probably not correct.  Since I don't have the knowledge nor
setup to try this on Neutrino, somebody else would have to fix it.  But
I am confident that I am not making things worse than they already are.

gdb/ChangeLog:

	* solist.h (struct lm_info): Remove.
	(struct lm_info_base): New class.
	(struct so_list) <lm_info>: Change type to lm_info_base *.
	* nto-tdep.c (struct lm_info): Remove.
	(lm_addr): Adjust.
	* solib-aix.c (struct lm_info): Rename to ...
	(struct lm_info_aix): ... this.  Extend lm_info_base.
	(lm_info_p): Rename to ...
	(lm_info_aix_p): ... this, and adjust.
	(solib_aix_new_lm_info, solib_aix_xfree_lm_info,
	solib_aix_parse_libraries, library_list_start_library,
	solib_aix_free_library_list, solib_aix_parse_libraries,
	solib_aix_get_library_list,
	solib_aix_relocate_section_addresses, solib_aix_free_so,
	solib_aix_get_section_offsets,
	solib_aix_solib_create_inferior_hook, solib_aix_current_sos):
	Adjust.
	(struct solib_aix_inferior_data) <library_list>: Adjust.
	* solib-darwin.c (struct lm_info): Rename to ...
	(struct lm_info_darwin): ... this.  Extend lm_info_base.
	(darwin_current_sos, darwin_relocate_section_addresses): Adjust.
	* solib-dsbt.c (struct lm_info): Rename to ...
	(struct lm_info_dsbt): ... this.  Extend lm_info_base.
	(struct dsbt_info) <main_executable_lm_info): Adjust.
	(dsbt_current_sos, dsbt_relocate_main_executable, dsbt_free_so,
	dsbt_relocate_section_addresses): Adjust.
	* solib-frv.c (struct lm_info): Rename to ...
	(struct lm_info_frv): ... this.  Extend lm_info_base.
	(main_executable_lm_info): Adjust.
	(frv_current_sos, frv_relocate_main_executable, frv_free_so,
	frv_relocate_section_addresses, frv_fdpic_find_global_pointer,
	find_canonical_descriptor_in_load_object,
	frv_fdpic_find_canonical_descriptor): Adjust.
	* solib-svr4.c (struct lm_info): Move to solib-svr4.h, renamed
	to lm_info_svr4.
	(lm_info_read, lm_addr_check, svr4_keep_data_in_core,
	svr4_clear_so, svr4_copy_library_list,
	library_list_start_library, svr4_default_sos, svr4_read_so_list,
	svr4_current_sos, svr4_fetch_objfile_link_map,
	solist_update_incremental): Adjust.
	* solib-svr4.h (struct lm_info_svr4): Move here from
	solib-svr4.c.
	* solib-target.c (struct lm_info): Rename to ...
	(struct lm_info_target): ... this.  Extend lm_info_base.
	(lm_info_p): Rename to ...
	(lm_info_target_p): ... this.
	(solib_target_parse_libraries, library_list_start_segment,
	library_list_start_section, library_list_start_library,
	library_list_end_library, solib_target_free_library_list,
	solib_target_current_sos, solib_target_free_so,
	solib_target_relocate_section_addresses): Adjust.
	* windows-nat.c (struct lm_info): Rename to ...
	(struct lm_info_windows): ... this.  Extend lm_info_base.
	(windows_make_so, handle_load_dll, handle_unload_dll,
	windows_xfer_shared_libraries): Adjust.
---
 gdb/nto-tdep.c     |  35 +----------------
 gdb/solib-aix.c    |  62 +++++++++++++++---------------
 gdb/solib-darwin.c |  13 ++++---
 gdb/solib-dsbt.c   |  22 ++++++-----
 gdb/solib-frv.c    |  49 ++++++++++++-----------
 gdb/solib-svr4.c   | 111 +++++++++++++++++++++++++----------------------------
 gdb/solib-svr4.h   |  23 +++++++++++
 gdb/solib-target.c |  85 ++++++++++++++++++++--------------------
 gdb/solist.h       |  10 +++--
 gdb/windows-nat.c  |  45 ++++++++++++++--------
 10 files changed, 233 insertions(+), 222 deletions(-)

diff --git a/gdb/nto-tdep.c b/gdb/nto-tdep.c
index 8f8d1d5384..f9959ca9e0 100644
--- a/gdb/nto-tdep.c
+++ b/gdb/nto-tdep.c
@@ -239,43 +239,12 @@ nto_parse_redirection (char *pargv[], const char **pin, const char **pout,
   return argv;
 }
 
-/* The struct lm_info, lm_addr, and nto_truncate_ptr are copied from
-   solib-svr4.c to support nto_relocate_section_addresses
-   which is different from the svr4 version.  */
-
-/* Link map info to include in an allocated so_list entry */
-
-struct lm_info
-  {
-    /* Pointer to copy of link map from inferior.  The type is char *
-       rather than void *, so that we may use byte offsets to find the
-       various fields without the need for a cast.  */
-    gdb_byte *lm;
-
-    /* Amount by which addresses in the binary should be relocated to
-       match the inferior.  This could most often be taken directly
-       from lm, but when prelinking is involved and the prelink base
-       address changes, we may need a different offset, we want to
-       warn about the difference and compute it only once.  */
-    CORE_ADDR l_addr;
-
-    /* The target location of lm.  */
-    CORE_ADDR lm_addr;
-  };
-
-
 static CORE_ADDR
 lm_addr (struct so_list *so)
 {
-  if (so->lm_info->l_addr == (CORE_ADDR)-1)
-    {
-      struct link_map_offsets *lmo = nto_fetch_link_map_offsets ();
-      struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
+  lm_info_svr4 *li = (lm_info_svr4 *) so->lm_info;
 
-      so->lm_info->l_addr =
-	extract_typed_address (so->lm_info->lm + lmo->l_addr_offset, ptr_type);
-    }
-  return so->lm_info->l_addr;
+  return li->l_addr;
 }
 
 static CORE_ADDR
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 66add03054..54b8c38032 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -33,7 +33,7 @@ static int solib_aix_debug;
 
 /* Our private data in struct so_list.  */
 
-struct lm_info
+struct lm_info_aix : public lm_info_base
 {
   /* The name of the file mapped by the loader.  Apart from the entry
      for the main executable, this is usually a shared library (which,
@@ -58,17 +58,17 @@ struct lm_info
   ULONGEST data_size;
 };
 
-typedef struct lm_info *lm_info_p;
-DEF_VEC_P(lm_info_p);
+typedef lm_info_aix *lm_info_aix_p;
+DEF_VEC_P(lm_info_aix_p);
 
 /* Return a deep copy of the given struct lm_info object.  */
 
-static struct lm_info *
-solib_aix_new_lm_info (struct lm_info *info)
+static lm_info_aix *
+solib_aix_new_lm_info (lm_info_aix *info)
 {
-  struct lm_info *result = XNEW (struct lm_info);
+  lm_info_aix *result = XCNEW (lm_info_aix);
 
-  memcpy (result, info, sizeof (struct lm_info));
+  memcpy (result, info, sizeof (lm_info_aix));
   result->filename = xstrdup (info->filename);
   if (info->member_name != NULL)
     result->member_name = xstrdup (info->member_name);
@@ -79,7 +79,7 @@ solib_aix_new_lm_info (struct lm_info *info)
 /* Free the memory allocated for the given lm_info.  */
 
 static void
-solib_aix_xfree_lm_info (struct lm_info *info)
+solib_aix_xfree_lm_info (lm_info_aix *info)
 {
   xfree (info->filename);
   xfree (info->member_name);
@@ -98,7 +98,7 @@ struct solib_aix_inferior_data
      the same principles applied to shared libraries also apply
      to the main executable.  So it's simpler to keep it as part
      of this list.  */
-  VEC (lm_info_p) *library_list;
+  VEC (lm_info_aix_p) *library_list;
 };
 
 /* Key to our per-inferior data.  */
@@ -127,7 +127,7 @@ get_solib_aix_inferior_data (struct inferior *inf)
 
 /* Dummy implementation if XML support is not compiled in.  */
 
-static VEC (lm_info_p) *
+static VEC (lm_info_aix_p) *
 solib_aix_parse_libraries (const char *library)
 {
   static int have_warned;
@@ -161,8 +161,8 @@ library_list_start_library (struct gdb_xml_parser *parser,
 			    void *user_data,
 			    VEC (gdb_xml_value_s) *attributes)
 {
-  VEC (lm_info_p) **list = (VEC (lm_info_p) **) user_data;
-  struct lm_info *item = XCNEW (struct lm_info);
+  VEC (lm_info_aix_p) **list = (VEC (lm_info_aix_p) **) user_data;
+  lm_info_aix *item = XCNEW (lm_info_aix);
   struct gdb_xml_value *attr;
 
   attr = xml_find_attribute (attributes, "name");
@@ -184,7 +184,7 @@ library_list_start_library (struct gdb_xml_parser *parser,
   attr = xml_find_attribute (attributes, "data_size");
   item->data_size = * (ULONGEST *) attr->value;
 
-  VEC_safe_push (lm_info_p, *list, item);
+  VEC_safe_push (lm_info_aix_p, *list, item);
 }
 
 /* Handle the start of a <library-list-aix> element.  */
@@ -207,16 +207,16 @@ library_list_start_list (struct gdb_xml_parser *parser,
 static void
 solib_aix_free_library_list (void *p)
 {
-  VEC (lm_info_p) **result = (VEC (lm_info_p) **) p;
-  struct lm_info *info;
+  VEC (lm_info_aix_p) **result = (VEC (lm_info_aix_p) **) p;
+  lm_info_aix *info;
   int ix;
 
   if (solib_aix_debug)
     fprintf_unfiltered (gdb_stdlog, "DEBUG: solib_aix_free_library_list\n");
 
-  for (ix = 0; VEC_iterate (lm_info_p, *result, ix, info); ix++)
+  for (ix = 0; VEC_iterate (lm_info_aix_p, *result, ix, info); ix++)
     solib_aix_xfree_lm_info (info);
-  VEC_free (lm_info_p, *result);
+  VEC_free (lm_info_aix_p, *result);
   *result = NULL;
 }
 
@@ -256,14 +256,14 @@ static const struct gdb_xml_element library_list_elements[] =
 };
 
 /* Parse LIBRARY, a string containing the loader info in XML format,
-   and return an lm_info_p vector.
+   and return an lm_info_aix_p vector.
 
    Return NULL if the parsing failed.  */
 
-static VEC (lm_info_p) *
+static VEC (lm_info_aix_p) *
 solib_aix_parse_libraries (const char *library)
 {
-  VEC (lm_info_p) *result = NULL;
+  VEC (lm_info_aix_p) *result = NULL;
   struct cleanup *back_to = make_cleanup (solib_aix_free_library_list,
                                           &result);
 
@@ -291,7 +291,7 @@ solib_aix_parse_libraries (const char *library)
    is not NULL, then print a warning including WARNING_MSG and
    a description of the error.  */
 
-static VEC (lm_info_p) *
+static VEC (lm_info_aix_p) *
 solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
 {
   struct solib_aix_inferior_data *data;
@@ -394,7 +394,7 @@ solib_aix_relocate_section_addresses (struct so_list *so,
   struct bfd_section *bfd_sect = sec->the_bfd_section;
   bfd *abfd = bfd_sect->owner;
   const char *section_name = bfd_section_name (abfd, bfd_sect);
-  struct lm_info *info = so->lm_info;
+  lm_info_aix *info = (lm_info_aix *) so->lm_info;
 
   if (strcmp (section_name, ".text") == 0)
     {
@@ -446,7 +446,7 @@ solib_aix_free_so (struct so_list *so)
   if (solib_aix_debug)
     fprintf_unfiltered (gdb_stdlog, "DEBUG: solib_aix_free_so (%s)\n",
 			so->so_name);
-  solib_aix_xfree_lm_info (so->lm_info);
+  solib_aix_xfree_lm_info ((lm_info_aix *) so->lm_info);
 }
 
 /* Implement the "clear_solib" target_so_ops method.  */
@@ -465,7 +465,7 @@ solib_aix_clear_solib (void)
 
 static struct section_offsets *
 solib_aix_get_section_offsets (struct objfile *objfile,
-			       struct lm_info *info)
+			       lm_info_aix *info)
 {
   struct section_offsets *offsets;
   bfd *abfd = objfile->obfd;
@@ -519,8 +519,8 @@ static void
 solib_aix_solib_create_inferior_hook (int from_tty)
 {
   const char *warning_msg = "unable to relocate main executable";
-  VEC (lm_info_p) *library_list;
-  struct lm_info *exec_info;
+  VEC (lm_info_aix_p) *library_list;
+  lm_info_aix *exec_info;
 
   /* We need to relocate the main executable...  */
 
@@ -529,13 +529,13 @@ solib_aix_solib_create_inferior_hook (int from_tty)
   if (library_list == NULL)
     return;  /* Warning already printed.  */
 
-  if (VEC_length (lm_info_p, library_list) < 1)
+  if (VEC_length (lm_info_aix_p, library_list) < 1)
     {
       warning (_("unable to relocate main executable (no info from loader)"));
       return;
     }
 
-  exec_info = VEC_index (lm_info_p, library_list, 0);
+  exec_info = VEC_index (lm_info_aix_p, library_list, 0);
 
   if (symfile_objfile != NULL)
     {
@@ -554,8 +554,8 @@ static struct so_list *
 solib_aix_current_sos (void)
 {
   struct so_list *start = NULL, *last = NULL;
-  VEC (lm_info_p) *library_list;
-  struct lm_info *info;
+  VEC (lm_info_aix_p) *library_list;
+  lm_info_aix *info;
   int ix;
 
   library_list = solib_aix_get_library_list (current_inferior (), NULL);
@@ -565,7 +565,7 @@ solib_aix_current_sos (void)
   /* Build a struct so_list for each entry on the list.
      We skip the first entry, since this is the entry corresponding
      to the main executable, not a shared library.  */
-  for (ix = 1; VEC_iterate (lm_info_p, library_list, ix, info); ix++)
+  for (ix = 1; VEC_iterate (lm_info_aix_p, library_list, ix, info); ix++)
     {
       struct so_list *new_solib = XCNEW (struct so_list);
       char *so_name;
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index c507e13caf..03211cfb92 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -153,7 +153,7 @@ darwin_load_image_infos (struct darwin_info *info)
 
 /* Link map info to include in an allocated so_list entry.  */
 
-struct lm_info
+struct lm_info_darwin : public lm_info_base
 {
   /* The target location of lm.  */
   CORE_ADDR lm_addr;
@@ -296,13 +296,14 @@ darwin_current_sos (void)
       newobj = XCNEW (struct so_list);
       old_chain = make_cleanup (xfree, newobj);
 
-      newobj->lm_info = XCNEW (struct lm_info);
+      lm_info_darwin *li = XCNEW (lm_info_darwin);
+      newobj->lm_info = li;
 
       strncpy (newobj->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
       newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       strcpy (newobj->so_original_name, newobj->so_name);
       xfree (file_path);
-      newobj->lm_info->lm_addr = load_addr;
+      li->lm_addr = load_addr;
 
       if (head == NULL)
 	head = newobj;
@@ -587,8 +588,10 @@ static void
 darwin_relocate_section_addresses (struct so_list *so,
 				   struct target_section *sec)
 {
-  sec->addr += so->lm_info->lm_addr;
-  sec->endaddr += so->lm_info->lm_addr;
+  lm_info_darwin *li = (lm_info_darwin *) so->lm_info;
+
+  sec->addr += li->lm_addr;
+  sec->endaddr += li->lm_addr;
 
   /* Best effort to set addr_high/addr_low.  This is used only by
      'info sharedlibary'.  */
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index c4071066f1..6d410ac90b 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -123,7 +123,7 @@ struct ext_link_map
 
 /* Link map info to include in an allocated so_list entry */
 
-struct lm_info
+struct lm_info_dsbt : public lm_info_base
 {
   /* The loadmap, digested into an easier to use form.  */
   struct int_elf32_dsbt_loadmap *map;
@@ -137,7 +137,7 @@ struct dsbt_info
      of loaded shared objects.  ``main_executable_lm_info'' provides
      a way to get at this information so that it doesn't need to be
      frequently recomputed.  Initialized by dsbt_relocate_main_executable.  */
-  struct lm_info *main_executable_lm_info;
+  struct lm_info_dsbt *main_executable_lm_info;
 
   /* Load maps for the main executable and the interpreter.  These are obtained
      from ptrace.  They are the starting point for getting into the program,
@@ -711,8 +711,9 @@ dsbt_current_sos (void)
 	    }
 
 	  sop = XCNEW (struct so_list);
-	  sop->lm_info = XCNEW (struct lm_info);
-	  sop->lm_info->map = loadmap;
+	  lm_info_dsbt *li = XCNEW (lm_info_dsbt);
+	  sop->lm_info = li;
+	  li->map = loadmap;
 	  /* Fetch the name.  */
 	  addr = extract_unsigned_integer (lm_buf.l_name,
 					   sizeof (lm_buf.l_name),
@@ -930,7 +931,7 @@ dsbt_relocate_main_executable (void)
   ldm = info->exec_loadmap;
 
   xfree (info->main_executable_lm_info);
-  info->main_executable_lm_info = XCNEW (struct lm_info);
+  info->main_executable_lm_info = XCNEW (lm_info_dsbt);
   info->main_executable_lm_info->map = ldm;
 
   new_offsets = XCNEWVEC (struct section_offsets,
@@ -1016,8 +1017,10 @@ dsbt_clear_solib (void)
 static void
 dsbt_free_so (struct so_list *so)
 {
-  xfree (so->lm_info->map);
-  xfree (so->lm_info);
+  lm_info_dsbt *li = (lm_info_dsbt *) so->lm_info;
+
+  xfree (li->map);
+  xfree (li);
 }
 
 static void
@@ -1025,9 +1028,8 @@ dsbt_relocate_section_addresses (struct so_list *so,
 				 struct target_section *sec)
 {
   int seg;
-  struct int_elf32_dsbt_loadmap *map;
-
-  map = so->lm_info->map;
+  lm_info_dsbt *li = (lm_info_dsbt *) so->lm_info;
+  int_elf32_dsbt_loadmap *map = li->map;
 
   for (seg = 0; seg < map->nsegs; seg++)
     {
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index e8d5f20d4f..0108d97815 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -202,7 +202,7 @@ struct ext_link_map
 
 /* Link map info to include in an allocated so_list entry.  */
 
-struct lm_info
+struct lm_info_frv : public lm_info_base
   {
     /* The loadmap, digested into an easier to use form.  */
     struct int_elf32_fdpic_loadmap *map;
@@ -231,7 +231,7 @@ struct lm_info
    of loaded shared objects.  ``main_executable_lm_info'' provides
    a way to get at this information so that it doesn't need to be
    frequently recomputed.  Initialized by frv_relocate_main_executable().  */
-static struct lm_info *main_executable_lm_info;
+static lm_info_frv *main_executable_lm_info;
 
 static void frv_relocate_main_executable (void);
 static CORE_ADDR main_got (void);
@@ -389,10 +389,11 @@ frv_current_sos (void)
 	    }
 
 	  sop = XCNEW (struct so_list);
-	  sop->lm_info = XCNEW (struct lm_info);
-	  sop->lm_info->map = loadmap;
-	  sop->lm_info->got_value = got_addr;
-	  sop->lm_info->lm_addr = lm_addr;
+	  lm_info_frv *li = XCNEW (lm_info_frv);
+	  sop->lm_info = li;
+	  li->map = loadmap;
+	  li->got_value = got_addr;
+	  li->lm_addr = lm_addr;
 	  /* Fetch the name.  */
 	  addr = extract_unsigned_integer (lm_buf.l_name,
 					   sizeof (lm_buf.l_name),
@@ -783,7 +784,7 @@ frv_relocate_main_executable (void)
 
   if (main_executable_lm_info)
     xfree (main_executable_lm_info);
-  main_executable_lm_info = XCNEW (struct lm_info);
+  main_executable_lm_info = XCNEW (lm_info_frv);
   main_executable_lm_info->map = ldm;
 
   new_offsets = XCNEWVEC (struct section_offsets,
@@ -870,10 +871,12 @@ frv_clear_solib (void)
 static void
 frv_free_so (struct so_list *so)
 {
-  xfree (so->lm_info->map);
-  xfree (so->lm_info->dyn_syms);
-  xfree (so->lm_info->dyn_relocs);
-  xfree (so->lm_info);
+  lm_info_frv *li = (lm_info_frv *) so->lm_info;
+
+  xfree (li->map);
+  xfree (li->dyn_syms);
+  xfree (li->dyn_relocs);
+  xfree (li);
 }
 
 static void
@@ -881,9 +884,8 @@ frv_relocate_section_addresses (struct so_list *so,
                                  struct target_section *sec)
 {
   int seg;
-  struct int_elf32_fdpic_loadmap *map;
-
-  map = so->lm_info->map;
+  lm_info_frv *li = (lm_info_frv *) so->lm_info;
+  int_elf32_fdpic_loadmap *map = li->map;
 
   for (seg = 0; seg < map->nsegs; seg++)
     {
@@ -926,15 +928,14 @@ frv_fdpic_find_global_pointer (CORE_ADDR addr)
   while (so)
     {
       int seg;
-      struct int_elf32_fdpic_loadmap *map;
-
-      map = so->lm_info->map;
+      lm_info_frv *li = (lm_info_frv *) so->lm_info;
+      int_elf32_fdpic_loadmap *map = li->map;
 
       for (seg = 0; seg < map->nsegs; seg++)
 	{
 	  if (map->segs[seg].addr <= addr
 	      && addr < map->segs[seg].addr + map->segs[seg].p_memsz)
-	    return so->lm_info->got_value;
+	    return li->got_value;
 	}
 
       so = so->next;
@@ -947,7 +948,7 @@ frv_fdpic_find_global_pointer (CORE_ADDR addr)
 
 /* Forward declarations for frv_fdpic_find_canonical_descriptor().  */
 static CORE_ADDR find_canonical_descriptor_in_load_object
-  (CORE_ADDR, CORE_ADDR, const char *, bfd *, struct lm_info *);
+  (CORE_ADDR, CORE_ADDR, const char *, bfd *, lm_info_frv *);
 
 /* Given a function entry point, attempt to find the canonical descriptor
    associated with that entry point.  Return 0 if no canonical descriptor
@@ -987,8 +988,10 @@ frv_fdpic_find_canonical_descriptor (CORE_ADDR entry_point)
       so = master_so_list ();
       while (so)
 	{
+	  lm_info_frv *li = (lm_info_frv *) so->lm_info;
+
 	  addr = find_canonical_descriptor_in_load_object
-		   (entry_point, got_value, name, so->abfd, so->lm_info);
+		   (entry_point, got_value, name, so->abfd, li);
 
 	  if (addr != 0)
 	    break;
@@ -1003,7 +1006,7 @@ frv_fdpic_find_canonical_descriptor (CORE_ADDR entry_point)
 static CORE_ADDR
 find_canonical_descriptor_in_load_object
   (CORE_ADDR entry_point, CORE_ADDR got_value, const char *name, bfd *abfd,
-   struct lm_info *lm)
+   lm_info_frv *lm)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
   arelent *rel;
@@ -1141,8 +1144,10 @@ frv_fetch_objfile_link_map (struct objfile *objfile)
      of shared libraries.  */
   for (so = master_so_list (); so; so = so->next)
     {
+      lm_info_frv *li = (lm_info_frv *) so->lm_info;
+
       if (so->objfile == objfile)
-	return so->lm_info->lm_addr;
+	return li->lm_addr;
     }
 
   /* Not found!  */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 4cb6127af3..6098d505da 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -51,27 +51,6 @@ static int svr4_have_link_map_offsets (void);
 static void svr4_relocate_main_executable (void);
 static void svr4_free_library_list (void *p_list);
 
-/* Link map info to include in an allocated so_list entry.  */
-
-struct lm_info
-  {
-    /* Amount by which addresses in the binary should be relocated to
-       match the inferior.  The direct inferior value is L_ADDR_INFERIOR.
-       When prelinking is involved and the prelink base address changes,
-       we may need a different offset - the recomputed offset is in L_ADDR.
-       It is commonly the same value.  It is cached as we want to warn about
-       the difference and compute it only once.  L_ADDR is valid
-       iff L_ADDR_P.  */
-    CORE_ADDR l_addr, l_addr_inferior;
-    unsigned int l_addr_p : 1;
-
-    /* The target location of lm.  */
-    CORE_ADDR lm_addr;
-
-    /* Values read in from inferior's fields of the same name.  */
-    CORE_ADDR l_ld, l_next, l_prev, l_name;
-  };
-
 /* On SVR4 systems, a list of symbols in the dynamic linker where
    GDB can try to place a breakpoint to monitor shared library
    events.
@@ -189,12 +168,12 @@ svr4_same (struct so_list *gdb, struct so_list *inferior)
   return (svr4_same_1 (gdb->so_original_name, inferior->so_original_name));
 }
 
-static struct lm_info *
+static lm_info_svr4 *
 lm_info_read (CORE_ADDR lm_addr)
 {
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
   gdb_byte *lm;
-  struct lm_info *lm_info;
+  lm_info_svr4 *lm_info;
   struct cleanup *back_to;
 
   lm = (gdb_byte *) xmalloc (lmo->link_map_size);
@@ -210,7 +189,7 @@ lm_info_read (CORE_ADDR lm_addr)
     {
       struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
 
-      lm_info = XCNEW (struct lm_info);
+      lm_info = XCNEW (lm_info_svr4);
       lm_info->lm_addr = lm_addr;
 
       lm_info->l_addr_inferior = extract_typed_address (&lm[lmo->l_addr_offset],
@@ -240,17 +219,19 @@ has_lm_dynamic_from_link_map (void)
 static CORE_ADDR
 lm_addr_check (const struct so_list *so, bfd *abfd)
 {
-  if (!so->lm_info->l_addr_p)
+  lm_info_svr4 *li = (lm_info_svr4 *) so->lm_info;
+
+  if (!li->l_addr_p)
     {
       struct bfd_section *dyninfo_sect;
       CORE_ADDR l_addr, l_dynaddr, dynaddr;
 
-      l_addr = so->lm_info->l_addr_inferior;
+      l_addr = li->l_addr_inferior;
 
       if (! abfd || ! has_lm_dynamic_from_link_map ())
 	goto set_addr;
 
-      l_dynaddr = so->lm_info->l_ld;
+      l_dynaddr = li->l_ld;
 
       dyninfo_sect = bfd_get_section_by_name (abfd, ".dynamic");
       if (dyninfo_sect == NULL)
@@ -333,11 +314,11 @@ lm_addr_check (const struct so_list *so, bfd *abfd)
 	}
 
     set_addr:
-      so->lm_info->l_addr = l_addr;
-      so->lm_info->l_addr_p = 1;
+      li->l_addr = l_addr;
+      li->l_addr_p = 1;
     }
 
-  return so->lm_info->l_addr;
+  return li->l_addr;
 }
 
 /* Per pspace SVR4 specific data.  */
@@ -994,9 +975,10 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
 
   newobj = XCNEW (struct so_list);
   old_chain = make_cleanup (xfree, newobj);
-  newobj->lm_info = lm_info_read (ldsomap);
+  lm_info_svr4 *li = lm_info_read (ldsomap);
+  newobj->lm_info = li;
   make_cleanup (xfree, newobj->lm_info);
-  name_lm = newobj->lm_info ? newobj->lm_info->l_name : 0;
+  name_lm = li != NULL ? li->l_name : 0;
   do_cleanups (old_chain);
 
   return (name_lm >= vaddr && name_lm < vaddr + size);
@@ -1106,8 +1088,10 @@ svr4_free_so (struct so_list *so)
 static void
 svr4_clear_so (struct so_list *so)
 {
-  if (so->lm_info != NULL)
-    so->lm_info->l_addr_p = 0;
+  lm_info_svr4 *li = (lm_info_svr4 *) so->lm_info;
+
+  if (li != NULL)
+    li->l_addr_p = 0;
 }
 
 /* Free so_list built so far (called via cleanup).  */
@@ -1141,8 +1125,8 @@ svr4_copy_library_list (struct so_list *src)
       newobj = XNEW (struct so_list);
       memcpy (newobj, src, sizeof (struct so_list));
 
-      newobj->lm_info = XNEW (struct lm_info);
-      memcpy (newobj->lm_info, src->lm_info, sizeof (struct lm_info));
+      newobj->lm_info = XNEW (lm_info_svr4);
+      memcpy (newobj->lm_info, src->lm_info, sizeof (lm_info_svr4));
 
       newobj->next = NULL;
       *link = newobj;
@@ -1178,10 +1162,11 @@ library_list_start_library (struct gdb_xml_parser *parser,
   struct so_list *new_elem;
 
   new_elem = XCNEW (struct so_list);
-  new_elem->lm_info = XCNEW (struct lm_info);
-  new_elem->lm_info->lm_addr = *lmp;
-  new_elem->lm_info->l_addr_inferior = *l_addrp;
-  new_elem->lm_info->l_ld = *l_ldp;
+  lm_info_svr4 *li = XCNEW (lm_info_svr4);
+  new_elem->lm_info = li;
+  li->lm_addr = *lmp;
+  li->l_addr_inferior = *l_addrp;
+  li->l_ld = *l_ldp;
 
   strncpy (new_elem->so_name, name, sizeof (new_elem->so_name) - 1);
   new_elem->so_name[sizeof (new_elem->so_name) - 1] = 0;
@@ -1332,12 +1317,12 @@ svr4_default_sos (void)
     return NULL;
 
   newobj = XCNEW (struct so_list);
-
-  newobj->lm_info = XCNEW (struct lm_info);
+  lm_info_svr4 *li = XCNEW (lm_info_svr4);
+  newobj->lm_info = li;
 
   /* Nothing will ever check the other fields if we set l_addr_p.  */
-  newobj->lm_info->l_addr = info->debug_loader_offset;
-  newobj->lm_info->l_addr_p = 1;
+  li->l_addr = info->debug_loader_offset;
+  li->l_addr_p = 1;
 
   strncpy (newobj->so_name, info->debug_loader_name, SO_NAME_MAX_PATH_SIZE - 1);
   newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
@@ -1371,20 +1356,21 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
       newobj = XCNEW (struct so_list);
       old_chain = make_cleanup_free_so (newobj);
 
-      newobj->lm_info = lm_info_read (lm);
-      if (newobj->lm_info == NULL)
+      lm_info_svr4 *li = lm_info_read (lm);
+      newobj->lm_info = li;
+      if (li == NULL)
 	{
 	  do_cleanups (old_chain);
 	  return 0;
 	}
 
-      next_lm = newobj->lm_info->l_next;
+      next_lm = li->l_next;
 
-      if (newobj->lm_info->l_prev != prev_lm)
+      if (li->l_prev != prev_lm)
 	{
 	  warning (_("Corrupted shared library list: %s != %s"),
 		   paddress (target_gdbarch (), prev_lm),
-		   paddress (target_gdbarch (), newobj->lm_info->l_prev));
+		   paddress (target_gdbarch (), li->l_prev));
 	  do_cleanups (old_chain);
 	  return 0;
 	}
@@ -1394,26 +1380,26 @@ svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
          SVR4, it has no name.  For others (Solaris 2.3 for example), it
          does have a name, so we can no longer use a missing name to
          decide when to ignore it.  */
-      if (ignore_first && newobj->lm_info->l_prev == 0)
+      if (ignore_first && li->l_prev == 0)
 	{
 	  struct svr4_info *info = get_svr4_info ();
 
-	  first_l_name = newobj->lm_info->l_name;
-	  info->main_lm_addr = newobj->lm_info->lm_addr;
+	  first_l_name = li->l_name;
+	  info->main_lm_addr = li->lm_addr;
 	  do_cleanups (old_chain);
 	  continue;
 	}
 
       /* Extract this shared object's name.  */
-      target_read_string (newobj->lm_info->l_name, &buffer,
-			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
+      target_read_string (li->l_name, &buffer, SO_NAME_MAX_PATH_SIZE - 1,
+			  &errcode);
       if (errcode != 0)
 	{
 	  /* If this entry's l_name address matches that of the
 	     inferior executable, then this is not a normal shared
 	     object, but (most likely) a vDSO.  In this case, silently
 	     skip it; otherwise emit a warning. */
-	  if (first_l_name == 0 || newobj->lm_info->l_name != first_l_name)
+	  if (first_l_name == 0 || li->l_name != first_l_name)
 	    warning (_("Can't read pathname for load map: %s."),
 		     safe_strerror (errcode));
 	  do_cleanups (old_chain);
@@ -1594,7 +1580,10 @@ svr4_current_sos (void)
 	      [...]
 		[ 9] .dynamic DYNAMIC ffffffffff700580 000580 0000f0
 	  */
-	  if (address_in_mem_range (so->lm_info->l_ld, &vsyscall_range))
+
+	  lm_info_svr4 *li = (lm_info_svr4 *) so->lm_info;
+
+	  if (address_in_mem_range (li->l_ld, &vsyscall_range))
 	    {
 	      *sop = so->next;
 	      free_so (so);
@@ -1628,7 +1617,11 @@ svr4_fetch_objfile_link_map (struct objfile *objfile)
      of shared libraries.  */
   for (so = master_so_list (); so; so = so->next)
     if (so->objfile == objfile)
-      return so->lm_info->lm_addr;
+      {
+	lm_info_svr4 *li = (lm_info_svr4 *) so->lm_info;
+
+	return li->lm_addr;
+      }
 
   /* Not found!  */
   return 0;
@@ -1860,7 +1853,9 @@ solist_update_incremental (struct svr4_info *info, CORE_ADDR lm)
   /* Walk to the end of the list.  */
   for (tail = info->solib_list; tail->next != NULL; tail = tail->next)
     /* Nothing.  */;
-  prev_lm = tail->lm_info->lm_addr;
+
+  lm_info_svr4 *li = (lm_info_svr4 *) tail->lm_info;
+  prev_lm = li->lm_addr;
 
   /* Read the new objects.  */
   if (info->using_xfer)
diff --git a/gdb/solib-svr4.h b/gdb/solib-svr4.h
index b97fbd62f5..a9b09aa56c 100644
--- a/gdb/solib-svr4.h
+++ b/gdb/solib-svr4.h
@@ -20,11 +20,34 @@
 #ifndef SOLIB_SVR4_H
 #define SOLIB_SVR4_H
 
+#include "solist.h"
+
 struct objfile;
 struct target_so_ops;
 
 extern struct target_so_ops svr4_so_ops;
 
+/* Link map info to include in an allocated so_list entry.  */
+
+struct lm_info_svr4 : public lm_info_base
+{
+  /* Amount by which addresses in the binary should be relocated to
+     match the inferior.  The direct inferior value is L_ADDR_INFERIOR.
+     When prelinking is involved and the prelink base address changes,
+     we may need a different offset - the recomputed offset is in L_ADDR.
+     It is commonly the same value.  It is cached as we want to warn about
+     the difference and compute it only once.  L_ADDR is valid
+     iff L_ADDR_P.  */
+  CORE_ADDR l_addr, l_addr_inferior;
+  unsigned int l_addr_p : 1;
+
+  /* The target location of lm.  */
+  CORE_ADDR lm_addr;
+
+  /* Values read in from inferior's fields of the same name.  */
+  CORE_ADDR l_ld, l_next, l_prev, l_name;
+};
+
 /* Critical offsets and sizes which describe struct r_debug and
    struct link_map on SVR4-like targets.  All offsets and sizes are
    in bytes unless otherwise specified.  */
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index 1b10e4ea41..e40acc1823 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -27,7 +27,7 @@
 #include "solib-target.h"
 
 /* Private data for each loaded library.  */
-struct lm_info
+struct lm_info_target : public lm_info_base
 {
   /* The library's name.  The name is normally kept in the struct
      so_list; it is only here during XML parsing.  */
@@ -49,12 +49,12 @@ struct lm_info
   struct section_offsets *offsets;
 };
 
-typedef struct lm_info *lm_info_p;
-DEF_VEC_P(lm_info_p);
+typedef lm_info_target *lm_info_target_p;
+DEF_VEC_P(lm_info_target_p);
 
 #if !defined(HAVE_LIBEXPAT)
 
-static VEC(lm_info_p) *
+static VEC(lm_info_target_p) *
 solib_target_parse_libraries (const char *library)
 {
   static int have_warned;
@@ -80,8 +80,8 @@ library_list_start_segment (struct gdb_xml_parser *parser,
 			    const struct gdb_xml_element *element,
 			    void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
-  VEC(lm_info_p) **list = (VEC(lm_info_p) **) user_data;
-  struct lm_info *last = VEC_last (lm_info_p, *list);
+  VEC(lm_info_target_p) **list = (VEC(lm_info_target_p) **) user_data;
+  lm_info_target *last = VEC_last (lm_info_target_p, *list);
   ULONGEST *address_p
     = (ULONGEST *) xml_find_attribute (attributes, "address")->value;
   CORE_ADDR address = (CORE_ADDR) *address_p;
@@ -98,8 +98,8 @@ library_list_start_section (struct gdb_xml_parser *parser,
 			    const struct gdb_xml_element *element,
 			    void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
-  VEC(lm_info_p) **list = (VEC(lm_info_p) **) user_data;
-  struct lm_info *last = VEC_last (lm_info_p, *list);
+  VEC(lm_info_target_p) **list = (VEC(lm_info_target_p) **) user_data;
+  lm_info_target *last = VEC_last (lm_info_target_p, *list);
   ULONGEST *address_p
     = (ULONGEST *) xml_find_attribute (attributes, "address")->value;
   CORE_ADDR address = (CORE_ADDR) *address_p;
@@ -118,13 +118,13 @@ library_list_start_library (struct gdb_xml_parser *parser,
 			    const struct gdb_xml_element *element,
 			    void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
-  VEC(lm_info_p) **list = (VEC(lm_info_p) **) user_data;
-  struct lm_info *item = XCNEW (struct lm_info);
+  VEC(lm_info_target_p) **list = (VEC(lm_info_target_p) **) user_data;
+  lm_info_target *item = XCNEW (lm_info_target);
   const char *name
     = (const char *) xml_find_attribute (attributes, "name")->value;
 
   item->name = xstrdup (name);
-  VEC_safe_push (lm_info_p, *list, item);
+  VEC_safe_push (lm_info_target_p, *list, item);
 }
 
 static void
@@ -132,8 +132,8 @@ library_list_end_library (struct gdb_xml_parser *parser,
 			  const struct gdb_xml_element *element,
 			  void *user_data, const char *body_text)
 {
-  VEC(lm_info_p) **list = (VEC(lm_info_p) **) user_data;
-  struct lm_info *lm_info = VEC_last (lm_info_p, *list);
+  VEC(lm_info_target_p) **list = (VEC(lm_info_target_p) **) user_data;
+  lm_info_target *lm_info = VEC_last (lm_info_target_p, *list);
 
   if (lm_info->segment_bases == NULL
       && lm_info->section_bases == NULL)
@@ -168,18 +168,18 @@ library_list_start_list (struct gdb_xml_parser *parser,
 static void
 solib_target_free_library_list (void *p)
 {
-  VEC(lm_info_p) **result = (VEC(lm_info_p) **) p;
-  struct lm_info *info;
+  VEC(lm_info_target_p) **result = (VEC(lm_info_target_p) **) p;
+  lm_info_target *info;
   int ix;
 
-  for (ix = 0; VEC_iterate (lm_info_p, *result, ix, info); ix++)
+  for (ix = 0; VEC_iterate (lm_info_target_p, *result, ix, info); ix++)
     {
       xfree (info->name);
       VEC_free (CORE_ADDR, info->segment_bases);
       VEC_free (CORE_ADDR, info->section_bases);
       xfree (info);
     }
-  VEC_free (lm_info_p, *result);
+  VEC_free (lm_info_target_p, *result);
   *result = NULL;
 }
 
@@ -229,10 +229,10 @@ static const struct gdb_xml_element library_list_elements[] = {
   { NULL, NULL, NULL, GDB_XML_EF_NONE, NULL, NULL }
 };
 
-static VEC(lm_info_p) *
+static VEC(lm_info_target_p) *
 solib_target_parse_libraries (const char *library)
 {
-  VEC(lm_info_p) *result = NULL;
+  VEC(lm_info_target_p) *result = NULL;
   struct cleanup *back_to = make_cleanup (solib_target_free_library_list,
 					  &result);
 
@@ -255,8 +255,8 @@ solib_target_current_sos (void)
   struct so_list *new_solib, *start = NULL, *last = NULL;
   char *library_document;
   struct cleanup *old_chain;
-  VEC(lm_info_p) *library_list;
-  struct lm_info *info;
+  VEC(lm_info_target_p) *library_list;
+  lm_info_target *info;
   int ix;
 
   /* Fetch the list of shared libraries.  */
@@ -279,7 +279,7 @@ solib_target_current_sos (void)
     return NULL;
 
   /* Build a struct so_list for each entry on the list.  */
-  for (ix = 0; VEC_iterate (lm_info_p, library_list, ix, info); ix++)
+  for (ix = 0; VEC_iterate (lm_info_target_p, library_list, ix, info); ix++)
     {
       new_solib = XCNEW (struct so_list);
       strncpy (new_solib->so_name, info->name, SO_NAME_MAX_PATH_SIZE - 1);
@@ -304,7 +304,7 @@ solib_target_current_sos (void)
     }
 
   /* Free the library list, but not its members.  */
-  VEC_free (lm_info_p, library_list);
+  VEC_free (lm_info_target_p, library_list);
 
   return start;
 }
@@ -324,10 +324,12 @@ solib_target_clear_solib (void)
 static void
 solib_target_free_so (struct so_list *so)
 {
-  gdb_assert (so->lm_info->name == NULL);
-  xfree (so->lm_info->offsets);
-  VEC_free (CORE_ADDR, so->lm_info->segment_bases);
-  xfree (so->lm_info);
+  lm_info_target *li = (lm_info_target *) so->lm_info;
+
+  gdb_assert (li->name == NULL);
+  xfree (li->offsets);
+  VEC_free (CORE_ADDR, li->segment_bases);
+  xfree (li);
 }
 
 static void
@@ -335,23 +337,24 @@ solib_target_relocate_section_addresses (struct so_list *so,
 					 struct target_section *sec)
 {
   CORE_ADDR offset;
+  lm_info_target *li = (lm_info_target *) so->lm_info;
 
   /* Build the offset table only once per object file.  We can not do
      it any earlier, since we need to open the file first.  */
-  if (so->lm_info->offsets == NULL)
+  if (li->offsets == NULL)
     {
       int num_sections = gdb_bfd_count_sections (so->abfd);
 
-      so->lm_info->offsets
+      li->offsets
 	= ((struct section_offsets *)
 	   xzalloc (SIZEOF_N_SECTION_OFFSETS (num_sections)));
 
-      if (so->lm_info->section_bases)
+      if (li->section_bases)
 	{
 	  int i;
 	  asection *sect;
 	  int num_section_bases
-	    = VEC_length (CORE_ADDR, so->lm_info->section_bases);
+	    = VEC_length (CORE_ADDR, li->section_bases);
 	  int num_alloc_sections = 0;
 
 	  for (i = 0, sect = so->abfd->sections;
@@ -371,7 +374,7 @@ Could not relocate shared library \"%s\": wrong number of ALLOC sections"),
 	      CORE_ADDR *section_bases;
 
 	      section_bases = VEC_address (CORE_ADDR,
-					   so->lm_info->section_bases);
+					   li->section_bases);
 
 	      so->addr_low = ~(CORE_ADDR) 0;
 	      so->addr_high = 0;
@@ -395,7 +398,7 @@ Could not relocate shared library \"%s\": wrong number of ALLOC sections"),
 		      gdb_assert (so->addr_low <= so->addr_high);
 		      found_range = 1;
 		    }
-		  so->lm_info->offsets->offsets[i]
+		  li->offsets->offsets[i]
 		    = section_bases[bases_index];
 		  bases_index++;
 		}
@@ -404,7 +407,7 @@ Could not relocate shared library \"%s\": wrong number of ALLOC sections"),
 	      gdb_assert (so->addr_low <= so->addr_high);
 	    }
 	}
-      else if (so->lm_info->segment_bases)
+      else if (li->segment_bases)
 	{
 	  struct symfile_segment_data *data;
 
@@ -419,12 +422,10 @@ Could not relocate shared library \"%s\": no segments"), so->so_name);
 	      int num_bases;
 	      CORE_ADDR *segment_bases;
 
-	      num_bases = VEC_length (CORE_ADDR, so->lm_info->segment_bases);
-	      segment_bases = VEC_address (CORE_ADDR,
-					   so->lm_info->segment_bases);
+	      num_bases = VEC_length (CORE_ADDR, li->segment_bases);
+	      segment_bases = VEC_address (CORE_ADDR, li->segment_bases);
 
-	      if (!symfile_map_offsets_to_segments (so->abfd, data,
-						    so->lm_info->offsets,
+	      if (!symfile_map_offsets_to_segments (so->abfd, data, li->offsets,
 						    num_bases, segment_bases))
 		warning (_("\
 Could not relocate shared library \"%s\": bad offsets"), so->so_name);
@@ -459,9 +460,9 @@ Could not relocate shared library \"%s\": bad offsets"), so->so_name);
 	}
     }
 
-  offset = so->lm_info->offsets->offsets[gdb_bfd_section_index
-					 (sec->the_bfd_section->owner,
-					  sec->the_bfd_section)];
+  offset = li->offsets->offsets[gdb_bfd_section_index
+			        (sec->the_bfd_section->owner,
+				 sec->the_bfd_section)];
   sec->addr += offset;
   sec->endaddr += offset;
 }
diff --git a/gdb/solist.h b/gdb/solist.h
index 378d60d7b8..54c9902b4a 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -29,9 +29,11 @@
 	 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;
+/* Base class for target-specific link map information.  */
+
+struct lm_info_base
+{
+};
 
 struct so_list
   {
@@ -45,7 +47,7 @@ struct so_list
        will be a copy of struct link_map from the user process, but
        it need not be; it can be any collection of data needed to
        traverse the dynamic linker's data structures.  */
-    struct lm_info *lm_info;
+    lm_info_base *lm_info;
 
     /* Shared object file name, exactly as it appears in the
        inferior's link map.  This may be a relative path, or something
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 805fb4358d..ef1c2914f1 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -585,7 +585,7 @@ struct safe_symbol_file_add_args
 };
 
 /* Maintain a linked list of "so" information.  */
-struct lm_info
+struct lm_info_windows : public lm_info_base
 {
   LPVOID load_addr;
 };
@@ -645,8 +645,9 @@ windows_make_so (const char *name, LPVOID load_addr)
     }
 #endif
   so = XCNEW (struct so_list);
-  so->lm_info = XNEW (struct lm_info);
-  so->lm_info->load_addr = load_addr;
+  lm_info_windows *li = XCNEW (struct lm_info_windows);
+  so->lm_info = li;
+  li->load_addr = load_addr;
   strcpy (so->so_original_name, name);
 #ifndef __CYGWIN__
   strcpy (so->so_name, buf);
@@ -772,8 +773,10 @@ handle_load_dll (void *dummy)
   solib_end->next = windows_make_so (dll_name, event->lpBaseOfDll);
   solib_end = solib_end->next;
 
+  lm_info_windows *li = (lm_info_windows *) solib_end->lm_info;
+
   DEBUG_EVENTS (("gdb: Loading dll \"%s\" at %s.\n", solib_end->so_name,
-		 host_address_to_string (solib_end->lm_info->load_addr)));
+		 host_address_to_string (li->load_addr)));
 
   return 1;
 }
@@ -801,18 +804,22 @@ handle_unload_dll (void *dummy)
   struct so_list *so;
 
   for (so = &solib_start; so->next != NULL; so = so->next)
-    if (so->next->lm_info->load_addr == lpBaseOfDll)
-      {
-	struct so_list *sodel = so->next;
+    {
+      lm_info_windows *li_next = (lm_info_windows *) so->next->lm_info;
 
-	so->next = sodel->next;
-	if (!so->next)
-	  solib_end = so;
-	DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n", sodel->so_name));
+      if (li_next->load_addr == lpBaseOfDll)
+	{
+	  struct so_list *sodel = so->next;
 
-	windows_free_so (sodel);
-	return 1;
-      }
+	  so->next = sodel->next;
+	  if (!so->next)
+	    solib_end = so;
+	  DEBUG_EVENTS (("gdb: Unloading dll \"%s\".\n", sodel->so_name));
+
+	  windows_free_so (sodel);
+	  return 1;
+	}
+    }
 
   /* We did not find any DLL that was previously loaded at this address,
      so register a complaint.  We do not report an error, because we have
@@ -2842,9 +2849,13 @@ windows_xfer_shared_libraries (struct target_ops *ops,
   obstack_init (&obstack);
   obstack_grow_str (&obstack, "<library-list>\n");
   for (so = solib_start.next; so; so = so->next)
-    windows_xfer_shared_library (so->so_name, (CORE_ADDR)
-				 (uintptr_t) so->lm_info->load_addr,
-				 target_gdbarch (), &obstack);
+    {
+      lm_info_windows *li = (lm_info_windows *) so->lm_info;
+
+      windows_xfer_shared_library (so->so_name, (CORE_ADDR)
+				   (uintptr_t) li->load_addr,
+				   target_gdbarch (), &obstack);
+    }
   obstack_grow_str0 (&obstack, "</library-list>\n");
 
   buf = (const char *) obstack_finish (&obstack);
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 03/10] Class-ify lm_info_aix
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
  2017-04-26 22:47 ` [PATCH 02/10] Make various lm_info implementations inherit from a base class Simon Marchi
@ 2017-04-26 22:47 ` Simon Marchi
  2017-04-28 15:58   ` Pedro Alves
  2017-04-26 22:47 ` [PATCH 04/10] Class-ify lm_info_dsbt Simon Marchi
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

This patch makes lm_info_aix a "real" class.  It uses std::string,
initializes fields in-class and replaces XCNEW/xfree with new/delete.
The solib_aix_new_lm_info can be replaced by using the default copy
constructor.

gdb/ChangeLog:

	* solib-aix.c (struct lm_info_aix): Initialize fields in-class.
	<filename, member_name>: Change type to std::string.
	(solib_aix_new_lm_info, solib_aix_xfree_lm_info): Remove.
	(library_list_start_library): Allocate lm_info_aix with new.
	(solib_aix_free_library_list, solib_aix_free_so): Free with delete.
	(solib_aix_current_sos): Adapt to std::string, copy lm_info_aix
	with copy constructor.
---
 gdb/solib-aix.c | 62 +++++++++++++++++++--------------------------------------
 1 file changed, 21 insertions(+), 41 deletions(-)

diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 54b8c38032..d0f5e96119 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -39,53 +39,28 @@ struct lm_info_aix : public lm_info_base
      for the main executable, this is usually a shared library (which,
      on AIX, is an archive library file, created using the "ar"
      command).  */
-  char *filename;
+  std::string filename;
 
   /* The name of the shared object file with the actual dynamic
-     loading dependency.  This may be NULL (Eg. main executable).  */
-  char *member_name;
+     loading dependency.  This may be empty (Eg. main executable).  */
+  std::string member_name;
 
   /* The address in inferior memory where the text section got mapped.  */
-  CORE_ADDR text_addr;
+  CORE_ADDR text_addr = 0;
 
   /* The size of the text section, obtained via the loader data.  */
-  ULONGEST text_size;
+  ULONGEST text_size = 0;
 
   /* The address in inferior memory where the data section got mapped.  */
-  CORE_ADDR data_addr;
+  CORE_ADDR data_addr = 0;
 
   /* The size of the data section, obtained via the loader data.  */
-  ULONGEST data_size;
+  ULONGEST data_size = 0;
 };
 
 typedef lm_info_aix *lm_info_aix_p;
 DEF_VEC_P(lm_info_aix_p);
 
-/* Return a deep copy of the given struct lm_info object.  */
-
-static lm_info_aix *
-solib_aix_new_lm_info (lm_info_aix *info)
-{
-  lm_info_aix *result = XCNEW (lm_info_aix);
-
-  memcpy (result, info, sizeof (lm_info_aix));
-  result->filename = xstrdup (info->filename);
-  if (info->member_name != NULL)
-    result->member_name = xstrdup (info->member_name);
-
-  return result;
-}
-
-/* Free the memory allocated for the given lm_info.  */
-
-static void
-solib_aix_xfree_lm_info (lm_info_aix *info)
-{
-  xfree (info->filename);
-  xfree (info->member_name);
-  xfree (info);
-}
-
 /* This module's per-inferior data.  */
 
 struct solib_aix_inferior_data
@@ -162,7 +137,7 @@ library_list_start_library (struct gdb_xml_parser *parser,
 			    VEC (gdb_xml_value_s) *attributes)
 {
   VEC (lm_info_aix_p) **list = (VEC (lm_info_aix_p) **) user_data;
-  lm_info_aix *item = XCNEW (lm_info_aix);
+  lm_info_aix *item = new lm_info_aix;
   struct gdb_xml_value *attr;
 
   attr = xml_find_attribute (attributes, "name");
@@ -215,7 +190,8 @@ solib_aix_free_library_list (void *p)
     fprintf_unfiltered (gdb_stdlog, "DEBUG: solib_aix_free_library_list\n");
 
   for (ix = 0; VEC_iterate (lm_info_aix_p, *result, ix, info); ix++)
-    solib_aix_xfree_lm_info (info);
+    delete info;
+
   VEC_free (lm_info_aix_p, *result);
   *result = NULL;
 }
@@ -443,10 +419,13 @@ solib_aix_relocate_section_addresses (struct so_list *so,
 static void
 solib_aix_free_so (struct so_list *so)
 {
+  lm_info_aix *li = (lm_info_aix *) so->lm_info;
+
   if (solib_aix_debug)
     fprintf_unfiltered (gdb_stdlog, "DEBUG: solib_aix_free_so (%s)\n",
 			so->so_name);
-  solib_aix_xfree_lm_info ((lm_info_aix *) so->lm_info);
+
+  delete li;
 }
 
 /* Implement the "clear_solib" target_so_ops method.  */
@@ -568,15 +547,15 @@ solib_aix_current_sos (void)
   for (ix = 1; VEC_iterate (lm_info_aix_p, library_list, ix, info); ix++)
     {
       struct so_list *new_solib = XCNEW (struct so_list);
-      char *so_name;
+      std::string so_name;
 
-      if (info->member_name == NULL)
+      if (info->member_name.empty ())
 	{
 	 /* INFO->FILENAME is probably not an archive, but rather
 	    a shared object.  Unusual, but it should be possible
 	    to link a program against a shared object directory,
 	    without having to put it in an archive first.  */
-	 so_name = xstrdup (info->filename);
+	 so_name = info->filename;
 	}
       else
 	{
@@ -584,14 +563,15 @@ solib_aix_current_sos (void)
 	    is a member of an archive.  Create a synthetic so_name
 	    that follows the same convention as AIX's ldd tool
 	    (Eg: "/lib/libc.a(shr.o)").  */
-	 so_name = xstrprintf ("%s(%s)", info->filename, info->member_name);
+	 so_name = string_printf ("%s(%s)", info->filename.c_str (),
+				  info->member_name.c_str ());
 	}
-      strncpy (new_solib->so_original_name, so_name,
+      strncpy (new_solib->so_original_name, so_name.c_str (),
 	       SO_NAME_MAX_PATH_SIZE - 1);
       new_solib->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       memcpy (new_solib->so_name, new_solib->so_original_name,
 	      SO_NAME_MAX_PATH_SIZE);
-      new_solib->lm_info = solib_aix_new_lm_info (info);
+      new_solib->lm_info = new lm_info_aix (*info);
 
       /* Add it to the list.  */
       if (!start)
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 00/10] Make lm_info class hierarchy
@ 2017-04-26 22:47 Simon Marchi
  2017-04-26 22:47 ` [PATCH 02/10] Make various lm_info implementations inherit from a base class Simon Marchi
                   ` (10 more replies)
  0 siblings, 11 replies; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch series makes a class hierarchy out of all the lm_info
implementations.  This is done to address the problem identified by Pedro in
[1].

What I did corresponds to suggestion #1 in Pedro's message.  #3 would probably
be better, but it's not straightforward.  Some implementations keep their own
list of lm_info, separate from the global (per pspace) so_list.  So if we
eliminate the lm_info classes and replace them with classes that extend
so_list, it's not clear what we should do with these private lists.

So, suggestion #1 is a small step forward, and puts us in good position if we
want to do #3 one day.  We would then have to change the lm_info_* classes to
extend so_list instead of lm_info (and probably rename them).

This series went through the buildbot (in its almost final form), it contains
code that the buildbot doesn't test, and that is difficult for most of us to
test.

[1] https://sourceware.org/ml/gdb-patches/2017-04/msg00559.html

Simon Marchi (10):
  Standardize darwin's lm_info
  Make various lm_info implementations inherit from a base class
  Class-ify lm_info_aix
  Class-ify lm_info_dsbt
  Fix indentation of lm_info_frv
  Class-ify lm_info_frv
  Class-ify lm_info_target
  Class-ify lm_info_svr4
  Class-ify lm_info_darwin
  Class-ify lm_info_windows

 gdb/nto-tdep.c     |  35 +---------------
 gdb/solib-aix.c    | 110 +++++++++++++++++++++-----------------------------
 gdb/solib-darwin.c |  34 ++++++++--------
 gdb/solib-dsbt.c   |  38 ++++++++++--------
 gdb/solib-frv.c    | 109 ++++++++++++++++++++++++++------------------------
 gdb/solib-svr4.c   | 115 ++++++++++++++++++++++++++---------------------------
 gdb/solib-svr4.h   |  23 +++++++++++
 gdb/solib-target.c | 114 ++++++++++++++++++++++++++--------------------------
 gdb/solist.h       |  10 +++--
 gdb/windows-nat.c  |  54 +++++++++++++++----------
 10 files changed, 319 insertions(+), 323 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 06/10] Class-ify lm_info_frv
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (4 preceding siblings ...)
  2017-04-26 22:52 ` [PATCH 05/10] Fix indentation of lm_info_frv Simon Marchi
@ 2017-04-26 22:52 ` Simon Marchi
  2017-04-28 15:59   ` Pedro Alves
  2017-04-26 22:53 ` [PATCH 08/10] Class-ify lm_info_svr4 Simon Marchi
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:52 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes lm_info_frv a "real" class.  It adds a destructor,
initializes the fields and replaces XCNEW/xfree with new/delete.

gdb/ChangeLog:

	* solib-frv.c (struct lm_info_frv): Add destructor, initialize
	fields.
	(frv_current_sos): Allocate lm_info_frv with new.
	(frv_relocate_main_executable): Free lm_info_frv with delete,
	allocate with new.
	(frv_clear_solib, frv_free_so): Free lm_info_frv with delete.
---
 gdb/solib-frv.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index e11de253b0..85d980f84c 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -204,13 +204,19 @@ struct ext_link_map
 
 struct lm_info_frv : public lm_info_base
 {
+  ~lm_info_frv ()
+  {
+    xfree (this->map);
+    xfree (this->dyn_syms);
+    xfree (this->dyn_relocs);
+  }
 
   /* The loadmap, digested into an easier to use form.  */
-  struct int_elf32_fdpic_loadmap *map;
+  int_elf32_fdpic_loadmap *map = NULL;
   /* The GOT address for this link map entry.  */
-  CORE_ADDR got_value;
+  CORE_ADDR got_value = 0;
   /* The link map address, needed for frv_fetch_objfile_link_map().  */
-  CORE_ADDR lm_addr;
+  CORE_ADDR lm_addr = 0;
 
   /* Cached dynamic symbol table and dynamic relocs initialized and
      used only by find_canonical_descriptor_in_load_object().
@@ -222,10 +228,9 @@ struct lm_info_frv : public lm_info_base
      supplied to the first call.  Thus the caching of the dynamic
      symbols (dyn_syms) is critical for correct operation.  The
      caching of the dynamic relocations could be dispensed with.  */
-  asymbol **dyn_syms;
-  arelent **dyn_relocs;
-  int dyn_reloc_count;	/* Number of dynamic relocs.  */
-
+  asymbol **dyn_syms = NULL;
+  arelent **dyn_relocs = NULL;
+  int dyn_reloc_count = 0;	/* Number of dynamic relocs.  */
 };
 
 /* The load map, got value, etc. are not available from the chain
@@ -390,7 +395,7 @@ frv_current_sos (void)
 	    }
 
 	  sop = XCNEW (struct so_list);
-	  lm_info_frv *li = XCNEW (lm_info_frv);
+	  lm_info_frv *li = new lm_info_frv;
 	  sop->lm_info = li;
 	  li->map = loadmap;
 	  li->got_value = got_addr;
@@ -783,9 +788,9 @@ frv_relocate_main_executable (void)
   if (ldm == NULL)
     error (_("Unable to load the executable's loadmap."));
 
-  if (main_executable_lm_info)
-    xfree (main_executable_lm_info);
-  main_executable_lm_info = XCNEW (lm_info_frv);
+  if (main_executable_lm_info != NULL)
+    delete main_executable_lm_info;
+  main_executable_lm_info = new lm_info_frv;
   main_executable_lm_info->map = ldm;
 
   new_offsets = XCNEWVEC (struct section_offsets,
@@ -859,13 +864,10 @@ frv_clear_solib (void)
   lm_base_cache = 0;
   enable_break2_done = 0;
   main_lm_addr = 0;
-  if (main_executable_lm_info != 0)
+  if (main_executable_lm_info != NULL)
     {
-      xfree (main_executable_lm_info->map);
-      xfree (main_executable_lm_info->dyn_syms);
-      xfree (main_executable_lm_info->dyn_relocs);
-      xfree (main_executable_lm_info);
-      main_executable_lm_info = 0;
+      delete main_executable_lm_info;
+      main_executable_lm_info = NULL;
     }
 }
 
@@ -874,10 +876,7 @@ frv_free_so (struct so_list *so)
 {
   lm_info_frv *li = (lm_info_frv *) so->lm_info;
 
-  xfree (li->map);
-  xfree (li->dyn_syms);
-  xfree (li->dyn_relocs);
-  xfree (li);
+  delete li;
 }
 
 static void
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 05/10] Fix indentation of lm_info_frv
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (3 preceding siblings ...)
  2017-04-26 22:47 ` [PATCH 01/10] Standardize darwin's lm_info Simon Marchi
@ 2017-04-26 22:52 ` Simon Marchi
  2017-04-28 15:59   ` Pedro Alves
  2017-04-26 22:52 ` [PATCH 06/10] Class-ify lm_info_frv Simon Marchi
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:52 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch fixes the indentation of lm_info_frv, so that the real
changes of the following patch are not lost in the reformatting.

gdb/ChangeLog:

	* solib-frv.c (struct lm_info_frv): Fix indentation.
---
 gdb/solib-frv.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 0108d97815..e11de253b0 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -203,29 +203,30 @@ struct ext_link_map
 /* Link map info to include in an allocated so_list entry.  */
 
 struct lm_info_frv : public lm_info_base
-  {
-    /* The loadmap, digested into an easier to use form.  */
-    struct int_elf32_fdpic_loadmap *map;
-    /* The GOT address for this link map entry.  */
-    CORE_ADDR got_value;
-    /* The link map address, needed for frv_fetch_objfile_link_map().  */
-    CORE_ADDR lm_addr;
-
-    /* Cached dynamic symbol table and dynamic relocs initialized and
-       used only by find_canonical_descriptor_in_load_object().
-
-       Note: kevinb/2004-02-26: It appears that calls to
-       bfd_canonicalize_dynamic_reloc() will use the same symbols as
-       those supplied to the first call to this function.  Therefore,
-       it's important to NOT free the asymbol ** data structure
-       supplied to the first call.  Thus the caching of the dynamic
-       symbols (dyn_syms) is critical for correct operation.  The
-       caching of the dynamic relocations could be dispensed with.  */
-    asymbol **dyn_syms;
-    arelent **dyn_relocs;
-    int dyn_reloc_count;	/* Number of dynamic relocs.  */
-
-  };
+{
+
+  /* The loadmap, digested into an easier to use form.  */
+  struct int_elf32_fdpic_loadmap *map;
+  /* The GOT address for this link map entry.  */
+  CORE_ADDR got_value;
+  /* The link map address, needed for frv_fetch_objfile_link_map().  */
+  CORE_ADDR lm_addr;
+
+  /* Cached dynamic symbol table and dynamic relocs initialized and
+     used only by find_canonical_descriptor_in_load_object().
+
+     Note: kevinb/2004-02-26: It appears that calls to
+     bfd_canonicalize_dynamic_reloc() will use the same symbols as
+     those supplied to the first call to this function.  Therefore,
+     it's important to NOT free the asymbol ** data structure
+     supplied to the first call.  Thus the caching of the dynamic
+     symbols (dyn_syms) is critical for correct operation.  The
+     caching of the dynamic relocations could be dispensed with.  */
+  asymbol **dyn_syms;
+  arelent **dyn_relocs;
+  int dyn_reloc_count;	/* Number of dynamic relocs.  */
+
+};
 
 /* The load map, got value, etc. are not available from the chain
    of loaded shared objects.  ``main_executable_lm_info'' provides
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 10/10] Class-ify lm_info_windows
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (7 preceding siblings ...)
  2017-04-26 22:53 ` [PATCH 07/10] Class-ify lm_info_target Simon Marchi
@ 2017-04-26 22:53 ` Simon Marchi
  2017-04-28 16:01   ` Pedro Alves
  2017-04-26 22:54 ` [PATCH 09/10] Class-ify lm_info_darwin Simon Marchi
  2017-04-28 16:08 ` [PATCH 00/10] Make lm_info class hierarchy Pedro Alves
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes lm_info_windows a "real" class.  It initializes the field
and replaces XCNEW/xfree with new/delete.

gdb/ChangeLog:

	* windows-nat.c (struct lm_info_windows): Initialize field.
	(windows_make_so): Allocate lm_info_windows with new.
	(windows_free_so): Free lm_info_windows with delete.
---
 gdb/windows-nat.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index ef1c2914f1..c54c9ee5d5 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -587,7 +587,7 @@ struct safe_symbol_file_add_args
 /* Maintain a linked list of "so" information.  */
 struct lm_info_windows : public lm_info_base
 {
-  LPVOID load_addr;
+  LPVOID load_addr = 0;
 };
 
 static struct so_list solib_start, *solib_end;
@@ -645,7 +645,7 @@ windows_make_so (const char *name, LPVOID load_addr)
     }
 #endif
   so = XCNEW (struct so_list);
-  lm_info_windows *li = XCNEW (struct lm_info_windows);
+  lm_info_windows *li = new lm_info_windows;
   so->lm_info = li;
   li->load_addr = load_addr;
   strcpy (so->so_original_name, name);
@@ -784,8 +784,11 @@ handle_load_dll (void *dummy)
 static void
 windows_free_so (struct so_list *so)
 {
-  if (so->lm_info)
-    xfree (so->lm_info);
+  lm_info_windows *li = (lm_info_windows *) so->lm_info;
+
+  if (li != NULL)
+    delete li;
+
   xfree (so);
 }
 
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 08/10] Class-ify lm_info_svr4
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (5 preceding siblings ...)
  2017-04-26 22:52 ` [PATCH 06/10] Class-ify lm_info_frv Simon Marchi
@ 2017-04-26 22:53 ` Simon Marchi
  2017-04-28 16:00   ` Pedro Alves
  2017-04-26 22:53 ` [PATCH 07/10] Class-ify lm_info_target Simon Marchi
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes lm_info_svr4 a "real" class.  It initializes fields,
uses bool and replaces XCNEW/xfree with new/delete.

The memcpy in svr4_copy_library_list is replaced by a usage of the
default copy constructor.

gdb/ChangeLog:

	* solib-svr4.h (struct lm_info_svr4): Initialize fields.
	<l_addr_p>: Change type to bool.
	* solib-svr4.c (lm_info_read): Allocate lm_info_svr4 with new.
	(svr4_free_so): Free lm_info_svr4 with delete.
	(svr4_copy_library_list): Replace memcpy with call to copy
	constructor.
	(library_list_start_library, svr4_default_sos): Allocate
	lm_info_svr4 with new.
---
 gdb/solib-svr4.c | 14 ++++++++------
 gdb/solib-svr4.h |  8 ++++----
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6098d505da..080fd795ed 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -189,7 +189,7 @@ lm_info_read (CORE_ADDR lm_addr)
     {
       struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
 
-      lm_info = XCNEW (lm_info_svr4);
+      lm_info = new lm_info_svr4;
       lm_info->lm_addr = lm_addr;
 
       lm_info->l_addr_inferior = extract_typed_address (&lm[lmo->l_addr_offset],
@@ -1080,7 +1080,9 @@ struct svr4_library_list
 static void
 svr4_free_so (struct so_list *so)
 {
-  xfree (so->lm_info);
+  lm_info_svr4 *li = (lm_info_svr4 *) so->lm_info;
+
+  delete li;
 }
 
 /* Implement target_so_ops.clear_so.  */
@@ -1125,8 +1127,8 @@ svr4_copy_library_list (struct so_list *src)
       newobj = XNEW (struct so_list);
       memcpy (newobj, src, sizeof (struct so_list));
 
-      newobj->lm_info = XNEW (lm_info_svr4);
-      memcpy (newobj->lm_info, src->lm_info, sizeof (lm_info_svr4));
+      lm_info_svr4 *src_li = (lm_info_svr4 *) src->lm_info;
+      newobj->lm_info = new lm_info_svr4 (*src_li);
 
       newobj->next = NULL;
       *link = newobj;
@@ -1162,7 +1164,7 @@ library_list_start_library (struct gdb_xml_parser *parser,
   struct so_list *new_elem;
 
   new_elem = XCNEW (struct so_list);
-  lm_info_svr4 *li = XCNEW (lm_info_svr4);
+  lm_info_svr4 *li = new lm_info_svr4;
   new_elem->lm_info = li;
   li->lm_addr = *lmp;
   li->l_addr_inferior = *l_addrp;
@@ -1317,7 +1319,7 @@ svr4_default_sos (void)
     return NULL;
 
   newobj = XCNEW (struct so_list);
-  lm_info_svr4 *li = XCNEW (lm_info_svr4);
+  lm_info_svr4 *li = new lm_info_svr4;
   newobj->lm_info = li;
 
   /* Nothing will ever check the other fields if we set l_addr_p.  */
diff --git a/gdb/solib-svr4.h b/gdb/solib-svr4.h
index a9b09aa56c..705c587d0a 100644
--- a/gdb/solib-svr4.h
+++ b/gdb/solib-svr4.h
@@ -38,14 +38,14 @@ struct lm_info_svr4 : public lm_info_base
      It is commonly the same value.  It is cached as we want to warn about
      the difference and compute it only once.  L_ADDR is valid
      iff L_ADDR_P.  */
-  CORE_ADDR l_addr, l_addr_inferior;
-  unsigned int l_addr_p : 1;
+  CORE_ADDR l_addr = 0, l_addr_inferior = 0;
+  bool l_addr_p = false;
 
   /* The target location of lm.  */
-  CORE_ADDR lm_addr;
+  CORE_ADDR lm_addr = 0;
 
   /* Values read in from inferior's fields of the same name.  */
-  CORE_ADDR l_ld, l_next, l_prev, l_name;
+  CORE_ADDR l_ld = 0, l_next = 0, l_prev = 0, l_name = 0;
 };
 
 /* Critical offsets and sizes which describe struct r_debug and
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 07/10] Class-ify lm_info_target
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (6 preceding siblings ...)
  2017-04-26 22:53 ` [PATCH 08/10] Class-ify lm_info_svr4 Simon Marchi
@ 2017-04-26 22:53 ` Simon Marchi
  2017-04-28 16:00   ` Pedro Alves
  2017-04-26 22:53 ` [PATCH 10/10] Class-ify lm_info_windows Simon Marchi
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes lm_info_target a "real" class.  It adds a destructor,
uses std::string, initializes the fields and replaces XCNEW/xfree with
new/delete.

gdb/ChangeLog:

	* solib-target.c (struct lm_info_target): Add destructor,
	initialize fields.
	<name>: Change type to std::string.
	(library_list_start_library): Allocate lm_info_target with new.
	(solib_target_free_library_list): Free lm_info_target with
	delete.
	(solib_target_current_sos): Adapt to std::string.
	(solib_target_free_so): Free lm_info_target with delete.
---
 gdb/solib-target.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index e40acc1823..e25f6ab252 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -29,24 +29,30 @@
 /* Private data for each loaded library.  */
 struct lm_info_target : public lm_info_base
 {
+  ~lm_info_target ()
+  {
+    VEC_free (CORE_ADDR, this->segment_bases);
+    VEC_free (CORE_ADDR, this->section_bases);
+  }
+
   /* The library's name.  The name is normally kept in the struct
      so_list; it is only here during XML parsing.  */
-  char *name;
+  std::string name;
 
   /* The target can either specify segment bases or section bases, not
      both.  */
 
   /* The base addresses for each independently relocatable segment of
      this shared library.  */
-  VEC(CORE_ADDR) *segment_bases;
+  VEC(CORE_ADDR) *segment_bases = NULL;
 
   /* The base addresses for each independently allocatable,
      relocatable section of this shared library.  */
-  VEC(CORE_ADDR) *section_bases;
+  VEC(CORE_ADDR) *section_bases = NULL;
 
   /* The cached offsets for each section of this shared library,
      determined from SEGMENT_BASES, or SECTION_BASES.  */
-  struct section_offsets *offsets;
+  section_offsets *offsets = NULL;
 };
 
 typedef lm_info_target *lm_info_target_p;
@@ -119,7 +125,7 @@ library_list_start_library (struct gdb_xml_parser *parser,
 			    void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
   VEC(lm_info_target_p) **list = (VEC(lm_info_target_p) **) user_data;
-  lm_info_target *item = XCNEW (lm_info_target);
+  lm_info_target *item = new lm_info_target;
   const char *name
     = (const char *) xml_find_attribute (attributes, "name")->value;
 
@@ -173,12 +179,8 @@ solib_target_free_library_list (void *p)
   int ix;
 
   for (ix = 0; VEC_iterate (lm_info_target_p, *result, ix, info); ix++)
-    {
-      xfree (info->name);
-      VEC_free (CORE_ADDR, info->segment_bases);
-      VEC_free (CORE_ADDR, info->section_bases);
-      xfree (info);
-    }
+    delete info;
+
   VEC_free (lm_info_target_p, *result);
   *result = NULL;
 }
@@ -282,16 +284,16 @@ solib_target_current_sos (void)
   for (ix = 0; VEC_iterate (lm_info_target_p, library_list, ix, info); ix++)
     {
       new_solib = XCNEW (struct so_list);
-      strncpy (new_solib->so_name, info->name, SO_NAME_MAX_PATH_SIZE - 1);
+      strncpy (new_solib->so_name, info->name.c_str (),
+	       SO_NAME_MAX_PATH_SIZE - 1);
       new_solib->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
-      strncpy (new_solib->so_original_name, info->name,
+      strncpy (new_solib->so_original_name, info->name.c_str (),
 	       SO_NAME_MAX_PATH_SIZE - 1);
       new_solib->so_original_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       new_solib->lm_info = info;
 
       /* We no longer need this copy of the name.  */
-      xfree (info->name);
-      info->name = NULL;
+      info->name.clear ();
 
       /* Add it to the list.  */
       if (!start)
@@ -326,10 +328,9 @@ solib_target_free_so (struct so_list *so)
 {
   lm_info_target *li = (lm_info_target *) so->lm_info;
 
-  gdb_assert (li->name == NULL);
-  xfree (li->offsets);
-  VEC_free (CORE_ADDR, li->segment_bases);
-  xfree (li);
+  gdb_assert (li->name.empty ());
+
+  delete li;
 }
 
 static void
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 09/10] Class-ify lm_info_darwin
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (8 preceding siblings ...)
  2017-04-26 22:53 ` [PATCH 10/10] Class-ify lm_info_windows Simon Marchi
@ 2017-04-26 22:54 ` Simon Marchi
  2017-04-28 16:00   ` Pedro Alves
  2017-04-28 16:08 ` [PATCH 00/10] Make lm_info class hierarchy Pedro Alves
  10 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-26 22:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes lm_info_darwin a "real" class.  It initializes the
field and replaces XCNEW/xfree with new/delete.

I believe the cleanup in darwin_current_sos can be removed, I don't see
anything that can throw after the allocation and the call to
discard_cleanups.

gdb/ChangeLog:

	* solib-darwin.c (struct lm_info_darwin): Initialize field.
	(darwin_current_sos): Allocate lm_info_darwin with new, remove
	cleanup.
	(darwin_free_so): Free lm_info_darwin with delete.
---
 gdb/solib-darwin.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 03211cfb92..0e20dc4fc5 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -41,6 +41,8 @@
 #include "mach-o.h"
 #include "mach-o/external.h"
 
+#include <memory>
+
 struct gdb_dyld_image_info
 {
   /* Base address (which corresponds to the Mach-O header).  */
@@ -156,7 +158,7 @@ darwin_load_image_infos (struct darwin_info *info)
 struct lm_info_darwin : public lm_info_base
 {
   /* The target location of lm.  */
-  CORE_ADDR lm_addr;
+  CORE_ADDR lm_addr = 0;
 };
 
 /* Lookup the value for a specific symbol.  */
@@ -296,7 +298,7 @@ darwin_current_sos (void)
       newobj = XCNEW (struct so_list);
       old_chain = make_cleanup (xfree, newobj);
 
-      lm_info_darwin *li = XCNEW (lm_info_darwin);
+      lm_info_darwin *li = new lm_info_darwin;
       newobj->lm_info = li;
 
       strncpy (newobj->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
@@ -578,7 +580,9 @@ darwin_clear_solib (void)
 static void
 darwin_free_so (struct so_list *so)
 {
-  xfree (so->lm_info);
+  lm_info_darwin *li = (lm_info_darwin *) so->lm_info;
+
+  delete li;
 }
 
 /* The section table is built from bfd sections using bfd VMAs.
-- 
2.11.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 01/10] Standardize darwin's lm_info
  2017-04-26 22:47 ` [PATCH 01/10] Standardize darwin's lm_info Simon Marchi
@ 2017-04-28 15:57   ` Pedro Alves
  2017-04-28 19:32     ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 15:57 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Simon Marchi

On 04/26/2017 11:46 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> Darwin's lm_info structure is used a little bit differently than the
> other solib implementations.  The other implementations first allocate
> an so_list object, then instanciate their specific lm_info structure,
> and assign it to so_list::lm_info.
> 
> The Darwin implementation allocates both at the same time
> (darwin_so_list).  This patch changes it to be like the others, so that
> we'll be able to do some generalizations later.

Funny, so this port was already "inheriting" from so_list.

LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 02/10] Make various lm_info implementations inherit from a base class
  2017-04-26 22:47 ` [PATCH 02/10] Make various lm_info implementations inherit from a base class Simon Marchi
@ 2017-04-28 15:57   ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 15:57 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Simon Marchi

On 04/26/2017 11:46 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> The lm_info structure is used to store target specific information about
> mapped libraries.  It is currently defined as an opaque type in solist.h
> and a pointer to it is included in solist, the target-agnostic object
> representing a loaded shared library.  Multiple targets define their own
> implementation of lm_info.
> 
> In anticipation of using C++ stuff (e.g. vector) in the lm_info objects,
> we first need to avoid different definitions of classes with the same
> name (which violates the one definition rule).  This patch does it by
> having a base class (lm_info_base) from which all the specific lm_info
> derive.  Each implementation is renamed to something that makes sense
> (e.g. lm_info_aix for AIX).  The next logical step would probably be to
> derive directly from so_list, but it's not really obvious, so I'll keep
> that for another day.
> 
> One special case is the Neutrino (nto) support.  It uses SVR4-style
> libraries, but overrides some methods.  To do that, it needed to have
> its own copy of SVR4's lm_info structure in nto-tdep.c, because it was
> just not possible to put it in solib-svr4.h and include that file.  Over
> time, that copy got out of sync, which is still the case today.  I can
> only assume that the lm_addr function in nto-tdep.c is broken right now.
> The first field of the old lm_info was a pointer (gdb_byte *), whereas
> in the new lm_info it's an address in the inferior (CORE_ADDR).  Trying
> to use that field today probably results in a crash.  With this
> refactor, it's now possible to put lm_info_svr4 in solib-svr4.h and just
> include it.  I have adapted the code in nto-tdep.c to that it builds,
> but it's probably not correct.  Since I don't have the knowledge nor
> setup to try this on Neutrino, somebody else would have to fix it.  But
> I am confident that I am not making things worse than they already are.

LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 03/10] Class-ify lm_info_aix
  2017-04-26 22:47 ` [PATCH 03/10] Class-ify lm_info_aix Simon Marchi
@ 2017-04-28 15:58   ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 15:58 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Simon Marchi

On 04/26/2017 11:46 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> This patch makes lm_info_aix a "real" class.  It uses std::string,
> initializes fields in-class and replaces XCNEW/xfree with new/delete.
> The solib_aix_new_lm_info can be replaced by using the default copy
> constructor.

Nice.  LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 05/10] Fix indentation of lm_info_frv
  2017-04-26 22:52 ` [PATCH 05/10] Fix indentation of lm_info_frv Simon Marchi
@ 2017-04-28 15:59   ` Pedro Alves
  2017-04-28 21:02     ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 15:59 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

LGTM.

On 04/26/2017 11:51 PM, Simon Marchi wrote:
> +  int dyn_reloc_count;	/* Number of dynamic relocs.  */
> +
> +};

You may want to remove that spurious newline while at it.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 06/10] Class-ify lm_info_frv
  2017-04-26 22:52 ` [PATCH 06/10] Class-ify lm_info_frv Simon Marchi
@ 2017-04-28 15:59   ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 15:59 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 04/26/2017 11:51 PM, Simon Marchi wrote:
> This patch makes lm_info_frv a "real" class.  It adds a destructor,
> initializes the fields and replaces XCNEW/xfree with new/delete.
> 

LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 04/10] Class-ify lm_info_dsbt
  2017-04-26 22:47 ` [PATCH 04/10] Class-ify lm_info_dsbt Simon Marchi
@ 2017-04-28 15:59   ` Pedro Alves
  2017-04-28 20:57     ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 15:59 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Simon Marchi

On 04/26/2017 11:47 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> This patch makes lm_info_dsbt a "real" class.  It introduces a
> destructor, initializes the field and replaces XCNEW/xfree with
> new/delete.

LGTM.

> @@ -930,8 +935,9 @@ dsbt_relocate_main_executable (void)
>    dsbt_get_initial_loadmaps ();
>    ldm = info->exec_loadmap;
>  
> -  xfree (info->main_executable_lm_info);
> -  info->main_executable_lm_info = XCNEW (lm_info_dsbt);
> +  if (info->main_executable_lm_info != NULL)
> +    delete info->main_executable_lm_info;

Note that like with xfree, it's fine to call delete
on a NULL pointer.  I spotted the same pattern on several
of the following patches in the series.

> +  info->main_executable_lm_info = new lm_info_dsbt;
>    info->main_executable_lm_info->map = ldm;
>  


Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 08/10] Class-ify lm_info_svr4
  2017-04-26 22:53 ` [PATCH 08/10] Class-ify lm_info_svr4 Simon Marchi
@ 2017-04-28 16:00   ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 16:00 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 04/26/2017 11:51 PM, Simon Marchi wrote:
> This patch makes lm_info_svr4 a "real" class.  It initializes fields,
> uses bool and replaces XCNEW/xfree with new/delete.
> 
> The memcpy in svr4_copy_library_list is replaced by a usage of the
> default copy constructor.

LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 07/10] Class-ify lm_info_target
  2017-04-26 22:53 ` [PATCH 07/10] Class-ify lm_info_target Simon Marchi
@ 2017-04-28 16:00   ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 16:00 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 04/26/2017 11:51 PM, Simon Marchi wrote:
> This patch makes lm_info_target a "real" class.  It adds a destructor,
> uses std::string, initializes the fields and replaces XCNEW/xfree with
> new/delete.
> 
> gdb/ChangeLog:
> 
> 	* solib-target.c (struct lm_info_target): Add destructor,
> 	initialize fields.
> 	<name>: Change type to std::string.
> 	(library_list_start_library): Allocate lm_info_target with new.
> 	(solib_target_free_library_list): Free lm_info_target with
> 	delete.
> 	(solib_target_current_sos): Adapt to std::string.
> 	(solib_target_free_so): Free lm_info_target with delete.

LGTM.  There was a report a while ago about SO_NAME_MAX_PATH_SIZE
being shorter than some paths Windows reports.  Moving in the direction
of std::string will end up fixing that as "fallout".

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 09/10] Class-ify lm_info_darwin
  2017-04-26 22:54 ` [PATCH 09/10] Class-ify lm_info_darwin Simon Marchi
@ 2017-04-28 16:00   ` Pedro Alves
  2017-04-28 21:13     ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 16:00 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 04/26/2017 11:51 PM, Simon Marchi wrote:
> This patch makes lm_info_darwin a "real" class.  It initializes the
> field and replaces XCNEW/xfree with new/delete.
> 
> I believe the cleanup in darwin_current_sos can be removed, I don't see
> anything that can throw after the allocation and the call to
> discard_cleanups.

Agreed.

LGTM but ...

> --- a/gdb/solib-darwin.c
> +++ b/gdb/solib-darwin.c
> @@ -41,6 +41,8 @@
>  #include "mach-o.h"
>  #include "mach-o/external.h"
>  
> +#include <memory>

... this doesn't look necessary?  (it was not in the ChangeLog.)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] Class-ify lm_info_windows
  2017-04-26 22:53 ` [PATCH 10/10] Class-ify lm_info_windows Simon Marchi
@ 2017-04-28 16:01   ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 16:01 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 04/26/2017 11:51 PM, Simon Marchi wrote:
> This patch makes lm_info_windows a "real" class.  It initializes the field
> and replaces XCNEW/xfree with new/delete.

LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 00/10] Make lm_info class hierarchy
  2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
                   ` (9 preceding siblings ...)
  2017-04-26 22:54 ` [PATCH 09/10] Class-ify lm_info_darwin Simon Marchi
@ 2017-04-28 16:08 ` Pedro Alves
  2017-04-28 21:18   ` Simon Marchi
  10 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2017-04-28 16:08 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 04/26/2017 11:46 PM, Simon Marchi wrote:
> This patch series makes a class hierarchy out of all the lm_info
> implementations.  This is done to address the problem identified by Pedro in
> [1].

(The other big manifestation of the same problem is with all the
struct gdbarch_tdep types.)

> 
> What I did corresponds to suggestion #1 in Pedro's message.  #3 would probably
> be better, but it's not straightforward.  Some implementations keep their own
> list of lm_info, separate from the global (per pspace) so_list.  So if we
> eliminate the lm_info classes and replace them with classes that extend
> so_list, it's not clear what we should do with these private lists.

For those, we could keep their lm_info as separate objects.  In essence
remove the lm_info pointer from so_list, and move it to the ports
that need it.  I.e., while most ports would do this:

struct bar_solib : so_list
{
  // old lm_info fields here.
};

The ports that want to keep a list of lm_info objects and don't want
that list to be a list of full blown so_list objects could do either:

struct foo_solib : so_list
{
   struct lm_info *lm_info;  // heap allocated.
};

or

struct foo_solib : so_list
{
   struct lm_info lm_info;
};

> So, suggestion #1 is a small step forward, and puts us in good position if we
> want to do #3 one day.  We would then have to change the lm_info_* classes to
> extend so_list instead of lm_info (and probably rename them).

Agreed.  Thanks much for doing this.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 01/10] Standardize darwin's lm_info
  2017-04-28 15:57   ` Pedro Alves
@ 2017-04-28 19:32     ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2017-04-28 19:32 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-04-28 11:57, Pedro Alves wrote:
> On 04/26/2017 11:46 PM, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> 
>> Darwin's lm_info structure is used a little bit differently than the
>> other solib implementations.  The other implementations first allocate
>> an so_list object, then instanciate their specific lm_info structure,
>> and assign it to so_list::lm_info.
>> 
>> The Darwin implementation allocates both at the same time
>> (darwin_so_list).  This patch changes it to be like the others, so 
>> that
>> we'll be able to do some generalizations later.
> 
> Funny, so this port was already "inheriting" from so_list.

Indeed.  But I thought it would be better as a first step to make all of 
them work in a similar way.  If we proceed with inheriting from so_list, 
we'll do them all at once

> LGTM.
> 
> Thanks,
> Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 04/10] Class-ify lm_info_dsbt
  2017-04-28 15:59   ` Pedro Alves
@ 2017-04-28 20:57     ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2017-04-28 20:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-04-28 11:59, Pedro Alves wrote:
> On 04/26/2017 11:47 PM, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> 
>> This patch makes lm_info_dsbt a "real" class.  It introduces a
>> destructor, initializes the field and replaces XCNEW/xfree with
>> new/delete.
> 
> LGTM.
> 
>> @@ -930,8 +935,9 @@ dsbt_relocate_main_executable (void)
>>    dsbt_get_initial_loadmaps ();
>>    ldm = info->exec_loadmap;
>> 
>> -  xfree (info->main_executable_lm_info);
>> -  info->main_executable_lm_info = XCNEW (lm_info_dsbt);
>> +  if (info->main_executable_lm_info != NULL)
>> +    delete info->main_executable_lm_info;
> 
> Note that like with xfree, it's fine to call delete
> on a NULL pointer.  I spotted the same pattern on several
> of the following patches in the series.

Indeed.  I'll go over the series and change that.

Thanks,

Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 05/10] Fix indentation of lm_info_frv
  2017-04-28 15:59   ` Pedro Alves
@ 2017-04-28 21:02     ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2017-04-28 21:02 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-04-28 11:59, Pedro Alves wrote:
> LGTM.
> 
> On 04/26/2017 11:51 PM, Simon Marchi wrote:
>> +  int dyn_reloc_count;	/* Number of dynamic relocs.  */
>> +
>> +};
> 
> You may want to remove that spurious newline while at it.

Bah, it's removed in the following patch.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 09/10] Class-ify lm_info_darwin
  2017-04-28 16:00   ` Pedro Alves
@ 2017-04-28 21:13     ` Simon Marchi
  2017-05-02 14:42       ` Pedro Alves
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2017-04-28 21:13 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-04-28 12:00, Pedro Alves wrote:
> On 04/26/2017 11:51 PM, Simon Marchi wrote:
>> This patch makes lm_info_darwin a "real" class.  It initializes the
>> field and replaces XCNEW/xfree with new/delete.
>> 
>> I believe the cleanup in darwin_current_sos can be removed, I don't 
>> see
>> anything that can throw after the allocation and the call to
>> discard_cleanups.
> 
> Agreed.
> 
> LGTM but ...
> 
>> --- a/gdb/solib-darwin.c
>> +++ b/gdb/solib-darwin.c
>> @@ -41,6 +41,8 @@
>>  #include "mach-o.h"
>>  #include "mach-o/external.h"
>> 
>> +#include <memory>
> 
> ... this doesn't look necessary?  (it was not in the ChangeLog.)

Ah, the comment in the commit log and this include is a leftover.  While 
doing the preparatory patch for darwin (01/10), I had put an xfree 
cleanup for the lm_info_darwin structure, but I later removed it.

Should we expect that the "new" operator can throw if memory allocation 
fails?  In that case we can't get rid of the cleanup for newobj.

Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 00/10] Make lm_info class hierarchy
  2017-04-28 16:08 ` [PATCH 00/10] Make lm_info class hierarchy Pedro Alves
@ 2017-04-28 21:18   ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2017-04-28 21:18 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-04-28 12:08, Pedro Alves wrote:
> On 04/26/2017 11:46 PM, Simon Marchi wrote:
>> This patch series makes a class hierarchy out of all the lm_info
>> implementations.  This is done to address the problem identified by 
>> Pedro in
>> [1].
> 
> (The other big manifestation of the same problem is with all the
> struct gdbarch_tdep types.)
> 
>> 
>> What I did corresponds to suggestion #1 in Pedro's message.  #3 would 
>> probably
>> be better, but it's not straightforward.  Some implementations keep 
>> their own
>> list of lm_info, separate from the global (per pspace) so_list.  So if 
>> we
>> eliminate the lm_info classes and replace them with classes that 
>> extend
>> so_list, it's not clear what we should do with these private lists.
> 
> For those, we could keep their lm_info as separate objects.  In essence
> remove the lm_info pointer from so_list, and move it to the ports
> that need it.  I.e., while most ports would do this:
> 
> struct bar_solib : so_list
> {
>   // old lm_info fields here.
> };
> 
> The ports that want to keep a list of lm_info objects and don't want
> that list to be a list of full blown so_list objects could do either:
> 
> struct foo_solib : so_list
> {
>    struct lm_info *lm_info;  // heap allocated.
> };
> 
> or
> 
> struct foo_solib : so_list
> {
>    struct lm_info lm_info;
> };

Thanks for the suggestion.

>> So, suggestion #1 is a small step forward, and puts us in good 
>> position if we
>> want to do #3 one day.  We would then have to change the lm_info_* 
>> classes to
>> extend so_list instead of lm_info (and probably rename them).
> 
> Agreed.  Thanks much for doing this.
> 
> Thanks,
> Pedro Alves

I pushed the series with the occasional NULL checks removed.

Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 09/10] Class-ify lm_info_darwin
  2017-04-28 21:13     ` Simon Marchi
@ 2017-05-02 14:42       ` Pedro Alves
  0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2017-05-02 14:42 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Simon Marchi, gdb-patches

On 04/28/2017 10:13 PM, Simon Marchi wrote:
> On 2017-04-28 12:00, Pedro Alves wrote:
>> On 04/26/2017 11:51 PM, Simon Marchi wrote:
>>> This patch makes lm_info_darwin a "real" class.  It initializes the
>>> field and replaces XCNEW/xfree with new/delete.
>>>
>>> I believe the cleanup in darwin_current_sos can be removed, I don't see
>>> anything that can throw after the allocation and the call to
>>> discard_cleanups.
>>
>> Agreed.
>>
>> LGTM but ...
>>
>>> --- a/gdb/solib-darwin.c
>>> +++ b/gdb/solib-darwin.c
>>> @@ -41,6 +41,8 @@
>>>  #include "mach-o.h"
>>>  #include "mach-o/external.h"
>>>
>>> +#include <memory>
>>
>> ... this doesn't look necessary?  (it was not in the ChangeLog.)
> 
> Ah, the comment in the commit log and this include is a leftover.  While
> doing the preparatory patch for darwin (01/10), I had put an xfree
> cleanup for the lm_info_darwin structure, but I later removed it.
> 
> Should we expect that the "new" operator can throw if memory allocation
> fails?  In that case we can't get rid of the cleanup for newobj.

Yeah, pedantically new can throw here.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-05-02 14:42 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-26 22:47 [PATCH 00/10] Make lm_info class hierarchy Simon Marchi
2017-04-26 22:47 ` [PATCH 02/10] Make various lm_info implementations inherit from a base class Simon Marchi
2017-04-28 15:57   ` Pedro Alves
2017-04-26 22:47 ` [PATCH 03/10] Class-ify lm_info_aix Simon Marchi
2017-04-28 15:58   ` Pedro Alves
2017-04-26 22:47 ` [PATCH 04/10] Class-ify lm_info_dsbt Simon Marchi
2017-04-28 15:59   ` Pedro Alves
2017-04-28 20:57     ` Simon Marchi
2017-04-26 22:47 ` [PATCH 01/10] Standardize darwin's lm_info Simon Marchi
2017-04-28 15:57   ` Pedro Alves
2017-04-28 19:32     ` Simon Marchi
2017-04-26 22:52 ` [PATCH 05/10] Fix indentation of lm_info_frv Simon Marchi
2017-04-28 15:59   ` Pedro Alves
2017-04-28 21:02     ` Simon Marchi
2017-04-26 22:52 ` [PATCH 06/10] Class-ify lm_info_frv Simon Marchi
2017-04-28 15:59   ` Pedro Alves
2017-04-26 22:53 ` [PATCH 08/10] Class-ify lm_info_svr4 Simon Marchi
2017-04-28 16:00   ` Pedro Alves
2017-04-26 22:53 ` [PATCH 07/10] Class-ify lm_info_target Simon Marchi
2017-04-28 16:00   ` Pedro Alves
2017-04-26 22:53 ` [PATCH 10/10] Class-ify lm_info_windows Simon Marchi
2017-04-28 16:01   ` Pedro Alves
2017-04-26 22:54 ` [PATCH 09/10] Class-ify lm_info_darwin Simon Marchi
2017-04-28 16:00   ` Pedro Alves
2017-04-28 21:13     ` Simon Marchi
2017-05-02 14:42       ` Pedro Alves
2017-04-28 16:08 ` [PATCH 00/10] Make lm_info class hierarchy Pedro Alves
2017-04-28 21:18   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).