[PATCH 0/9] Use the type-safe registry in more cases

[PATCH 0/9] Use the type-safe registry in more cases

[Tom Tromey]

This series changes more code to use the type-safe registry.

Regression tested by the buildbot.  I also built using a simulator, to
make sure that the remote-sim.c change would at least compile.

Tom

[PATCH 8/9] Change solib-spu.c to use type-safe registry

[Tom Tromey]

This changes solib-spu.c to use the type-safe registry.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* solib-spu.c (ocl_program_data_key): Change type.
	(append_ocl_sos, ocl_enable_break, _initialize_spu_solib):
	Update.
---
 gdb/ChangeLog   |  6 ++++++
 gdb/solib-spu.c | 12 +++++-------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index c5dc8639f1e..448e1a64f4c 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -94,7 +94,8 @@ spu_skip_standalone_loader (void)
     }
 }
 
-static const struct objfile_data *ocl_program_data_key;
+static objfile_key<CORE_ADDR, gdb::noop_deleter<CORE_ADDR>>
+  ocl_program_data_key;
 
 /* Appends OpenCL programs to the list of `struct so_list' objects.  */
 static void
@@ -104,8 +105,7 @@ append_ocl_sos (struct so_list **link_ptr)
 
   for (objfile *objfile : current_program_space->objfiles ())
     {
-      ocl_program_addr_base
-	= (CORE_ADDR *) objfile_data (objfile, ocl_program_data_key);
+      ocl_program_addr_base = ocl_program_data_key.get (objfile);
       if (ocl_program_addr_base != NULL)
         {
 	  enum bfd_endian byte_order = bfd_big_endian (objfile->obfd)?
@@ -448,15 +448,14 @@ ocl_enable_break (struct objfile *objfile)
 
       /* Store the address of the symbol that will point to OpenCL program
          using the per-objfile private data mechanism.  */
-      if (objfile_data (objfile, ocl_program_data_key) == NULL)
+      if (ocl_program_data_key.get (objfile) == NULL)
         {
           CORE_ADDR *ocl_program_addr_base = OBSTACK_CALLOC (
 		  &objfile->objfile_obstack,
 		  objfile->sections_end - objfile->sections,
 		  CORE_ADDR);
 	  *ocl_program_addr_base = BMSYMBOL_VALUE_ADDRESS (addr_sym);
-	  set_objfile_data (objfile, ocl_program_data_key,
-			    ocl_program_addr_base);
+	  ocl_program_data_key.set (objfile, ocl_program_addr_base);
         }
     }
 }
@@ -544,6 +543,5 @@ void
 _initialize_spu_solib (void)
 {
   gdb::observers::solib_loaded.attach (spu_solib_loaded);
-  ocl_program_data_key = register_objfile_data ();
 }
 
-- 
2.20.1

[PATCH 1/9] Change remote-sim.c to use type-safe registry

[Tom Tromey]

This changes remote-sim.c to use the type-safe registry.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* remote-sim.c (struct sim_inferior_data): Add initializers and
	destructor.
	(sim_inferior_data_key): Change type.  Move lower.
	(check_for_duplicate_sim_descriptor): Update.
	(get_sim_inferior_data): Use new.  Update.
	(~sim_inferior_data_cleanup): Rename from
	sim_inferior_data_cleanup.  Simplify.
	(gdbsim_close_inferior, simulator_command)
	(sim_command_completer, _initialize_remote_sim): Update.
---
 gdb/ChangeLog    | 12 ++++++++++
 gdb/remote-sim.c | 57 +++++++++++++++---------------------------------
 2 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index a22a59ed399..9030ead562d 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -132,15 +132,15 @@ struct gdbsim_target final
 
 static struct gdbsim_target gdbsim_ops;
 
-static const struct inferior_data *sim_inferior_data_key;
-
 /* Simulator-specific, per-inferior state.  */
 struct sim_inferior_data {
+  ~sim_inferior_data ();
+
   /* Flag which indicates whether or not the program has been loaded.  */
-  int program_loaded;
+  int program_loaded = 0;
 
   /* Simulator descriptor for this inferior.  */
-  SIM_DESC gdbsim_desc;
+  SIM_DESC gdbsim_desc = nullptr;
 
   /* This is the ptid we use for this particular simulator instance.  Its
      value is somewhat arbitrary, as the simulator target don't have a
@@ -150,12 +150,14 @@ struct sim_inferior_data {
   ptid_t remote_sim_ptid;
 
   /* Signal with which to resume.  */
-  enum gdb_signal resume_siggnal;
+  enum gdb_signal resume_siggnal = GDB_SIGNAL_0;
 
   /* Flag which indicates whether resume should step or not.  */
-  int resume_step;
+  int resume_step = 0;
 };
 
+static inferior_key<sim_inferior_data> sim_inferior_data_key;
+
 /* Flag indicating the "open" status of this module.  It's set to 1
    in gdbsim_open() and 0 in gdbsim_close().  */
 static int gdbsim_is_open = 0;
@@ -186,8 +188,7 @@ check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
   struct sim_inferior_data *sim_data;
   SIM_DESC new_sim_desc = (SIM_DESC) arg;
 
-  sim_data = ((struct sim_inferior_data *)
-	      inferior_data (inf, sim_inferior_data_key));
+  sim_data = sim_inferior_data_key.get (inf);
 
   return (sim_data != NULL && sim_data->gdbsim_desc == new_sim_desc);
 }
@@ -204,8 +205,7 @@ static struct sim_inferior_data *
 get_sim_inferior_data (struct inferior *inf, int sim_instance_needed)
 {
   SIM_DESC sim_desc = NULL;
-  struct sim_inferior_data *sim_data
-    = (struct sim_inferior_data *) inferior_data (inf, sim_inferior_data_key);
+  struct sim_inferior_data *sim_data = sim_inferior_data_key.get (inf);
 
   /* Try to allocate a new sim instance, if needed.  We do this ahead of
      a potential allocation of a sim_inferior_data struct in order to
@@ -240,18 +240,14 @@ get_sim_inferior_data (struct inferior *inf, int sim_instance_needed)
 
   if (sim_data == NULL)
     {
-      sim_data = XCNEW(struct sim_inferior_data);
-      set_inferior_data (inf, sim_inferior_data_key, sim_data);
+      sim_data = sim_inferior_data_key.emplace (inf);
 
       /* Allocate a ptid for this inferior.  */
       sim_data->remote_sim_ptid = ptid_t (next_pid, 0, next_pid);
       next_pid++;
 
       /* Initialize the other instance variables.  */
-      sim_data->program_loaded = 0;
       sim_data->gdbsim_desc = sim_desc;
-      sim_data->resume_siggnal = GDB_SIGNAL_0;
-      sim_data->resume_step = 0;
     }
   else if (sim_desc)
     {
@@ -287,20 +283,10 @@ get_sim_inferior_data_by_ptid (ptid_t ptid, int sim_instance_needed)
 
 /* Free the per-inferior simulator data.  */
 
-static void
-sim_inferior_data_cleanup (struct inferior *inf, void *data)
+sim_inferior_data::~sim_inferior_data ()
 {
-  struct sim_inferior_data *sim_data = (struct sim_inferior_data *) data;
-
-  if (sim_data != NULL)
-    {
-      if (sim_data->gdbsim_desc)
-	{
-	  sim_close (sim_data->gdbsim_desc, 0);
-	  sim_data->gdbsim_desc = NULL;
-	}
-      xfree (sim_data);
-    }
+  if (gdbsim_desc)
+    sim_close (gdbsim_desc, 0);
 }
 
 static void
@@ -789,14 +775,12 @@ gdbsim_target_open (const char *args, int from_tty)
 static int
 gdbsim_close_inferior (struct inferior *inf, void *arg)
 {
-  struct sim_inferior_data *sim_data
-    = (struct sim_inferior_data *) inferior_data (inf, sim_inferior_data_key);
+  struct sim_inferior_data *sim_data = sim_inferior_data_key.get (inf);
   if (sim_data != NULL)
     {
       ptid_t ptid = sim_data->remote_sim_ptid;
 
-      sim_inferior_data_cleanup (inf, sim_data);
-      set_inferior_data (inf, sim_inferior_data_key, NULL);
+      sim_inferior_data_key.clear (inf);
 
       /* Having a ptid allocated and stored in remote_sim_ptid does
 	 not mean that a corresponding inferior was ever created.
@@ -1193,8 +1177,7 @@ simulator_command (const char *args, int from_tty)
      thus allocating memory that would not be garbage collected until
      the ultimate destruction of the associated inferior.  */
 
-  sim_data  = ((struct sim_inferior_data *)
-	       inferior_data (current_inferior (), sim_inferior_data_key));
+  sim_data  = sim_inferior_data_key.get (current_inferior ());
   if (sim_data == NULL || sim_data->gdbsim_desc == NULL)
     {
 
@@ -1225,8 +1208,7 @@ sim_command_completer (struct cmd_list_element *ignore,
 {
   struct sim_inferior_data *sim_data;
 
-  sim_data = ((struct sim_inferior_data *)
-	      inferior_data (current_inferior (), sim_inferior_data_key));
+  sim_data = sim_inferior_data_key.get (current_inferior ());
   if (sim_data == NULL || sim_data->gdbsim_desc == NULL)
     return;
 
@@ -1324,7 +1306,4 @@ _initialize_remote_sim (void)
   c = add_com ("sim", class_obscure, simulator_command,
 	       _("Send a command to the simulator."));
   set_cmd_completer (c, sim_command_completer);
-
-  sim_inferior_data_key
-    = register_inferior_data_with_cleanup (NULL, sim_inferior_data_cleanup);
 }
-- 
2.20.1

Re: [PATCH 1/9] Change remote-sim.c to use type-safe registry

[Simon Marchi]

Hi Tom,

Just one nit:

> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
> index a22a59ed399..9030ead562d 100644
> --- a/gdb/remote-sim.c
> +++ b/gdb/remote-sim.c
> @@ -132,15 +132,15 @@ struct gdbsim_target final
>  
>  static struct gdbsim_target gdbsim_ops;
>  
> -static const struct inferior_data *sim_inferior_data_key;
> -
>  /* Simulator-specific, per-inferior state.  */
>  struct sim_inferior_data {
> +  ~sim_inferior_data ();
> +
>    /* Flag which indicates whether or not the program has been loaded.  */
> -  int program_loaded;
> +  int program_loaded = 0;
>  
>    /* Simulator descriptor for this inferior.  */
> -  SIM_DESC gdbsim_desc;
> +  SIM_DESC gdbsim_desc = nullptr;
>  
>    /* This is the ptid we use for this particular simulator instance.  Its
>       value is somewhat arbitrary, as the simulator target don't have a
> @@ -150,12 +150,14 @@ struct sim_inferior_data {
>    ptid_t remote_sim_ptid;

It probably doesn't matter because it's reassigned after sim_inferior_data,
is allocated, but this field should probably be initialized too, to be consistent
with the other fields.  ptid_t doesn't initialize its fields by default (to remain
a POD), so its initial content would be some garbage.

Otherwise, LGTM.

Simon

Re: [PATCH 1/9] Change remote-sim.c to use type-safe registry

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> It probably doesn't matter because it's reassigned after sim_inferior_data,
Simon> is allocated, but this field should probably be initialized too, to be consistent
Simon> with the other fields.  ptid_t doesn't initialize its fields by default (to remain
Simon> a POD), so its initial content would be some garbage.

Thanks for catching that.  I forgot about this aspect of ptid_t.  I've
made the change locally.

Tom

[PATCH 3/9] Change jit.c to use type-safe registry

[Tom Tromey]

This changes jit.c to use the type-safe registry.  Only one of the
registry keys in jit.c is converted; the other is trickier and so I've
left it be for now.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* jit.c (jit_program_space_key): Change type.  Move lower.
	(get_jit_program_space_data): Update.
	(jit_program_space_data_cleanup): Remove.
	(jit_breakpoint_deleted, free_objfile_data, _initialize_jit):
	Update.
---
 gdb/ChangeLog |  8 ++++++++
 gdb/jit.c     | 33 +++++++--------------------------
 2 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 62942fc7ab0..fba2482ff13 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -50,8 +50,6 @@ static const char *const jit_break_name = "__jit_debug_register_code";
 
 static const char *const jit_descriptor_name = "__jit_debug_descriptor";
 
-static const struct program_space_data *jit_program_space_data = NULL;
-
 static void jit_inferior_init (struct gdbarch *gdbarch);
 static void jit_inferior_exit_hook (struct inferior *inf);
 
@@ -263,6 +261,8 @@ struct jit_program_space_data
   struct breakpoint *jit_breakpoint;
 };
 
+static program_space_key<jit_program_space_data> jit_program_space_key;
+
 /* Per-objfile structure recording the addresses in the program space.
    This object serves two purposes: for ordinary objfiles, it may
    cache some symbols related to the JIT interface; and for
@@ -316,29 +316,16 @@ add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
    if not already present.  */
 
 static struct jit_program_space_data *
-get_jit_program_space_data (void)
+get_jit_program_space_data ()
 {
   struct jit_program_space_data *ps_data;
 
-  ps_data
-    = ((struct jit_program_space_data *)
-       program_space_data (current_program_space, jit_program_space_data));
+  ps_data = jit_program_space_key.get (current_program_space);
   if (ps_data == NULL)
-    {
-      ps_data = XCNEW (struct jit_program_space_data);
-      set_program_space_data (current_program_space, jit_program_space_data,
-			      ps_data);
-    }
-
+    ps_data = jit_program_space_key.emplace (current_program_space);
   return ps_data;
 }
 
-static void
-jit_program_space_data_cleanup (struct program_space *ps, void *arg)
-{
-  xfree (arg);
-}
-
 /* Helper function for reading the global JIT descriptor from remote
    memory.  Returns 1 if all went well, 0 otherwise.  */
 
@@ -1008,8 +995,7 @@ jit_breakpoint_deleted (struct breakpoint *b)
     {
       struct jit_program_space_data *ps_data;
 
-      ps_data = ((struct jit_program_space_data *)
-		 program_space_data (iter->pspace, jit_program_space_data));
+      ps_data = jit_program_space_key.get (iter->pspace);
       if (ps_data != NULL && ps_data->jit_breakpoint == iter->owner)
 	{
 	  ps_data->cached_code_address = 0;
@@ -1448,9 +1434,7 @@ free_objfile_data (struct objfile *objfile, void *data)
     {
       struct jit_program_space_data *ps_data;
 
-      ps_data
-	= ((struct jit_program_space_data *)
-	   program_space_data (objfile->pspace, jit_program_space_data));
+      ps_data = jit_program_space_key.get (objfile->pspace);
       if (ps_data != NULL && ps_data->objfile == objfile)
 	{
 	  ps_data->objfile = NULL;
@@ -1496,9 +1480,6 @@ _initialize_jit (void)
 
   jit_objfile_data =
     register_objfile_data_with_cleanup (NULL, free_objfile_data);
-  jit_program_space_data =
-    register_program_space_data_with_cleanup (NULL,
-					      jit_program_space_data_cleanup);
   jit_gdbarch_data = gdbarch_data_register_pre_init (jit_gdbarch_data_init);
   if (is_dl_available ())
     {
-- 
2.20.1

Re: [PATCH 3/9] Change jit.c to use type-safe registry

[Simon Marchi]

> diff --git a/gdb/jit.c b/gdb/jit.c
> index 62942fc7ab0..fba2482ff13 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -50,8 +50,6 @@ static const char *const jit_break_name = "__jit_debug_register_code";
>  
>  static const char *const jit_descriptor_name = "__jit_debug_descriptor";
>  
> -static const struct program_space_data *jit_program_space_data = NULL;
> -
>  static void jit_inferior_init (struct gdbarch *gdbarch);
>  static void jit_inferior_exit_hook (struct inferior *inf);
>  
> @@ -263,6 +261,8 @@ struct jit_program_space_data
>    struct breakpoint *jit_breakpoint;
>  };
>  
> +static program_space_key<jit_program_space_data> jit_program_space_key;
> +
>  /* Per-objfile structure recording the addresses in the program space.
>     This object serves two purposes: for ordinary objfiles, it may
>     cache some symbols related to the JIT interface; and for
> @@ -316,29 +316,16 @@ add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
>     if not already present.  */
>  
>  static struct jit_program_space_data *
> -get_jit_program_space_data (void)
> +get_jit_program_space_data ()
>  {
>    struct jit_program_space_data *ps_data;
>  
> -  ps_data
> -    = ((struct jit_program_space_data *)
> -       program_space_data (current_program_space, jit_program_space_data));
> +  ps_data = jit_program_space_key.get (current_program_space);
>    if (ps_data == NULL)
> -    {
> -      ps_data = XCNEW (struct jit_program_space_data);
> -      set_program_space_data (current_program_space, jit_program_space_data,
> -			      ps_data);
> -    }
> -
> +    ps_data = jit_program_space_key.emplace (current_program_space);
>    return ps_data;
>  }

Do the fields of jit_program_space_data need to be initialized?  The previous code used
XCNEW, initializing them to 0.

Simon

Re: [PATCH 3/9] Change jit.c to use type-safe registry

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Do the fields of jit_program_space_data need to be initialized?
Simon> The previous code used XCNEW, initializing them to 0.

Yep.  I've fixed this.  Thank you.

Tom

[PATCH 7/9] Change solib-aix.c to use type-safe registry

[Tom Tromey]

This changes solib-aix.c to use the type-safe registry, and removes a
use of VEC in the process.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* solib-aix.c (library_list_type): New typedef.
	(lm_info_aix_p): Remove typedef.  Don't define VEC.
	(struct solib_aix_inferior_data) <library_list>: Change type.
	(solib_aix_inferior_data_handle): Change type.
	(get_solib_aix_inferior_data): Update.
	(solib_aix_free_library_list): Remove.
	(library_list_start_library): Update.
	(solib_aix_parse_libraries, solib_aix_get_library_list): Change
	return type.
	(solib_aix_get_library_list)
	(solib_aix_solib_create_inferior_hook, solib_aix_current_sos)
	(solib_aix_normal_stop_observer, _initialize_solib_aix): Update.
---
 gdb/ChangeLog   |  15 ++++++
 gdb/solib-aix.c | 139 ++++++++++++++++--------------------------------
 2 files changed, 61 insertions(+), 93 deletions(-)

diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index bf2f30d01cd..8f0a3ca9231 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -59,14 +59,13 @@ struct lm_info_aix : public lm_info_base
   ULONGEST data_size = 0;
 };
 
-typedef lm_info_aix *lm_info_aix_p;
-DEF_VEC_P(lm_info_aix_p);
+typedef gdb::optional<std::vector<lm_info_aix>> library_list_type;
 
 /* This module's per-inferior data.  */
 
 struct solib_aix_inferior_data
 {
-  /* The list of shared libraries.  NULL if not computed yet.
+  /* The list of shared libraries.
 
      Note that the first element of this list is always the main
      executable, which is not technically a shared library.  But
@@ -74,11 +73,11 @@ 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_aix_p) *library_list;
+  library_list_type library_list;
 };
 
 /* Key to our per-inferior data.  */
-static const struct inferior_data *solib_aix_inferior_data_handle;
+static inferior_key<solib_aix_inferior_data> solib_aix_inferior_data_handle;
 
 /* Return this module's data for the given inferior.
    If none is found, add a zero'ed one now.  */
@@ -88,13 +87,9 @@ get_solib_aix_inferior_data (struct inferior *inf)
 {
   struct solib_aix_inferior_data *data;
 
-  data = ((struct solib_aix_inferior_data *)
-	  inferior_data (inf, solib_aix_inferior_data_handle));
+  data = solib_aix_inferior_data_handle.get (inf);
   if (data == NULL)
-    {
-      data = XCNEW (struct solib_aix_inferior_data);
-      set_inferior_data (inf, solib_aix_inferior_data_handle, data);
-    }
+    data = solib_aix_inferior_data_handle.emplace (inf);
 
   return data;
 }
@@ -103,7 +98,7 @@ get_solib_aix_inferior_data (struct inferior *inf)
 
 /* Dummy implementation if XML support is not compiled in.  */
 
-static VEC (lm_info_aix_p) *
+static library_list_type
 solib_aix_parse_libraries (const char *library)
 {
   static int have_warned;
@@ -115,14 +110,7 @@ solib_aix_parse_libraries (const char *library)
                  "at compile time"));
     }
 
-  return NULL;
-}
-
-/* Dummy implementation if XML support is not compiled in.  */
-
-static void
-solib_aix_free_library_list (void *p)
-{
+  return {};
 }
 
 #else /* HAVE_LIBEXPAT */
@@ -137,30 +125,30 @@ library_list_start_library (struct gdb_xml_parser *parser,
 			    void *user_data,
 			    std::vector<gdb_xml_value> &attributes)
 {
-  VEC (lm_info_aix_p) **list = (VEC (lm_info_aix_p) **) user_data;
-  lm_info_aix *item = new lm_info_aix;
+  std::vector<lm_info_aix> *list = (std::vector<lm_info_aix> *) user_data;
+  lm_info_aix item;
   struct gdb_xml_value *attr;
 
   attr = xml_find_attribute (attributes, "name");
-  item->filename = xstrdup ((const char *) attr->value.get ());
+  item.filename = (const char *) attr->value.get ();
 
   attr = xml_find_attribute (attributes, "member");
   if (attr != NULL)
-    item->member_name = xstrdup ((const char *) attr->value.get ());
+    item.member_name = (const char *) attr->value.get ();
 
   attr = xml_find_attribute (attributes, "text_addr");
-  item->text_addr = * (ULONGEST *) attr->value.get ();
+  item.text_addr = * (ULONGEST *) attr->value.get ();
 
   attr = xml_find_attribute (attributes, "text_size");
-  item->text_size = * (ULONGEST *) attr->value.get ();
+  item.text_size = * (ULONGEST *) attr->value.get ();
 
   attr = xml_find_attribute (attributes, "data_addr");
-  item->data_addr = * (ULONGEST *) attr->value.get ();
+  item.data_addr = * (ULONGEST *) attr->value.get ();
 
   attr = xml_find_attribute (attributes, "data_size");
-  item->data_size = * (ULONGEST *) attr->value.get ();
+  item.data_size = * (ULONGEST *) attr->value.get ();
 
-  VEC_safe_push (lm_info_aix_p, *list, item);
+  list->push_back (std::move (item));
 }
 
 /* Handle the start of a <library-list-aix> element.  */
@@ -180,25 +168,6 @@ library_list_start_list (struct gdb_xml_parser *parser,
                    version);
 }
 
-/* Discard the constructed library list.  */
-
-static void
-solib_aix_free_library_list (void *p)
-{
-  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_aix_p, *result, ix, info); ix++)
-    delete info;
-
-  VEC_free (lm_info_aix_p, *result);
-  *result = NULL;
-}
-
 /* The allowed elements and attributes for an AIX library list
    described in XML format.  The root element is a <library-list-aix>.  */
 
@@ -237,26 +206,18 @@ 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_aix_p vector.
 
-   Return NULL if the parsing failed.  */
+   Return false if the parsing failed.  */
 
-static VEC (lm_info_aix_p) *
+static library_list_type
 solib_aix_parse_libraries (const char *library)
 {
-  VEC (lm_info_aix_p) *result = NULL;
-  auto cleanup = make_scope_exit ([&] ()
-    {
-      solib_aix_free_library_list (&result);
-    });
+  std::vector<lm_info_aix> result;
 
   if (gdb_xml_parse_quick (_("aix library list"), "library-list-aix.dtd",
-                           library_list_elements, library, &result) == 0)
-    {
-      /* Parsed successfully, keep the result.  */
-      cleanup.release ();
-      return result;
-    }
+			   library_list_elements, library, &result) == 0)
+    return library_list_type (std::move (result));
 
-  return NULL;
+  return {};
 }
 
 #endif /* HAVE_LIBEXPAT */
@@ -271,14 +232,14 @@ 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_aix_p) *
+static library_list_type &
 solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
 {
   struct solib_aix_inferior_data *data;
 
   /* If already computed, return the cached value.  */
   data = get_solib_aix_inferior_data (inf);
-  if (data->library_list != NULL)
+  if (data->library_list.has_value ())
     return data->library_list;
 
   gdb::optional<gdb::char_vector> library_document
@@ -288,7 +249,7 @@ solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
     {
       warning (_("%s (failed to read TARGET_OBJECT_LIBRARIES_AIX)"),
 	       warning_msg);
-      return NULL;
+      return data->library_list;
     }
 
   if (solib_aix_debug)
@@ -297,11 +258,8 @@ solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
 			library_document->data ());
 
   data->library_list = solib_aix_parse_libraries (library_document->data ());
-  if (data->library_list == NULL && warning_msg != NULL)
-    {
-      warning (_("%s (missing XML support?)"), warning_msg);
-      return NULL;
-    }
+  if (!data->library_list.has_value () && warning_msg != NULL)
+    warning (_("%s (missing XML support?)"), warning_msg);
 
   return data->library_list;
 }
