Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA]: Add misc_obstack to object files.
@ 2001-05-29 10:31 Daniel Berlin
  2001-06-25 13:59 ` Elena Zannoni
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Berlin @ 2001-05-29 10:31 UTC (permalink / raw)
  To: gdb-patches

This way, we stop putting things in the psymbol_obstack that don't
belong.  

2001-05-27  Daniel Berlin  <dan@cgsoftware.com>

	* symfile.c (default_symfile_offsets): Allocate in misc_obstack.
	(reread_symbols): Handle misc_obstack too, and use it where
	psymbol_obstack didn't belong. 
	(allocate_psymtab): Ditto.

	* objfiles.c (add_to_objfile_sections):  Use misc_obstack, not psymbol_obstack.
	(build_objfile_section_table): Ditto.
	(free_objfile): Free the misc_obstack.
	(allocate_objfile): Setup misc_obstack too.

	* objfiles.h: Add misc_obstack to object file.

Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.32
diff -c -3 -p -w -B -b -r1.32 symfile.c
*** symfile.c	2001/05/10 15:33:21	1.32
--- symfile.c	2001/05/29 17:27:51
*************** default_symfile_offsets (struct objfile 
*** 500,506 ****
  
    objfile->num_sections = SECT_OFF_MAX;
    objfile->section_offsets = (struct section_offsets *)
!     obstack_alloc (&objfile->psymbol_obstack, SIZEOF_SECTION_OFFSETS);
    memset (objfile->section_offsets, 0, SIZEOF_SECTION_OFFSETS);
  
    /* Now calculate offsets for section that were specified by the
--- 500,506 ----
  
    objfile->num_sections = SECT_OFF_MAX;
    objfile->section_offsets = (struct section_offsets *)
!     obstack_alloc (&objfile->misc_obstack, SIZEOF_SECTION_OFFSETS);
    memset (objfile->section_offsets, 0, SIZEOF_SECTION_OFFSETS);
  
    /* Now calculate offsets for section that were specified by the
*************** reread_symbols (void)
*** 1679,1684 ****
--- 1679,1685 ----
  
  	      /* Free the obstacks for non-reusable objfiles */
  	      free_bcache (&objfile->psymbol_cache);
+ 	      obstack_free (&objfile->misc_obstack, 0);
  	      obstack_free (&objfile->psymbol_obstack, 0);
  	      obstack_free (&objfile->symbol_obstack, 0);
  	      obstack_free (&objfile->type_obstack, 0);
*************** reread_symbols (void)
*** 1704,1709 ****
--- 1705,1712 ----
  	         it is empty.  */
  	      obstack_specify_allocation (&objfile->psymbol_cache.cache, 0, 0,
  					  xmalloc, xfree);
+ 	      obstack_specify_allocation (&objfile->misc_obstack, 0, 0,
+ 					  xmalloc, xfree);
  	      obstack_specify_allocation (&objfile->psymbol_obstack, 0, 0,
  					  xmalloc, xfree);
  	      obstack_specify_allocation (&objfile->symbol_obstack, 0, 0,
*************** reread_symbols (void)
*** 1719,1725 ****
  	      /* We use the same section offsets as from last time.  I'm not
  	         sure whether that is always correct for shared libraries.  */
  	      objfile->section_offsets = (struct section_offsets *)
! 		obstack_alloc (&objfile->psymbol_obstack, SIZEOF_SECTION_OFFSETS);
  	      memcpy (objfile->section_offsets, offsets, SIZEOF_SECTION_OFFSETS);
  	      objfile->num_sections = num_offsets;
  
--- 1722,1728 ----
  	      /* We use the same section offsets as from last time.  I'm not
  	         sure whether that is always correct for shared libraries.  */
  	      objfile->section_offsets = (struct section_offsets *)
! 		obstack_alloc (&objfile->misc_obstack, SIZEOF_SECTION_OFFSETS);
  	      memcpy (objfile->section_offsets, offsets, SIZEOF_SECTION_OFFSETS);
  	      objfile->num_sections = num_offsets;
  
*************** allocate_psymtab (char *filename, struct
*** 1981,1993 ****
        objfile->free_psymtabs = psymtab->next;
      }
    else
!     psymtab = (struct partial_symtab *)
!       obstack_alloc (&objfile->psymbol_obstack,
! 		     sizeof (struct partial_symtab));
  
    memset (psymtab, 0, sizeof (struct partial_symtab));
!   psymtab->filename = obsavestring (filename, strlen (filename),
! 				    &objfile->psymbol_obstack);
    psymtab->symtab = NULL;
  
    /* Prepend it to the psymtab list for the objfile it belongs to.
--- 1984,1993 ----
        objfile->free_psymtabs = psymtab->next;
      }
    else
!     psymtab = (struct partial_symtab *) obstack_alloc (&objfile->misc_obstack, sizeof (struct partial_symtab));
  
    memset (psymtab, 0, sizeof (struct partial_symtab));
!   psymtab->filename = obsavestring (filename, strlen(filename), &objfile->misc_obstack);
    psymtab->symtab = NULL;
  
    /* Prepend it to the psymtab list for the objfile it belongs to.
Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.15
diff -c -3 -p -w -B -b -r1.15 objfiles.c
*** objfiles.c	2001/03/06 08:21:11	1.15
--- objfiles.c	2001/05/29 17:27:57
*************** add_to_objfile_sections (bfd *abfd, sec_
*** 96,102 ****
    section.ovly_mapped = 0;
    section.addr = bfd_section_vma (abfd, asect);
    section.endaddr = section.addr + bfd_section_size (abfd, asect);
!   obstack_grow (&objfile->psymbol_obstack, (char *) &section, sizeof (section));
    objfile->sections_end = (struct obj_section *) (((unsigned long) objfile->sections_end) + 1);
  }
  
--- 96,102 ----
    section.ovly_mapped = 0;
    section.addr = bfd_section_vma (abfd, asect);
    section.endaddr = section.addr + bfd_section_size (abfd, asect);
!   obstack_grow (&objfile->misc_obstack, (char *) &section, sizeof (section));
    objfile->sections_end = (struct obj_section *) (((unsigned long) objfile->sections_end) + 1);
  }
  
*************** build_objfile_section_table (struct objf
*** 121,134 ****
  {
    /* objfile->sections can be already set when reading a mapped symbol
       file.  I believe that we do need to rebuild the section table in
!      this case (we rebuild other things derived from the bfd), but we
!      can't free the old one (it's in the psymbol_obstack).  So we just
!      waste some memory.  */
  
    objfile->sections_end = 0;
    bfd_map_over_sections (objfile->obfd, add_to_objfile_sections, (char *) objfile);
    objfile->sections = (struct obj_section *)
!     obstack_finish (&objfile->psymbol_obstack);
    objfile->sections_end = objfile->sections + (unsigned long) objfile->sections_end;
    return (0);
  }
--- 121,137 ----
  {
    /* objfile->sections can be already set when reading a mapped symbol
       file.  I believe that we do need to rebuild the section table in
!      this case (we rebuild other things derived from the bfd).
!      DJB - 05-27-2001 
!      It's in the misc_obstack now, feel free to do what you need to.
!      All the stuff in objfile that was on the psymbol obstack, but didnt' belong, is in the misc obstack, which 
!      I think is all the stuff you want to blow away anyway.
!      */
  
    objfile->sections_end = 0;
    bfd_map_over_sections (objfile->obfd, add_to_objfile_sections, (char *) objfile);
    objfile->sections = (struct obj_section *)
!     obstack_finish (&objfile->misc_obstack);
    objfile->sections_end = objfile->sections + (unsigned long) objfile->sections_end;
    return (0);
  }
*************** allocate_objfile (bfd *abfd, int flags)
*** 188,193 ****
--- 191,198 ----
  	      /* Update pointers to functions to *our* copies */
  	      obstack_chunkfun (&objfile->psymbol_cache.cache, xmmalloc);
  	      obstack_freefun (&objfile->psymbol_cache.cache, mfree);
+ 	      obstack_chunkfun (&objfile->misc_obstack, xmmalloc);
+ 	      obstack_freefun (&objfile->misc_obstack, mfree);
  	      obstack_chunkfun (&objfile->psymbol_obstack, xmmalloc);
  	      obstack_freefun (&objfile->psymbol_obstack, mfree);
  	      obstack_chunkfun (&objfile->symbol_obstack, xmmalloc);
*************** allocate_objfile (bfd *abfd, int flags)
*** 218,223 ****
--- 223,231 ----
  	      obstack_specify_allocation_with_arg (&objfile->psymbol_cache.cache,
  						   0, 0, xmmalloc, mfree,
  						   objfile->md);
+ 	      obstack_specify_allocation_with_arg (&objfile->misc_obstack,
+ 						   0, 0, xmmalloc, mfree,
+ 						   objfile->md);
  	      obstack_specify_allocation_with_arg (&objfile->psymbol_obstack,
  						   0, 0, xmmalloc, mfree,
  						   objfile->md);
*************** allocate_objfile (bfd *abfd, int flags)
*** 264,269 ****
--- 272,279 ----
        objfile->md = NULL;
        obstack_specify_allocation (&objfile->psymbol_cache.cache, 0, 0,
  				  xmalloc, xfree);
+       obstack_specify_allocation (&objfile->misc_obstack, 0, 0, xmalloc,
+ 				  xfree);
        obstack_specify_allocation (&objfile->psymbol_obstack, 0, 0, xmalloc,
  				  xfree);
        obstack_specify_allocation (&objfile->symbol_obstack, 0, 0, xmalloc,
*************** free_objfile (struct objfile *objfile)
*** 475,480 ****
--- 485,491 ----
  	mfree (objfile->md, objfile->static_psymbols.list);
        /* Free the obstacks for non-reusable objfiles */
        free_bcache (&objfile->psymbol_cache);
+       obstack_free (&objfile->misc_obstack, 0);
        obstack_free (&objfile->psymbol_obstack, 0);
        obstack_free (&objfile->symbol_obstack, 0);
        obstack_free (&objfile->type_obstack, 0);
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.8
diff -c -3 -p -w -B -b -r1.8 objfiles.h
*** objfiles.h	2001/03/06 08:21:11	1.8
--- objfiles.h	2001/05/29 17:28:00
*************** struct objfile
*** 269,274 ****
--- 269,275 ----
      /* Obstacks to hold objects that should be freed when we load a new symbol
         table from this object file. */
  
+     struct obstack misc_obstack;	/* Misc stuff */
      struct obstack psymbol_obstack;	/* Partial symbols */
      struct obstack symbol_obstack;	/* Full symbols */
      struct obstack type_obstack;	/* Types */

-- 
"I used to be a narrator for bad mimes.
"-Steven Wright


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [RFA]: Add misc_obstack to object files.
@ 2001-06-25 22:26 Michael Elizabeth Chastain
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Elizabeth Chastain @ 2001-06-25 22:26 UTC (permalink / raw)
  To: dan, ezannoni; +Cc: gdb-patches

I'd like to throw my hat in the ring, too.  I spent a whole weekend 
several months ago changing the type of all the section offset tables.
I wound up with a 3000 line patch that I didn't even try to get
approved.  (I was working on PR gdb/29, "gdb has fixed size MAX_SECTIONS").
So I care about section offset tables.

> I'm just adding a bin for colored paper, so people will put that stuff
> in there instead in the first place, and it will be plainly clear that
> they are supposed to be doing so.

So you don't have a manifestation of a bug in your hand; rather, you are
cleaning up something that is broken.  That's fine.  I am comfortable
with your rationale, now that I know what it is.

I see three specific problems:

. in lines 1984-1993 of symfile.c, you change the "psymtab" pointer from
  psymbol_obstack to misc_obstack.  It's not obvious why this structure
  is on misc_obstack.  Since it's not obvious, a comment would help.

. as Elena mentioned, there are other files which allocate section
  offsets on the psymbol_obstack.  I see somread.c in particular.
  
  Right now it's difficult to test somread.c, because even if you have
  access to hppa hardware, FSF gdb does not link for that configuration
  (PR gdb/63).  If you can't test it, and you don't want to go for a
  blind patch to somread.c, then I think that a FIXME comment would be
  appropriate, to explain to the next hpux maintainer why this target
  allocates section_offsets on psymbol_objstack when other targets use
  misc_obstack.  Something like: "FIXME: this should be misc_obstack,
  same as symfile.c, but I can't test this file so I'm leaving it as
  psymbol_obstack, which ought to continue working for now".

. the new comment on lines 121-137 of objfiles.c is not true yet,
  even after applying your patch.  For example, dbxread.c puts
  DBX_STRINGTAB on psymbol_obstack.  So I challenge your claim
  "All the stuff in objfile that was on the psymbol obstack,
  but didnt' belong, is in the misc obstack ...".

> The only thing even related to psymtabs there is the fact that they
> psymtab structure itself is allocated on the misc obstack, since it
> isn't a partial symbol either. It's a partial symbol table structure,
> containing pointers to partial symbols. 

If you put this in a comment, that would answer my first question
(which might be the same as Elena's question about an extraneous patch
fragment, but I don't know what fragment Elena is referring to).

> I could do them all now if it helps, but it'll make the patch bigger.

I like it better this way, one structure at a time.  I think that
Elena's looking for some sense of whether this is a standalone patch
or part of a bigger series -- not one big patch.

Here are the reasons that I prefer series:

. It's easier to proof-read one concept at a time.

. It's easier to read the first "proof of concept" when there's only
  one structure in it, rather than old structures.

. If something breaks in the future, and I need to binary-search old
  versions of gdb, this improves the granularity that I can resolve the
  bug too.  This is important for symbol table patches as a bug can
  manifest subtly (e.g. lookup_symbol bug -> infinite loop in "maint"
  command when running on an inferior copy of gdb).

Basically there are two different ideas here.  First there is the idea of
what should actually be in gdb.  I'm comfortable with your vision there.
Second there is the idea of structuring one's work so that other people
can deduce that it actually implements the concept (including checking
for implementation bugs).

Just my two cents.  I'll try not to write again, as Elena is the
maintainer here.

Michael


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2001-06-25 23:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-05-29 10:31 [RFA]: Add misc_obstack to object files Daniel Berlin
2001-06-25 13:59 ` Elena Zannoni
2001-06-25 19:07   ` Daniel Berlin
2001-06-25 20:49     ` Elena Zannoni
2001-06-25 21:12       ` Daniel Berlin
2001-06-25 22:34         ` Elena Zannoni
2001-06-25 23:21           ` Daniel Berlin
2001-06-25 22:26 Michael Elizabeth Chastain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox