This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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>";
 >      }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]