Re: [PATCH] Support for multiple DWARF comp unit headers

[Petr Sorfa <petrs@caldera.com>]

Hi Jim,

> I think this patch is too big to review in one gulp.  Could you break
> this up into a succession of smaller patches?  For example, moving the
> abbrev tables into the CU header structure could be the first patch;
> making it possible to have more than one CU read in at a time would be
> the next (even though no code would use more than one yet); and adding
> support for DW_FORM_ref_addr could be the last.  Perhaps you can see
> finer divisions to make; the finer you can get them, the better.
Heh, you should have seen the original patch ;o) I've already split this
one. But yes, you are right, this patch is too big, so I will split it
into the smaller patches as you have suggested.

> Why does build_die_ref reject the result from follow_die_ref when its
> type isn't filled in?  If the DIE being referred to doesn't represent
> a type, won't this cause it to be re-read unnecessarily?
A valid point, I can't remember the exact reason for this, but I think
it is to handle and odd case that occurs once in a while. I'll verify
this when I split the patch.

> What happens if we hit a DW_FORM_ref_addr pointing to a DIE, and then
> later hit a DW_FORM_ref_addr pointing to one of its parents?  Won't we
> end up calling find_die_ref twice, and adding the dies in the first
> tree to the reference hash table twice?
I look into this when I do the split.

> Just out of curiosity --- after we've built `struct die_info' objects
> for a CU, we don't need the abbrev table any more, do we?  I may be
> misremembering the way the code behaves, but it seems to me that one
> could construct an abbrev table before calling read_comp_unit, and
> then throw it away when done.  Perhaps the abbrev tables are not big
> enough for this to be worth thinking about.
As Daniel pointed out the amount of space taken up by abbrev tables is
fairly tiny, usually you don't get over 15 entries and each entry is
usually less than 8 bytes. Again, I'll verify your observation, but I
think it might be worthwhile to keep it around for future DWARF
compatibility. For instance, an optimization that might be done later is
to only read necessary DWARF info when needed (this is a technique
implemented in a debugger I work with), in this case, hanging on the
abbrev table is a good idea.

Thanks for your great comments, I'll have the patch split and resent
this week. I'll also be sending some other smaller patches for support
of various little DWARF features.

Petr

Thanks 

> 
> Petr Sorfa <petrs@caldera.com> writes:
> 
> > Hi,
> >
> > Patch for supporting multiple DWARF comp unit headers. This is necessary
> > for supporting DW_FORM_ref_addr properly when the reference is to a
> > compilation unit outside of the current one. The current code only
> > supports one comp unit at a time.
> >
> > 2002-07-03 Petr Sorfa (petrs@caldera.com)
> >
> >         * dwarf2read.c (build_die_ref): New function that builds
> >           a new die reference if necessary, will search across
> >           multiple comp units to find the reference.
> >           (find_die_ref): New function that searches for a die
> >           reference for a given comp unit.
> >           (find_cu_header_from_begin_offset): New function that
> >           searches through a link list of comp unit headers for
> >           a comp unit that matches the given offset.
> >           (register_cu_header): New function that registers a
> >           comp unit header by allocating space for it and
> >           adding it to the comp unit header list.
> >           (free_cu_header_list): New function that frees up the
> >           memory taken up by the list of comp unit headers
> >           associated with a process.
> >           (dwarf_new_init): New function that initializes DWARF
> >           information for a new process.
> >           (struct comp_unit_head): Adds several new members
> >           to contain fully all the comp unit head's info.
> >           (first_cu_header): A new global variable that points
> >           to the first comp unit head for the process.
> >           (cu_header_offset): Global variable removed, as is now
> >           present in each comp unit head.
> >           (dwarf_abbrevs): Global variable removed, as is now
> >           present in each comp unit head.
> >           (dwarf2_read_abbrevs):
> >           (dwarf2_empty_abbrev_table):
> >           (dwarf2_lookup_abbrev):
> >           (dwarf_attr):
> >           (die_is_declaration):
> >           (read_base_type):
> >           (dwarf2_get_pc_bounds):
> >           (read_tag_string_type):
> >           (dwarf2_linkage_name):
> >           (dwarf2_get_ref_die_offset):
> >           All of these functions have a new argument added to
> >           pass the current comp unit header. All calls to these
> >           functions have been updated to handle the additional
> >           argument.
> >
> >         * elfread.c (elf_new_init): Now calls dwarf_new_init()
> >           to initialize DWARF related information for the
> >           new process.? err