This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfc/rfa] accept DW_TAG_namespace and friends, possibly on 5.3
- From: Elena Zannoni <ezannoni at redhat dot com>
- To: David Carlton <carlton at math dot stanford dot edu>
- Cc: gdb-patches at sources dot redhat dot com, Elena Zannoni <ezannoni at redhat dot com>, Jim Blandy <jimb at redhat dot com>
- Date: Tue, 22 Oct 2002 18:27:09 -0400
- Subject: Re: [rfc/rfa] accept DW_TAG_namespace and friends, possibly on 5.3
- References: <ro1smyy5hs6.fsf@jackfruit.Stanford.EDU>
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
> <http://sources.redhat.com/ml/gdb/2002-08/msg00312.html>, 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 <carlton@math.stanford.edu>
>
> * 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_<unknown>";
> }