[rfc/rfa] accept DW_TAG_namespace and friends, possibly on 5.3

[David Carlton <carlton at math dot stanford dot edu>]

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).

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.

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.  */
-	      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);
+	    }
 	}
     }
 
@@ -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>";
     }