From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27955 invoked by alias); 8 Jul 2002 15:28:05 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 27943 invoked from network); 8 Jul 2002 15:28:03 -0000 Received: from unknown (HELO tetsuo.nj.caldera.com) (63.124.204.226) by sources.redhat.com with SMTP; 8 Jul 2002 15:28:03 -0000 Received: from caldera.com (localhost.localdomain [127.0.0.1]) by tetsuo.nj.caldera.com (8.11.6/8.11.6) with ESMTP id g68Fdt413870; Mon, 8 Jul 2002 11:39:55 -0400 Message-ID: <3D29B24A.40619AE9@caldera.com> Date: Mon, 08 Jul 2002 09:41:00 -0000 From: Petr Sorfa Organization: Caldera X-Accept-Language: en MIME-Version: 1.0 To: Jim Blandy CC: "gdb-patches@sources.redhat.com" Subject: Re: [PATCH] Support for multiple DWARF comp unit headers References: <3D2365EA.E739FE66@caldera.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-07/txt/msg00112.txt.bz2 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 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