From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17189 invoked by alias); 22 Oct 2002 22:29:55 -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 17182 invoked from network); 22 Oct 2002 22:29:54 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 22 Oct 2002 22:29:54 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id g9MM8Yw11392 for ; Tue, 22 Oct 2002 18:08:34 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id g9MMTsf30888 for ; Tue, 22 Oct 2002 18:29:54 -0400 Received: from localhost.redhat.com (IDENT:ENKNaXiTbJ7kyBtkhwpNGZi4Ec2Tdso5@tooth.toronto.redhat.com [172.16.14.29]) by pobox.corp.redhat.com (8.11.6/8.11.6) with ESMTP id g9MMTpw23865; Tue, 22 Oct 2002 18:29:51 -0400 Received: by localhost.redhat.com (Postfix, from userid 469) id 831BDFF79; Tue, 22 Oct 2002 18:27:09 -0400 (EDT) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15797.53437.183222.336553@localhost.redhat.com> Date: Tue, 22 Oct 2002 15:29:00 -0000 To: David Carlton Cc: gdb-patches@sources.redhat.com, Elena Zannoni , Jim Blandy Subject: Re: [rfc/rfa] accept DW_TAG_namespace and friends, possibly on 5.3 In-Reply-To: References: X-SW-Source: 2002-10/txt/msg00440.txt.bz2 David Carlton writes: > The current situation around C++ namespace debugging info is that GCC > isn't generating it because, if it were generating it, it would > produce debugging info that GDB really can't handle. Basically, > DW_TAG_namespace entries have children that are important, so GDB has > to know a little bit about those nodes in order not to miss large > chunks of debugging info. (This is true whether or not GDB wants to > do anything particularly namespace-specific with that debugging info.) > > So it seems to me like it would be a good idea to change GDB as > quickly as possible to not get confused by DW_TAG_namespace (as well > as DW_TAG_imported_declaration and DW_TAG_imported_module): we > shouldn't wait until adding more namespace functionality to GDB. For > example, if that support makes it into GDB 5.3, then maybe GCC 3.3 > will be able to generate the appropriate debugging info, so when a GDB > 5.4 (or whatever) rolls around that handles namespaces better, users > will be able to take advantage of it immediately (instead of having to > wait for the next GCC release). > yes > Here are some patches to let GDB accept that debugging information: I > think it would be a good idea to get it into 5.3 as well as mainline, > if possible. They're quite minimal changes: they make sure that, when > reading partial symbols, we descend into DW_TAG_namespace entries, > that when reading full symbols, we read children of DW_TAG_namespace > entries (but we don't keep around any more namespace information than > we do currently: e.g. we still get names from > DW_AT_MIPS_linkage_name), and that we don't complain about the > presence of DW_TAG_imported_declaration or DW_TAG_imported_module (but > we also don't do anything useful about that info). > > I've tested this using current GCC, using GCC as patched according to > , and using > GCC as patched according to that message plus the following patch: > > --- dwarf2out.c-danielb Fri Oct 18 11:39:46 2002 > +++ dwarf2out.c Fri Oct 18 11:38:46 2002 > @@ -11453,7 +11453,11 @@ gen_namespace_die (decl, context_die) > { > /* Output a real namespace */ > dw_die_ref namespace_die = new_die (DW_TAG_namespace, context_die, decl); > - add_name_and_src_coords_attributes (namespace_die, decl); > + /* Anonymous namespaces shouldn't have a DW_AT_name. */ > + if (strncmp (dwarf2_name (decl, 0), "_GLOBAL__N", 10) == 0) > + add_src_coords_attributes (namespace_die, decl); > + else > + add_name_and_src_coords_attributes (namespace_die, decl); > equate_decl_number_to_die (decl, namespace_die); > } > else > > In all cases, there are no new regressions. Unfortunately, I don't > currently have a version of GCC that emits many > DW_TAG_imported_declarations or any DW_TAG_imported_modules; I hope > that I'll have one soon (Daniel Berlin is working on it but he's busy; > maybe I'll try to work on it myself, too), but it seems to me that the > part of this patch that refers to such entries is sufficiently small > that it should be correct. It's the following bit from within > process_die(): > > + case DW_TAG_imported_declaration: > + case DW_TAG_imported_module: > + /* FIXME: carlton/2002-10-16: Eventually, we should use the > + information contained in these. DW_TAG_imported_declaration > + dies shouldn't have children; DW_TAG_imported_module dies > + shouldn't in the C++ case, but conceivably could in the > + Fortran case, so we'll have to replace this gdb_assert if > + Fortran compilers start generating that info. */ > + gdb_assert (!die->has_children); > + break; > > (By the way, if you're looking at the DWARF 3 draft standard, I should > warn you that example D.3 makes it look like the > DW_TAG_imported_module entry labelled as (6) has children. I've > confirmed with the relevant people that this is a typo in the > indentation of that example.) > > So: what do people think? Should it go into mainline? Should it go > into 5.3? Should I wait until I've tested it using a GCC that > generates a bit more debugging info? Normally, I would prefer to wait > for a better GCC, but since 5.3 is going to be released fairly soon, I > wanted to raise the issue now. I think it could go in. But I have some questions below. > > David Carlton > carlton@math.stanford.edu > > 2002-10-16 David Carlton > > * dwarf2read.c (dwarf_tag_name): Add DWARF 3 names. > (dwarf_attr_name): Ditto. > (dwarf_type_encoding_name): Ditto. > (scan_partial_symbols): Descend into DW_TAG_namespace entries. > (process_die): Handle DW_TAG_namespace, > DW_TAG_imported_declaration, DW_TAG_imported_module. > (read_namespace): New function. > > Index: dwarf2read.c > =================================================================== > RCS file: /cvs/src/src/gdb/dwarf2read.c,v > retrieving revision 1.69 > diff -u -p -r1.69 dwarf2read.c > --- dwarf2read.c 16 Oct 2002 20:50:21 -0000 1.69 > +++ dwarf2read.c 16 Oct 2002 22:53:33 -0000 > @@ -842,6 +842,9 @@ static void read_structure_scope (struct > static void read_common_block (struct die_info *, struct objfile *, > const struct comp_unit_head *); > > +static void read_namespace (struct die_info *die, struct objfile *objfile, > + const struct comp_unit_head *cu_header); > + > static void read_enumeration (struct die_info *, struct objfile *, > const struct comp_unit_head *); > > @@ -1332,6 +1335,11 @@ scan_partial_symbols (char *info_ptr, st > > int nesting_level = 1; > > + /* What level do we consider to be file scope? This is normally 1, > + but can get pushed up by DW_TAG_namespace entries. */ > + > + int file_scope_level = 1; > + > *lowpc = ((CORE_ADDR) -1); > *highpc = ((CORE_ADDR) 0); > > @@ -1354,7 +1362,7 @@ scan_partial_symbols (char *info_ptr, st > { > *highpc = pdi.highpc; > } > - if ((pdi.is_external || nesting_level == 1) > + if ((pdi.is_external || nesting_level == file_scope_level) > && !pdi.is_declaration) > { > add_partial_symbol (&pdi, objfile, cu_header); > @@ -1367,7 +1375,7 @@ scan_partial_symbols (char *info_ptr, st > case DW_TAG_structure_type: > case DW_TAG_union_type: > case DW_TAG_enumeration_type: > - if ((pdi.is_external || nesting_level == 1) > + if ((pdi.is_external || nesting_level == file_scope_level) > && !pdi.is_declaration) > { > add_partial_symbol (&pdi, objfile, cu_header); > @@ -1376,37 +1384,54 @@ scan_partial_symbols (char *info_ptr, st > case DW_TAG_enumerator: > /* File scope enumerators are added to the partial symbol > table. */ Could you add a comment here about why/how we know that the level should be 2 here? > - if (nesting_level == 2) > + if (nesting_level == file_scope_level + 1) > add_partial_symbol (&pdi, objfile, cu_header); > break; > case DW_TAG_base_type: > /* File scope base type definitions are added to the partial > symbol table. */ > - if (nesting_level == 1) > + if (nesting_level == file_scope_level) > add_partial_symbol (&pdi, objfile, cu_header); > break; > + case DW_TAG_namespace: > + /* FIXME: carlton/2002-10-16: we're not yet doing > + anything useful with this, but for now make sure that > + these tags at least don't cause us to miss any > + important symbols. */ > + if (pdi.has_children) > + file_scope_level++; > default: > break; > } > } > > - /* If the die has a sibling, skip to the sibling. > - Do not skip enumeration types, we want to record their > - enumerators. */ > - if (pdi.sibling && pdi.tag != DW_TAG_enumeration_type) > + /* If the die has a sibling, skip to the sibling. Do not skip > + enumeration types, we want to record their enumerators. Do > + not skip namespaces, we want to record symbols inside > + them. */ > + if (pdi.sibling > + && pdi.tag != DW_TAG_enumeration_type > + && pdi.tag != DW_TAG_namespace) > { > info_ptr = pdi.sibling; > } > else if (pdi.has_children) > { > - /* Die has children, but the optional DW_AT_sibling attribute > - is missing. */ > + /* Die has children, but either the optional DW_AT_sibling > + attribute is missing or we want to look at them. */ > nesting_level++; > } > > if (pdi.tag == 0) > { > nesting_level--; > + /* If this is the end of a DW_TAG_namespace entry, then > + decrease the file_scope_level, too. */ > + if (nesting_level < file_scope_level) > + { > + file_scope_level--; > + gdb_assert (nesting_level == file_scope_level); > + } > } > } > Can you explain a bit about the levels? I am getting confused. About the new DW_TAGs etc. Are they handled by objdump and readelf? If not, you should add those to binutils and dwarf2.h as well. Elena > @@ -1706,6 +1731,19 @@ process_die (struct die_info *die, struc > break; > case DW_TAG_common_inclusion: > break; > + case DW_TAG_namespace: > + read_namespace (die, objfile, cu_header); > + break; > + case DW_TAG_imported_declaration: > + case DW_TAG_imported_module: > + /* FIXME: carlton/2002-10-16: Eventually, we should use the > + information contained in these. DW_TAG_imported_declaration > + dies shouldn't have children; DW_TAG_imported_module dies > + shouldn't in the C++ case, but conceivably could in the > + Fortran case, so we'll have to replace this gdb_assert if > + Fortran compilers start generating that info. */ > + gdb_assert (!die->has_children); > + break; > default: > new_symbol (die, NULL, objfile, cu_header); > break; > @@ -2921,6 +2959,27 @@ read_common_block (struct die_info *die, > } > } > > +/* Read a C++ namespace. */ > + > +/* FIXME: carlton/2002-10-16: For now, we don't actually do anything > + useful with the namespace data: we just process its children. */ > + > +static void > +read_namespace (struct die_info *die, struct objfile *objfile, > + const struct comp_unit_head *cu_header) > +{ > + if (die->has_children) > + { > + struct die_info *child_die = die->next; > + > + while (child_die && child_die->tag) > + { > + process_die (child_die, objfile, cu_header); > + child_die = sibling_die (child_die); > + } > + } > +} > + > /* Extract all information from a DW_TAG_pointer_type DIE and add to > the user defined type vector. */ > > @@ -5493,6 +5552,22 @@ dwarf_tag_name (register unsigned tag) > return "DW_TAG_variable"; > case DW_TAG_volatile_type: > return "DW_TAG_volatile_type"; > + case DW_TAG_dwarf_procedure: > + return "DW_TAG_dwarf_procedure"; > + case DW_TAG_restrict_type: > + return "DW_TAG_restrict_type"; > + case DW_TAG_interface_type: > + return "DW_TAG_interface_type"; > + case DW_TAG_namespace: > + return "DW_TAG_namespace"; > + case DW_TAG_imported_module: > + return "DW_TAG_imported_module"; > + case DW_TAG_unspecified_type: > + return "DW_TAG_unspecified_type"; > + case DW_TAG_partial_unit: > + return "DW_TAG_partial_unit"; > + case DW_TAG_imported_unit: > + return "DW_TAG_imported_unit"; > case DW_TAG_MIPS_loop: > return "DW_TAG_MIPS_loop"; > case DW_TAG_format_label: > @@ -5637,7 +5712,30 @@ dwarf_attr_name (register unsigned attr) > return "DW_AT_virtuality"; > case DW_AT_vtable_elem_location: > return "DW_AT_vtable_elem_location"; > - > + case DW_AT_allocated: > + return "DW_AT_allocated"; > + case DW_AT_associated: > + return "DW_AT_associated"; > + case DW_AT_data_location: > + return "DW_AT_data_location"; > + case DW_AT_stride: > + return "DW_AT_stride"; > + case DW_AT_entry_pc: > + return "DW_AT_entry_pc"; > + case DW_AT_use_UTF8: > + return "DW_AT_use_UTF8"; > + case DW_AT_extension: > + return "DW_AT_extension"; > + case DW_AT_ranges: > + return "DW_AT_ranges"; > + case DW_AT_trampoline: > + return "DW_AT_trampoline"; > + case DW_AT_call_column: > + return "DW_AT_call_column"; > + case DW_AT_call_file: > + return "DW_AT_call_file"; > + case DW_AT_call_line: > + return "DW_AT_call_line"; > #ifdef MIPS > case DW_AT_MIPS_fde: > return "DW_AT_MIPS_fde"; > @@ -6074,6 +6172,8 @@ dwarf_type_encoding_name (register unsig > return "DW_ATE_unsigned"; > case DW_ATE_unsigned_char: > return "DW_ATE_unsigned_char"; > + case DW_ATE_imaginary_float: > + return "DW_ATE_imaginary_float"; > default: > return "DW_ATE_"; > }