Re: [RFC/RFA] Per-objfile data mechanism

[Daniel Jacobowitz <drow at mvista dot com>]

On Fri, Aug 22, 2003 at 12:41:46AM +0200, Mark Kettenis wrote:
>    From: David Carlton <carlton@kealia.com>
>    Date: Tue, 12 Aug 2003 13:51:38 -0700
> 
>    > I just converted gdbarch to an obstack and encountered two occasions
>    > where xmrealloc would have made my life a little easier.  Instead of
>    > proposing the use of mmalloc (and hence xmrealloc) though, I modified
>    > the algorithms / structures a little and avoided the problem.
> 
>    > Is it possible that the same situtation is being encountered here?  A
>    > growable hash table can be implemented without needing to reclaim
>    > memory - something more along the lines of a btree?
> 
>    Sure, I could replace the data structure in question by a different
>    one which is more obstack-friendly; it would be work, and the
>    resulting code would initially be less reliable, but it could be done.
>    But I guess I don't understand why obstacks are supposed to be so
>    wonderful.  They're useful if you're allocating zillions of small
>    objects that should all disappear at the same time, but I don't see
>    the value of trying to fit all of our data structures into them.
> 
> OK, this discussion is moving away from the origional question.  I
> concluded that we want the per-objfile data, and that, we might want a
> destructor mechanism.  I checked in my origional patch, since the
> destructor mechanism can easily be added later.
> 
> It is my understanding that this patch fixes some real bugs, and that
> we should consider adding this to 6.0.  Having it on mainline is a
> first step.

Wandering through my todo list...

Mark, how do you feel about moving this to the branch now?  It looks
good on mainline.

> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* objfiles.h (struct objfile): Add memebers `data' and `num_data'.
> 	(register_objfile_data, set_objfile_data, objfile_data): New
> 	prototypes.
> 	* objfiles.c (objfile_alloc_data, objfile_free_data): New
> 	prototypes.
> 	(allocate_objfile): Call objfile_alloc_data.
> 	(free_objfile): Call objfile_free_data.
> 	(struct objfile_data): New.
> 	(struct objfile_data_registration): New.
> 	(struct objfile_data_registry): New.
> 	(objfile_data_registry): New variable.
> 	(register_objfile_data): New function.
> 	(objfile_alloc_data, objfile_free_data): New functions.
> 	(set_objfile_data, objfile_data): New functions.
> 	* dwarf2-frame.c (dwarf2_frame_data): New variable.
> 	(dwarf2_frame_find_fde, add_fde): Use new per-objfile data mechanism.
> 	(_initialize_dwarf2_frame): New function and prototype.
> 
> Index: dwarf2-frame.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 dwarf2-frame.c
> --- dwarf2-frame.c 18 Jul 2003 19:59:27 -0000 1.10
> +++ dwarf2-frame.c 21 Aug 2003 22:33:51 -0000
> @@ -780,6 +780,8 @@ struct comp_unit
>    bfd_vma dbase;
>  };
>  
> +const struct objfile_data *dwarf2_frame_data;
> +
>  static unsigned int
>  read_1_byte (bfd *bfd, char *buf)
>  {
> @@ -1024,7 +1026,7 @@ dwarf2_frame_find_fde (CORE_ADDR *pc)
>  
>        offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>        
> -      fde = objfile->sym_private;
> +      fde = objfile_data (objfile, dwarf2_frame_data);
>        while (fde)
>  	{
>  	  if (*pc >= fde->initial_location + offset
> @@ -1044,8 +1046,8 @@ dwarf2_frame_find_fde (CORE_ADDR *pc)
>  static void
>  add_fde (struct comp_unit *unit, struct dwarf2_fde *fde)
>  {
> -  fde->next = unit->objfile->sym_private;
> -  unit->objfile->sym_private = fde;
> +  fde->next = objfile_data (unit->objfile, dwarf2_frame_data);
> +  set_objfile_data (unit->objfile, dwarf2_frame_data, fde);
>  }
>  
>  #ifdef CC_HAS_LONG_LONG
> @@ -1440,4 +1442,13 @@ dwarf2_build_frame_info (struct objfile 
>        while (frame_ptr < unit.dwarf_frame_buffer + unit.dwarf_frame_size)
>  	frame_ptr = decode_frame_entry (&unit, frame_ptr, 0);
>      }
> +}
> +
> +/* Provide a prototype to silence -Wmissing-prototypes.  */
> +void _initialize_dwarf2_frame (void);
> +
> +void
> +_initialize_dwarf2_frame (void)
> +{
> +  dwarf2_frame_data = register_objfile_data ();
>  }
> Index: objfiles.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/objfiles.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 objfiles.c
> --- objfiles.c 11 Jun 2003 23:29:47 -0000 1.33
> +++ objfiles.c 21 Aug 2003 22:33:51 -0000
> @@ -34,6 +34,7 @@
>  #include "target.h"
>  #include "bcache.h"
>  
> +#include "gdb_assert.h"
>  #include <sys/types.h>
>  #include "gdb_stat.h"
>  #include <fcntl.h>
> @@ -61,6 +62,9 @@ static void *map_to_file (int);
>  
>  static void add_to_objfile_sections (bfd *, sec_ptr, void *);
>  
> +static void objfile_alloc_data (struct objfile *objfile);
> +static void objfile_free_data (struct objfile *objfile);
> +
>  /* Externally visible variables that are owned by this module.
>     See declarations in objfile.h for more info. */
>  
> @@ -302,6 +306,8 @@ allocate_objfile (bfd *abfd, int flags)
>        terminate_minimal_symbol_table (objfile);
>      }
>  
> +  objfile_alloc_data (objfile);
> +
>    /* Update the per-objfile information that comes from the bfd, ensuring
>       that any data that is reference is saved in the per-objfile data
>       region. */
> @@ -561,6 +567,7 @@ free_objfile (struct objfile *objfile)
>  
>    if (objfile != NULL)
>      {
> +      objfile_free_data (objfile);
>        if (objfile->name != NULL)
>  	{
>  	  xmfree (objfile->md, objfile->name);
> @@ -1097,4 +1104,74 @@ is_in_import_list (char *name, struct ob
>        return 1;
>    return 0;
>  }
> +
> +
> +/* Keep a registry of per-objfile data-pointers required by other GDB
> +   modules.  */
> +
> +struct objfile_data
> +{
> +  unsigned index;
> +};
> +
> +struct objfile_data_registration
> +{
> +  struct objfile_data *data;
> +  struct objfile_data_registration *next;
> +};
> +  
> +struct objfile_data_registry
> +{
> +  struct objfile_data_registration *registrations;
> +  unsigned num_registrations;
> +};
> +
> +static struct objfile_data_registry objfile_data_registry = { NULL, 0 };
> +
> +const struct objfile_data *
> +register_objfile_data (void)
> +{
> +  struct objfile_data_registration **curr;
> +
> +  /* Append new registration.  */
> +  for (curr = &objfile_data_registry.registrations;
> +       *curr != NULL; curr = &(*curr)->next);
>  
> +  *curr = XMALLOC (struct objfile_data_registration);
> +  (*curr)->next = NULL;
> +  (*curr)->data = XMALLOC (struct objfile_data);
> +  (*curr)->data->index = objfile_data_registry.num_registrations++;
> +
> +  return (*curr)->data;
> +}
> +
> +static void
> +objfile_alloc_data (struct objfile *objfile)
> +{
> +  gdb_assert (objfile->data == NULL);
> +  objfile->num_data = objfile_data_registry.num_registrations;
> +  objfile->data = XCALLOC (objfile->num_data, void *);
> +}
> +
> +static void
> +objfile_free_data (struct objfile *objfile)
> +{
> +  gdb_assert (objfile->data != NULL);
> +  xfree (objfile->data);
> +  objfile->data = NULL;
> +}
> +
> +void
> +set_objfile_data (struct objfile *objfile, const struct objfile_data *data,
> +		  void *value)
> +{
> +  gdb_assert (data->index < objfile->num_data);
> +  objfile->data[data->index] = value;
> +}
> +
> +void *
> +objfile_data (struct objfile *objfile, const struct objfile_data *data)
> +{
> +  gdb_assert (data->index < objfile->num_data);
> +  return objfile->data[data->index];
> +}
> Index: objfiles.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/objfiles.h,v
> retrieving revision 1.22
> diff -u -p -r1.22 objfiles.h
> --- objfiles.h 24 Mar 2003 03:54:48 -0000 1.22
> +++ objfiles.h 21 Aug 2003 22:33:52 -0000
> @@ -379,6 +379,13 @@ struct objfile
>  
>      void *obj_private;
>  
> +    /* Per objfile data-pointers required by other GDB modules.  */
> +    /* FIXME: kettenis/20030711: This mechanism could replace
> +       sym_stab_info, sym_private and obj_private entirely.  */
> +
> +    void **data;
> +    unsigned num_data;
> +
>      /* Set of relocation offsets to apply to each section.
>         Currently on the psymbol_obstack (which makes no sense, but I'm
>         not sure it's harming anything).
> @@ -564,6 +571,16 @@ extern struct obj_section *find_pc_sect_
>  extern int in_plt_section (CORE_ADDR, char *);
>  
>  extern int is_in_import_list (char *, struct objfile *);
> +
> +/* Keep a registry of per-objfile data-pointers required by other GDB
> +   modules.  */
> +
> +extern const struct objfile_data *register_objfile_data (void);
> +extern void set_objfile_data (struct objfile *objfile,
> +			      const struct objfile_data *data, void *value);
> +extern void *objfile_data (struct objfile *objfile,
> +			   const struct objfile_data *data);
> +
>  
>  /* Traverse all object files.  ALL_OBJFILES_SAFE works even if you delete
>     the objfile during the traversal.  */
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer