This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA]: Add misc_obstack to object files.
Daniel Berlin writes:
> Elena Zannoni <ezannoni@cygnus.com> writes:
>
> > Daniel Berlin writes:
> > > Elena Zannoni <ezannoni@cygnus.com> writes:
> > >
> > > > Daniel Berlin writes:
> > > > >
> > > > > This way, we stop putting things in the psymbol_obstack that don't
> > > > > belong.
> > > > >
> > > >
> > > > Dan, can you elaborate a bit on why we need this?
> > > Because these things don't belong in the psymbol obstack.
> > > They are section offsets, not partial symbols.
> > >
> >
> > Dan, I am really sorry, but still I cannot see a real reason for this
> > change emerging from your mail.
>
> Um.
> Okay.
> According to objfiles.h:
>
> struct obstack psymbol_obstack; /* Partial symbols */
> struct obstack symbol_obstack; /* Full symbols */
> struct obstack type_obstack; /* Types */
>
>
> Notice it doesn't say that section offsets, or other random junk,
> should be on the psymbol_obstack.
> It says "Partial symbols".
> Now, if you want to remove the partial symbols, for that object file,
> theoretically, you just remove the psymbol_obstack. However, because
> we allocate other random things, like section offset tables, etc,
> which have zip to do with partial symbols, on that obstack, we won't
> remove just the partial symbols by removing that obstack. We'll blow
> away the section offset table, the psymtab structures, etc.
> This should not be occurring.
> It makes as much sense as allocating object file section offsets on the
> type_obstack, which is for types only. That is to say, none.
>
Yes, and this is the bug I was alluding to. You mentioned you observed
that things like the section tables were freed too early. When does
this occur? Did you try to move around the call to
free_obstack(psymbol_obstack)? What motivated you to try and do that?
That's really what I am asking.
> It's the equivalent of having a trash bin marked "white paper only",
> and having people knowing throw all kinds of colored paper in there,
> for lack of a better place to throw colored paper.
> They don't belong.
> Not only that, but if you later wanted to remove the white paper, you'd have to sort out
> the colored paper, and do something with it, first.
>
> 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.
>
>
> > You are doing this in response to some bug you saw, I assume, but what that
> > bug is has not been explained.
> No, i'm doing it because partial symbols belong on the psymbol
> obstack, but section offsets, and other random things don't.
> In practice, we've not followed this, because nobody bothered to
> create somewhere *else* to put them. I'm attempting to bring us
> to doing what we should have been doing in the first place, by
> creating this place to put them.
>
Yes, true.
> Had I been trying to fix a bug, i would explain what the bug was, and
> where it showed up.
>
> This is not a bug, in that nothing is specifically *broken*. It's
> perfectly okay to allocate these structures on that obstack, given
> that gdb never frees the obstack, it's just not in line with what the
> comments say, and the purpose of the psymbol obstack.
>
Isn't the psymbol_obstack freed when you re-read symbols in?
> >
> >
> > > > Why are you removing
> > > > the partial symbol table from the psymbol_objstack?
> > > I'm not. At all in fact.
> > > I'm specifically trying to make it so *all* that is in the
> > > psymbol_obstack is the partial symbol table.
> > > we've been stuffing random stuff like section offsets into the
> > > psymbol_obstack, because it was handy.
> > > This is wrong.
> > > If we need a misc place to put info like section offsets, that isn't
> > > large enough to deserve it's own obstack on it's own, then all the
> > > misc stuff together should go in a misc_obstack.
> >
> > Then your patch includes a diff that is unrelated.
> Um, how so?
> Please point out the portion of the diff unrelated.
> 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.
Yes, that's the one.
I would think that the psymtab itself is better coupled with
psymbol_objstack, than misc_obstack.
I would think it safer to free both of them or none, than to risk
freeing the section_offsets and also the psymbol table by mistake.
> The actual partial symbols that fill that table are still allocated on
> the psymbol_obstack.
> As they should be.
yes, but looking at the way the symbol table is allocated, it follows
the same logic, symbol_obstack includes the symtab itself as well.
I mean, the real problem that you are trying to address is that you
need a different lifetime for psymbols vs psymtab, and for psymbols vs
section_offsets. So, why is this necessary? Are you addressing a
limitation that you have found while trying to do something else? Why
is it a problem that section_offsets, psymbols and psymtab have the
same lifetime?
[I am not arguing that section offsets are logically related to psymbols]
> >
> > >
> > > > Was this intended?
> > > > There are several other things that get allocated on the
> > > > psymbol_objstack, for instance look in xcoffread.c, somread.c,
> > > > etc. Why just remove the section offset stuff?
> > > Because I haven't gotten to the others yet.
> > > And there is no harm in taking time to make sure it gets done right.
> > >
> >
> > If you have a roadmap in mind, you should explain it when you submit
> > the patch.
>
> I have no roadmap for when i'd be getting to that.
> Eventually.
> I could do them all now if it helps, but it'll make the patch bigger.
>
I would suggest that you put the section offsets stuff on its own
separate obstack for now, but do that across the board, to avoid
inconsistencies. That would be a self contained patch.
Follow up patches would move other pieces.
Elena
>
> >
> > > >
> > > > If your intent was to change the allocation of the section offset
> > > > information, from psymbol_obstack to misc_obstack, why not modify also
> > > > the same allocations done in xcoffread.c, elfread.c, somread.c?
> > > Because i haven't gotten there yet.
> > > :)
> > > I did it as I saw them.
> > >
> > > In actuality, what i was doing was freeing the psymbol obstack when we
> > > were completely done with partial symbols, and seeing what broke.
> > > They never broke, so i never noticed them.
> > >
> > > When i submit a large patch that does all this in one fell swoop,
> > > someone would say to break it down.
> > > So consider this a first step, rather than a done deal.
> > >
> >
> > I am encouraging you to resubmit the patch accompanied by coherent
> > explanation and rationale.
> I believe you are misunderstanding the purpose of the patch, and what
> it does, or some portion of it.
> Please let me know if the above doesn't clear it up.
>
>
> >
> > Elena
> >
> >
> > > >
> > > > Thanks
> > > > Elena
> > > >
> > > >
> > > > > 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 *) §ion, 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 *) §ion, 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
> > > > >
> > >
> > > --
> > > "I went into a clothes store the other day and a salesman walked
> > > up to me and said, "Can I help you?" and I said "Yeah, do you
> > > got anything I like?" He said, "What do you mean do we have
> > > anything you like?" I said, "You started this."
> > > "-Steven Wright
> > >
>
> --
> "So I figured I'd leave the area, because I had no ties there
> anyway except for this girl I was seeing. We had conflicting
> attitudes: I really wasn't into meditating and she wasn't
> really into being alive. I told her I knew when I was going to
> die because my birth certificate has an expiration date.
> "-Steven Wright
>