@@ -497,28 +455,25 @@ static void
 solib_aix_solib_create_inferior_hook (int from_tty)
 {
   const char *warning_msg = "unable to relocate main executable";
-  VEC (lm_info_aix_p) *library_list;
-  lm_info_aix *exec_info;
 
   /* We need to relocate the main executable...  */
 
-  library_list = solib_aix_get_library_list (current_inferior (),
-					     warning_msg);
-  if (library_list == NULL)
+  library_list_type &library_list
+    = solib_aix_get_library_list (current_inferior (), warning_msg);
+  if (!library_list.has_value ())
     return;  /* Warning already printed.  */
 
-  if (VEC_length (lm_info_aix_p, library_list) < 1)
+  if (library_list->empty ())
     {
       warning (_("unable to relocate main executable (no info from loader)"));
       return;
     }
 
-  exec_info = VEC_index (lm_info_aix_p, library_list, 0);
-
+  lm_info_aix &exec_info = (*library_list)[0];
   if (symfile_objfile != NULL)
     {
       gdb::unique_xmalloc_ptr<struct section_offsets> offsets
-	= solib_aix_get_section_offsets (symfile_objfile, exec_info);
+	= solib_aix_get_section_offsets (symfile_objfile, &exec_info);
 
       objfile_relocate (symfile_objfile, offsets.get ());
     }
@@ -530,29 +485,29 @@ static struct so_list *
 solib_aix_current_sos (void)
 {
   struct so_list *start = NULL, *last = NULL;
-  VEC (lm_info_aix_p) *library_list;
-  lm_info_aix *info;
   int ix;
 
-  library_list = solib_aix_get_library_list (current_inferior (), NULL);
-  if (library_list == NULL)
+  library_list_type &library_list
+    = solib_aix_get_library_list (current_inferior (), NULL);
+  if (!library_list.has_value ())
     return NULL;
 
   /* 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_aix_p, library_list, ix, info); ix++)
+  for (ix = 1; ix < library_list->size (); ix++)
     {
       struct so_list *new_solib = XCNEW (struct so_list);
       std::string so_name;
 
-      if (info->member_name.empty ())
+      lm_info_aix &info = (*library_list)[ix];
+      if (info.member_name.empty ())
 	{
-	 /* INFO->FILENAME is probably not an archive, but rather
+	 /* 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 = info->filename;
+	 so_name = info.filename;
 	}
       else
 	{
@@ -560,15 +515,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 = string_printf ("%s(%s)", info->filename.c_str (),
-				  info->member_name.c_str ());
+	 so_name = string_printf ("%s(%s)", info.filename.c_str (),
+				  info.member_name.c_str ());
 	}
       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 = new lm_info_aix (*info);
+      new_solib->lm_info = new lm_info_aix (info);
 
       /* Add it to the list.  */
       if (!start)
@@ -758,7 +713,7 @@ solib_aix_normal_stop_observer (struct bpstats *unused_1, int unused_2)
   /* The inferior execution has been resumed, and it just stopped
      again.  This means that the list of shared libraries may have
      evolved.  Reset our cached value.  */
-  solib_aix_free_library_list (&data->library_list);
+  data->library_list.reset ();
 }
 
 /* Implements the "show debug aix-solib" command.  */
@@ -789,8 +744,6 @@ _initialize_solib_aix (void)
     = solib_aix_in_dynsym_resolve_code;
   solib_aix_so_ops.bfd_open = solib_aix_bfd_open;
 
-  solib_aix_inferior_data_handle = register_inferior_data ();
-
   gdb::observers::normal_stop.attach (solib_aix_normal_stop_observer);
 
   /* Debug this file's internals.  */
-- 
2.20.1

Re: [PATCH 7/9] Change solib-aix.c to use type-safe registry

[Simon Marchi]

On 2019-07-10 11:39 a.m., Tom Tromey wrote:
> This changes solib-aix.c to use the type-safe registry, and removes a
> use of VEC in the process.
> 
> gdb/ChangeLog
> 2019-07-10  Tom Tromey  <tromey@adacore.com>
> 
> 	* solib-aix.c (library_list_type): New typedef.
> 	(lm_info_aix_p): Remove typedef.  Don't define VEC.
> 	(struct solib_aix_inferior_data) <library_list>: Change type.
> 	(solib_aix_inferior_data_handle): Change type.
> 	(get_solib_aix_inferior_data): Update.
> 	(solib_aix_free_library_list): Remove.
> 	(library_list_start_library): Update.
> 	(solib_aix_parse_libraries, solib_aix_get_library_list): Change
> 	return type.
> 	(solib_aix_get_library_list)
> 	(solib_aix_solib_create_inferior_hook, solib_aix_current_sos)
> 	(solib_aix_normal_stop_observer, _initialize_solib_aix): Update.
> ---
>  gdb/ChangeLog   |  15 ++++++
>  gdb/solib-aix.c | 139 ++++++++++++++++--------------------------------
>  2 files changed, 61 insertions(+), 93 deletions(-)
> 
> diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
> index bf2f30d01cd..8f0a3ca9231 100644
> --- a/gdb/solib-aix.c
> +++ b/gdb/solib-aix.c
> @@ -59,14 +59,13 @@ struct lm_info_aix : public lm_info_base
>    ULONGEST data_size = 0;
>  };
>  
> -typedef lm_info_aix *lm_info_aix_p;
> -DEF_VEC_P(lm_info_aix_p);
> +typedef gdb::optional<std::vector<lm_info_aix>> library_list_type;

Hmm, I find that hiding the gdb::optional behind a typedef can be a bit
confusing.  It's easy to forget that it's an optional if it's not "in your
face".  I think it would help to be explicit and spell out gdb::optional
where needed.  If you want to keep the typedef, I would suggest defining it
as:

  typedef std::vector<lm_info_aix> library_list_type;

and using gdb::optional<library_list_type> in the code.  Or even
gdb::optional<std::vector<lm_info_aix>> is not too long.

> @@ -137,30 +125,30 @@ library_list_start_library (struct gdb_xml_parser *parser,
>  			    void *user_data,
>  			    std::vector<gdb_xml_value> &attributes)
>  {
> -  VEC (lm_info_aix_p) **list = (VEC (lm_info_aix_p) **) user_data;
> -  lm_info_aix *item = new lm_info_aix;
> +  std::vector<lm_info_aix> *list = (std::vector<lm_info_aix> *) user_data;
> +  lm_info_aix item;
>    struct gdb_xml_value *attr;
>  
>    attr = xml_find_attribute (attributes, "name");
> -  item->filename = xstrdup ((const char *) attr->value.get ());
> +  item.filename = (const char *) attr->value.get ();
>  
>    attr = xml_find_attribute (attributes, "member");
>    if (attr != NULL)
> -    item->member_name = xstrdup ((const char *) attr->value.get ());
> +    item.member_name = (const char *) attr->value.get ();

It seems like that fixes a mem leak, we were xstrdup-ing into an std::string.

> @@ -237,26 +206,18 @@ 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_aix_p vector.

The part about "lm_info_aix_p" should be updated.

>  
> -   Return NULL if the parsing failed.  */
> +   Return false if the parsing failed.  */

That comment change (Return false) doesn't look in sync with the code (the
function doesn't return a bool).

>  
> -static VEC (lm_info_aix_p) *
> +static library_list_type
>  solib_aix_parse_libraries (const char *library)
>  {
> -  VEC (lm_info_aix_p) *result = NULL;
> -  auto cleanup = make_scope_exit ([&] ()
> -    {
> -      solib_aix_free_library_list (&result);
> -    });
> +  std::vector<lm_info_aix> result;
>  
>    if (gdb_xml_parse_quick (_("aix library list"), "library-list-aix.dtd",
> -                           library_list_elements, library, &result) == 0)
> -    {
> -      /* Parsed successfully, keep the result.  */
> -      cleanup.release ();
> -      return result;
> -    }
> +			   library_list_elements, library, &result) == 0)
> +    return library_list_type (std::move (result));

I think you could do "return result;" directly and it will do The Right Thing.

>  
> -  return NULL;
> +  return {};
>  }
>  
>  #endif /* HAVE_LIBEXPAT */
> @@ -271,14 +232,14 @@ 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_aix_p) *
> +static library_list_type &
>  solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
>  {

The comment above says this function can return NULL, which can't happen if
returning a reference.

> @@ -530,29 +485,29 @@ static struct so_list *
>  solib_aix_current_sos (void)
>  {
>    struct so_list *start = NULL, *last = NULL;
> -  VEC (lm_info_aix_p) *library_list;
> -  lm_info_aix *info;
>    int ix;
>  
> -  library_list = solib_aix_get_library_list (current_inferior (), NULL);
> -  if (library_list == NULL)
> +  library_list_type &library_list
> +    = solib_aix_get_library_list (current_inferior (), NULL);
> +  if (!library_list.has_value ())
>      return NULL;
>  
>    /* 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_aix_p, library_list, ix, info); ix++)
> +  for (ix = 1; ix < library_list->size (); ix++)

This could be:

  for (lm_info_aix &info : *library_list)

Simon

Re: [PATCH 7/9] Change solib-aix.c to use type-safe registry

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +typedef gdb::optional<std::vector<lm_info_aix>> library_list_type;

Simon> Hmm, I find that hiding the gdb::optional behind a typedef can be a bit
Simon> confusing.  It's easy to forget that it's an optional if it's not "in your
Simon> face".  I think it would help to be explicit and spell out gdb::optional
Simon> where needed.

Makes sense, I've removed the typedef.

I made the other changes you mentioned, except...

>> -  for (ix = 1; VEC_iterate (lm_info_aix_p, library_list, ix, info); ix++)
>> +  for (ix = 1; ix < library_list->size (); ix++)

Simon> This could be:

Simon>   for (lm_info_aix &info : *library_list)

This isn't quite the same, because the current loop starts at index 1,
not 0.  So, I didn't make this change.

Tom

Re: [PATCH 7/9] Change solib-aix.c to use type-safe registry

[Simon Marchi]

On 2019-07-10 4:33 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
>>> +typedef gdb::optional<std::vector<lm_info_aix>> library_list_type;
> 
> Simon> Hmm, I find that hiding the gdb::optional behind a typedef can be a bit
> Simon> confusing.  It's easy to forget that it's an optional if it's not "in your
> Simon> face".  I think it would help to be explicit and spell out gdb::optional
> Simon> where needed.
> 
> Makes sense, I've removed the typedef.
> 
> I made the other changes you mentioned, except...
> 
>>> -  for (ix = 1; VEC_iterate (lm_info_aix_p, library_list, ix, info); ix++)
>>> +  for (ix = 1; ix < library_list->size (); ix++)
> 
> Simon> This could be:
> 
> Simon>   for (lm_info_aix &info : *library_list)
> 
> This isn't quite the same, because the current loop starts at index 1,
> not 0.  So, I didn't make this change.

Ah good point, you are right, sorry about that!

[PATCH 2/9] Change solib-darwin.c to use type-safe registry

[Tom Tromey]

This changes solib-darwin.c to use the type-safe registry.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* solib-darwin.c (struct darwin_info): Add initializers.
	(solib_darwin_pspace_data): Change type.
	(darwin_pspace_data_cleanup): Remove.
	(get_darwin_info, _initialize_darwin_solib): Update.
---
 gdb/ChangeLog      |  7 +++++++
 gdb/solib-darwin.c | 24 +++++-------------------
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 65a5b9afca0..443ebb64a47 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -73,20 +73,14 @@ struct gdb_dyld_all_image_infos
 struct darwin_info
 {
   /* Address of structure dyld_all_image_infos in inferior.  */
-  CORE_ADDR all_image_addr;
+  CORE_ADDR all_image_addr = 0;
 
   /* Gdb copy of dyld_all_info_infos.  */
-  struct gdb_dyld_all_image_infos all_image;
+  struct gdb_dyld_all_image_infos all_image {};
 };
 
 /* Per-program-space data key.  */
-static const struct program_space_data *solib_darwin_pspace_data;
-
-static void
-darwin_pspace_data_cleanup (struct program_space *pspace, void *arg)
-{
-  xfree (arg);
-}
+static program_space_key<darwin_info> solib_darwin_pspace_data;
 
 /* Get the current darwin data.  If none is found yet, add it now.  This
    function always returns a valid object.  */
@@ -96,15 +90,11 @@ get_darwin_info (void)
 {
   struct darwin_info *info;
 
-  info = (struct darwin_info *) program_space_data (current_program_space,
-						    solib_darwin_pspace_data);
+  info = solib_darwin_pspace_data.get (current_program_space);
   if (info != NULL)
     return info;
 
-  info = XCNEW (struct darwin_info);
-  set_program_space_data (current_program_space,
-			  solib_darwin_pspace_data, info);
-  return info;
+  return solib_darwin_pspace_data.emplace (current_program_space);
 }
 
 /* Return non-zero if the version in dyld_all_image is known.  */
@@ -691,10 +681,6 @@ struct target_so_ops darwin_so_ops;
 void
 _initialize_darwin_solib (void)
 {
-  solib_darwin_pspace_data
-    = register_program_space_data_with_cleanup (NULL,
-						darwin_pspace_data_cleanup);
-
   darwin_so_ops.relocate_section_addresses = darwin_relocate_section_addresses;
   darwin_so_ops.free_so = darwin_free_so;
   darwin_so_ops.clear_solib = darwin_clear_solib;
-- 
2.20.1

[PATCH 6/9] Change solib-dsbt.c to use type-safe registry

[Tom Tromey]

This changes solib-dsbt.c to use the type-safe registry.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* solib-dsbt.c (struct dsbt_info): Add initializers.
	(solib_dsbt_pspace_data): Change type.
	(dsbt_pspace_data_cleanup): Remove.
	(get_dsbt_info, _initialize_dsbt_solib): Update.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/solib-dsbt.c | 40 ++++++++++++----------------------------
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 3053c189802..59b195f491e 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -142,35 +142,29 @@ 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_dsbt *main_executable_lm_info;
+  struct lm_info_dsbt *main_executable_lm_info = nullptr;
 
   /* Load maps for the main executable and the interpreter.  These are obtained
      from ptrace.  They are the starting point for getting into the program,
      and are required to find the solib list with the individual load maps for
      each module.  */
-  struct int_elf32_dsbt_loadmap *exec_loadmap;
-  struct int_elf32_dsbt_loadmap *interp_loadmap;
+  struct int_elf32_dsbt_loadmap *exec_loadmap = nullptr;
+  struct int_elf32_dsbt_loadmap *interp_loadmap = nullptr;
 
   /* Cached value for lm_base, below.  */
-  CORE_ADDR lm_base_cache;
+  CORE_ADDR lm_base_cache = 0;
 
   /* Link map address for main module.  */
-  CORE_ADDR main_lm_addr;
+  CORE_ADDR main_lm_addr = 0;
 
-  CORE_ADDR interp_text_sect_low;
-  CORE_ADDR interp_text_sect_high;
-  CORE_ADDR interp_plt_sect_low;
-  CORE_ADDR interp_plt_sect_high;
+  CORE_ADDR interp_text_sect_low = 0;
+  CORE_ADDR interp_text_sect_high = 0;
+  CORE_ADDR interp_plt_sect_low = 0;
+  CORE_ADDR interp_plt_sect_high = 0;
 };
 
 /* Per-program-space data key.  */
-static const struct program_space_data *solib_dsbt_pspace_data;
-
-static void
-dsbt_pspace_data_cleanup (struct program_space *pspace, void *arg)
-{
-  xfree (arg);
-}
+static program_space_key<dsbt_info> solib_dsbt_pspace_data;
 
 /* Get the current dsbt data.  If none is found yet, add it now.  This
    function always returns a valid object.  */
@@ -180,18 +174,11 @@ get_dsbt_info (void)
 {
   struct dsbt_info *info;
 
-  info = (struct dsbt_info *) program_space_data (current_program_space,
-						  solib_dsbt_pspace_data);
+  info = solib_dsbt_pspace_data.get (current_program_space);
   if (info != NULL)
     return info;
 
-  info = XCNEW (struct dsbt_info);
-  set_program_space_data (current_program_space, solib_dsbt_pspace_data, info);
-
-  info->lm_base_cache = 0;
-  info->main_lm_addr = 0;
-
-  return info;
+  return solib_dsbt_pspace_data.emplace (current_program_space);
 }
 
 
@@ -1043,9 +1030,6 @@ struct target_so_ops dsbt_so_ops;
 void
 _initialize_dsbt_solib (void)
 {
-  solib_dsbt_pspace_data
-    = register_program_space_data_with_cleanup (NULL, dsbt_pspace_data_cleanup);
-
   dsbt_so_ops.relocate_section_addresses = dsbt_relocate_section_addresses;
   dsbt_so_ops.free_so = dsbt_free_so;
   dsbt_so_ops.clear_solib = dsbt_clear_solib;
-- 
2.20.1

[PATCH 4/9] Change dbxread.c to use type-safe registry

[Tom Tromey]

This changes dbxread.c to use the type-safe registry.  In a couple of
spots, you'll see that dbx_objfile_data_key.emplace is called but the
result is not used; this is because those functions refer to the key
via the various DBX_* macros.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* gdb-stabs.h (struct dbx_symfile_info): Add initializers and
	destructor.
	(dbx_objfile_data_key): Change type and declare later.
	(DBX_SYMFILE_INFO): Rewrite.
	* dbxread.c (dbx_objfile_data_key): Change type.
	(dbx_symfile_init): Update.
	(~dbx_symfile_info): Rename from dbx_free_symfile_info.  Update.
	(coffstab_build_psymtabs, elfstab_build_psymtabs)
	(stabsect_build_psymtabs, _initialize_dbxread): Update.
---
 gdb/ChangeLog   | 12 ++++++++++++
 gdb/dbxread.c   | 32 +++++++++-----------------------
 gdb/gdb-stabs.h | 41 +++++++++++++++++++++--------------------
 3 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 307debd2824..00eeb317dc4 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -61,7 +61,7 @@
 
 /* Key for dbx-associated data.  */
 
-const struct objfile_data *dbx_objfile_data_key;
+objfile_key<dbx_symfile_info> dbx_objfile_data_key;
 
 /* We put a pointer to this structure in the read_symtab_private field
    of the psymtab.  */
@@ -590,11 +590,9 @@ dbx_symfile_init (struct objfile *objfile)
   char *name = bfd_get_filename (sym_bfd);
   asection *text_sect;
   unsigned char size_temp[DBX_STRINGTAB_SIZE_SIZE];
-  struct dbx_symfile_info *dbx;
 
   /* Allocate struct to keep track of the symfile.  */
-  dbx = XCNEW (struct dbx_symfile_info);
-  set_objfile_data (objfile, dbx_objfile_data_key, dbx);
+  dbx_objfile_data_key.emplace (objfile);
 
   DBX_TEXT_SECTION (objfile) = bfd_get_section_by_name (sym_bfd, ".text");
   DBX_DATA_SECTION (objfile) = bfd_get_section_by_name (sym_bfd, ".data");
@@ -703,15 +701,12 @@ dbx_symfile_finish (struct objfile *objfile)
   free_header_files ();
 }
 
-static void
-dbx_free_symfile_info (struct objfile *objfile, void *arg)
+dbx_symfile_info::~dbx_symfile_info ()
 {
-  struct dbx_symfile_info *dbx = (struct dbx_symfile_info *) arg;
-
-  if (dbx->header_files != NULL)
+  if (header_files != NULL)
     {
-      int i = dbx->n_header_files;
-      struct header_file *hfiles = dbx->header_files;
+      int i = n_header_files;
+      struct header_file *hfiles = header_files;
 
       while (--i >= 0)
 	{
@@ -720,8 +715,6 @@ dbx_free_symfile_info (struct objfile *objfile, void *arg)
 	}
       xfree (hfiles);
     }
-
-  xfree (dbx);
 }
 
 \f
@@ -2952,8 +2945,7 @@ coffstab_build_psymtabs (struct objfile *objfile,
   unsigned int stabsize;
 
   /* Allocate struct to keep track of stab reading.  */
-  struct dbx_symfile_info *dbx = XCNEW (struct dbx_symfile_info);
-  set_objfile_data (objfile, dbx_objfile_data_key, dbx);
+  dbx_objfile_data_key.emplace (objfile);
 
   DBX_TEXT_ADDR (objfile) = textaddr;
   DBX_TEXT_SIZE (objfile) = textsize;
@@ -3041,8 +3033,7 @@ elfstab_build_psymtabs (struct objfile *objfile, asection *stabsect,
   stabsread_new_init ();
 
   /* Allocate struct to keep track of stab reading.  */
-  struct dbx_symfile_info *dbx = XCNEW (struct dbx_symfile_info);
-  set_objfile_data (objfile, dbx_objfile_data_key, dbx);
+  dbx_objfile_data_key.emplace (objfile);
 
   /* Find the first and last text address.  dbx_symfile_read seems to
      want this.  */
@@ -3122,7 +3113,6 @@ stabsect_build_psymtabs (struct objfile *objfile, char *stab_name,
   asection *stabsect;
   asection *stabstrsect;
   asection *text_sect;
-  struct dbx_symfile_info *dbx;
 
   stabsect = bfd_get_section_by_name (sym_bfd, stab_name);
   stabstrsect = bfd_get_section_by_name (sym_bfd, stabstr_name);
@@ -3135,8 +3125,7 @@ stabsect_build_psymtabs (struct objfile *objfile, char *stab_name,
 	     "but not string section (%s)"),
 	   stab_name, stabstr_name);
 
-  dbx = XCNEW (struct dbx_symfile_info);
-  set_objfile_data (objfile, dbx_objfile_data_key, dbx);
+  dbx_objfile_data_key.emplace (objfile);
 
   text_sect = bfd_get_section_by_name (sym_bfd, text_name);
   if (!text_sect)
@@ -3201,7 +3190,4 @@ void
 _initialize_dbxread (void)
 {
   add_symtab_fns (bfd_target_aout_flavour, &aout_sym_fns);
-
-  dbx_objfile_data_key
-    = register_objfile_data_with_cleanup (NULL, dbx_free_symfile_info);
 }
diff --git a/gdb/gdb-stabs.h b/gdb/gdb-stabs.h
index 3b64d328f5c..dfdd20be06d 100644
--- a/gdb/gdb-stabs.h
+++ b/gdb/gdb-stabs.h
@@ -27,41 +27,42 @@
    each others' functions as required.  */
 
 
-/* The tag used to find the DBX info attached to an objfile.  This is
-   global because it is referenced by several modules.  */
-extern const struct objfile_data *dbx_objfile_data_key;
-
 /* Information is passed among various dbxread routines for accessing
    symbol files.  A pointer to this structure is kept in the objfile,
    using the dbx_objfile_data_key.  */
 
 struct dbx_symfile_info
   {
-    CORE_ADDR text_addr;	/* Start of text section */
-    int text_size;		/* Size of text section */
-    int symcount;		/* How many symbols are there in the file */
-    char *stringtab;		/* The actual string table */
-    int stringtab_size;		/* Its size */
-    file_ptr symtab_offset;	/* Offset in file to symbol table */
-    int symbol_size;		/* Bytes in a single symbol */
+    ~dbx_symfile_info ();
+
+    CORE_ADDR text_addr = 0;	/* Start of text section */
+    int text_size = 0;		/* Size of text section */
+    int symcount = 0;		/* How many symbols are there in the file */
+    char *stringtab = nullptr;		/* The actual string table */
+    int stringtab_size = 0;		/* Its size */
+    file_ptr symtab_offset = 0;	/* Offset in file to symbol table */
+    int symbol_size = 0;		/* Bytes in a single symbol */
 
     /* See stabsread.h for the use of the following.  */
-    struct header_file *header_files;
-    int n_header_files;
-    int n_allocated_header_files;
+    struct header_file *header_files = nullptr;
+    int n_header_files = 0;
+    int n_allocated_header_files = 0;
 
     /* Pointers to BFD sections.  These are used to speed up the building of
        minimal symbols.  */
-    asection *text_section;
-    asection *data_section;
-    asection *bss_section;
+    asection *text_section = nullptr;
+    asection *data_section = nullptr;
+    asection *bss_section = nullptr;
 
     /* Pointer to the separate ".stab" section, if there is one.  */
-    asection *stab_section;
+    asection *stab_section = nullptr;
   };
 
-#define DBX_SYMFILE_INFO(o) \
-  ((struct dbx_symfile_info *) objfile_data ((o), dbx_objfile_data_key))
+/* The tag used to find the DBX info attached to an objfile.  This is
+   global because it is referenced by several modules.  */
+extern objfile_key<dbx_symfile_info> dbx_objfile_data_key;
+
+#define DBX_SYMFILE_INFO(o)	(dbx_objfile_data_key.get (o))
 #define DBX_TEXT_ADDR(o)	(DBX_SYMFILE_INFO(o)->text_addr)
 #define DBX_TEXT_SIZE(o)	(DBX_SYMFILE_INFO(o)->text_size)
 #define DBX_SYMCOUNT(o)		(DBX_SYMFILE_INFO(o)->symcount)
-- 
2.20.1

[PATCH 9/9] Change arm-tdep.c to use type-safe registry

[Tom Tromey]

This changes arm-tdep.c to use the type-safe registry, removing a use
of VEC in the process.

gdb/ChangeLog
2019-07-10  Tom Tromey  <tromey@adacore.com>

	* arm-tdep.c (arm_exidx_entry_s): Remove typedef.  Don't define
	VEC.
	(struct arm_exidx_entry): New method operator<.
	(struct arm_exidx_data) <section_maps>: Change type.
	(arm_exidx_data_free): Remove.
	(arm_exidx_data_key): Change type.  Move lower.
	(arm_exidx_new_objfile): Update.
	(arm_compare_exidx_entries): Remove.
	(arm_find_exidx_entry, _initialize_arm_tdep)
---
 gdb/ChangeLog  | 12 ++++++++
 gdb/arm-tdep.c | 78 +++++++++++++++++---------------------------------
 2 files changed, 38 insertions(+), 52 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index fe6251924f9..c1ee39714f9 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -1989,37 +1989,23 @@ struct frame_unwind arm_prologue_unwind = {
    personality routines; the cache will contain only the frame unwinding
    instructions associated with the entry (not the descriptors).  */
 
-static const struct objfile_data *arm_exidx_data_key;
-
 struct arm_exidx_entry
 {
   bfd_vma addr;
   gdb_byte *entry;
+
+  bool operator< (const arm_exidx_entry &other) const
+  {
+    return addr < other.addr;
+  }
 };
-typedef struct arm_exidx_entry arm_exidx_entry_s;
-DEF_VEC_O(arm_exidx_entry_s);
 
 struct arm_exidx_data
 {
-  VEC(arm_exidx_entry_s) **section_maps;
+  std::vector<std::vector<arm_exidx_entry>> section_maps;
 };
 
-static void
-arm_exidx_data_free (struct objfile *objfile, void *arg)
-{
-  struct arm_exidx_data *data = (struct arm_exidx_data *) arg;
-  unsigned int i;
-
-  for (i = 0; i < objfile->obfd->section_count; i++)
-    VEC_free (arm_exidx_entry_s, data->section_maps[i]);
-}
-
-static inline int
-arm_compare_exidx_entries (const struct arm_exidx_entry *lhs,
-			   const struct arm_exidx_entry *rhs)
-{
-  return lhs->addr < rhs->addr;
-}
+static const struct objfile_key<arm_exidx_data> arm_exidx_data_key;
 
 static struct obj_section *
 arm_obj_section_from_vma (struct objfile *objfile, bfd_vma vma)
@@ -2064,7 +2050,7 @@ arm_exidx_new_objfile (struct objfile *objfile)
   LONGEST i;
 
   /* If we've already touched this file, do nothing.  */
-  if (!objfile || objfile_data (objfile, arm_exidx_data_key) != NULL)
+  if (!objfile || arm_exidx_data_key.get (objfile) != NULL)
     return;
 
   /* Read contents of exception table and index.  */
@@ -2095,11 +2081,8 @@ arm_exidx_new_objfile (struct objfile *objfile)
     }
 
   /* Allocate exception table data structure.  */
-  data = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct arm_exidx_data);
-  set_objfile_data (objfile, arm_exidx_data_key, data);
-  data->section_maps = OBSTACK_CALLOC (&objfile->objfile_obstack,
-				       objfile->obfd->section_count,
-				       VEC(arm_exidx_entry_s) *);
+  data = arm_exidx_data_key.emplace (objfile);
+  data->section_maps.resize (objfile->obfd->section_count);
 
   /* Fill in exception table.  */
   for (i = 0; i < exidx_data.size () / 8; i++)
@@ -2250,9 +2233,8 @@ arm_exidx_new_objfile (struct objfile *objfile)
 	 appear in order of increasing addresses.  */
       new_exidx_entry.addr = idx;
       new_exidx_entry.entry = entry;
-      VEC_safe_push (arm_exidx_entry_s,
-		     data->section_maps[sec->the_bfd_section->index],
-		     &new_exidx_entry);
+      data->section_maps[sec->the_bfd_section->index].push_back
+	(new_exidx_entry);
     }
 }
 
@@ -2269,43 +2251,37 @@ arm_find_exidx_entry (CORE_ADDR memaddr, CORE_ADDR *start)
   if (sec != NULL)
     {
       struct arm_exidx_data *data;
-      VEC(arm_exidx_entry_s) *map;
       struct arm_exidx_entry map_key = { memaddr - obj_section_addr (sec), 0 };
-      unsigned int idx;
 
-      data = ((struct arm_exidx_data *)
-	      objfile_data (sec->objfile, arm_exidx_data_key));
+      data = arm_exidx_data_key.get (sec->objfile);
       if (data != NULL)
 	{
-	  map = data->section_maps[sec->the_bfd_section->index];
-	  if (!VEC_empty (arm_exidx_entry_s, map))
+	  std::vector<arm_exidx_entry> &map
+	    = data->section_maps[sec->the_bfd_section->index];
+	  if (!map.empty ())
 	    {
-	      struct arm_exidx_entry *map_sym;
-
-	      idx = VEC_lower_bound (arm_exidx_entry_s, map, &map_key,
-				     arm_compare_exidx_entries);
+	      auto idx = std::lower_bound (map.begin (), map.end (), map_key);
 
-	      /* VEC_lower_bound finds the earliest ordered insertion
+	      /* std::lower_bound finds the earliest ordered insertion
 		 point.  If the following symbol starts at this exact
 		 address, we use that; otherwise, the preceding
 		 exception table entry covers this address.  */
-	      if (idx < VEC_length (arm_exidx_entry_s, map))
+	      if (idx < map.end ())
 		{
-		  map_sym = VEC_index (arm_exidx_entry_s, map, idx);
-		  if (map_sym->addr == map_key.addr)
+		  if (idx->addr == map_key.addr)
 		    {
 		      if (start)
-			*start = map_sym->addr + obj_section_addr (sec);
-		      return map_sym->entry;
+			*start = idx->addr + obj_section_addr (sec);
+		      return idx->entry;
 		    }
 		}
 
-	      if (idx > 0)
+	      if (idx > map.begin ())
 		{
-		  map_sym = VEC_index (arm_exidx_entry_s, map, idx - 1);
+		  idx = idx - 1;
 		  if (start)
-		    *start = map_sym->addr + obj_section_addr (sec);
-		  return map_sym->entry;
+		    *start = idx->addr + obj_section_addr (sec);
+		  return idx->entry;
 		}
 	    }
 	}
@@ -9461,8 +9437,6 @@ _initialize_arm_tdep (void)
 
   /* Add ourselves to objfile event chain.  */
   gdb::observers::new_objfile.attach (arm_exidx_new_objfile);
-  arm_exidx_data_key
-    = register_objfile_data_with_cleanup (NULL, arm_exidx_data_free);
 
   /* Register an ELF OS ABI sniffer for ARM binaries.  */
   gdbarch_register_osabi_sniffer (bfd_arch_arm,
-- 
2.20.1

Re: [PATCH 0/9] Use the type-safe registry in more cases

[Simon Marchi]

On 2019-07-10 11:39 a.m., Tom Tromey wrote:
> This series changes more code to use the type-safe registry.
> 
> Regression tested by the buildbot.  I also built using a simulator, to
> make sure that the remote-sim.c change would at least compile.
> 
> Tom

Hi Tom,

The patches I have not replied to LGTM.  Thanks for doing this!

Simon

Re: [PATCH 0/9] Use the type-safe registry in more cases

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> The patches I have not replied to LGTM.  Thanks for doing this!

Thanks for your reviews.  They were very helpful.
I'm checking in the updated series.

Tom