[Dwarf Patch] Improve pubnames and pubtypes generation. (issue6197069)

[Dwarf Patch] Improve pubnames and pubtypes generation. (issue6197069)

[Sterling Augustine]

This is a revised edition of the fix pubnames patch discussed earlier; it
addresses all the earlier comments made.

This edition of the patch has three changes from the earlier one:

1. Fix a typo (anoymous -> anonymous).
2. Move enumerator names to pubnames from pubtypes.
3. Switch to using the comdat_type_p field.

It sticks with -gpubnames as earlier discussed.

Anything else?

Sterling

2012-06-14   Sterling Augustine  <saugustine@google.com>
        Cary Coutant  <ccoutant@google.com>

	* dwarf2out.c (is_cu_die, is_namespace_die, is_class_die,
	add_AT_pubnames, add_enumerator_pubname, want_pubnames): New functions.
	(comdat_type_struct): New field 'skeleton_die'.
	(breakout_comdat_types): Update it.
	(add_pubname): Rework logic.  Call is_class_die, is_cu_die and
	is_namespace_die.  Fix minor style violation.  Call want_pubnames.
	(add_pubname_string): Call want_pubnames.
	(add_pubtype): Rework logic for calculating type name.  Call
	is_namespace_die.  Call want_pubnames.
	(output_pubnames): Move conditional logic deciding when to produce the
	section from dwarf2out_finish.  Use new skeleton_die field.
	(base_type_die): Call add_pubtype.
	(gen_enumeration_type_die): Unconditionally call add_pubtype.
	(gen_subprogram_die): Adjust calls to add_pubname.
	(gen_namespace_die): Call add_pubname_string.
	(dwarf2out_finish): Call add_AT_pubnames; Move logic on when to
	produce pubnames and pubtypes sections to output_pubnames.
	(common.opt): New option '-gpubnames'.
	(invoke.texi): Document it.



Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 188622)
+++ gcc/doc/invoke.texi	(working copy)
@@ -4795,6 +4795,10 @@
 if neither of those are supported), including GDB extensions if at all
 possible.
 
+@item -gpubnames
+@opindex gpubnames
+Generate dwarf .debug_pubnames and .debug_pubtypes sections.
+
 @item -gstabs
 @opindex gstabs
 Produce debugging information in stabs format (if that is supported),
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 188622)
+++ gcc/dwarf2out.c	(working copy)
@@ -2539,6 +2539,7 @@
 {
   dw_die_ref root_die;
   dw_die_ref type_die;
+  dw_die_ref skeleton_die;
   char signature[DWARF_TYPE_SIGNATURE_SIZE];
   struct comdat_type_struct *next;
 }
@@ -3013,6 +3014,7 @@
 static void output_comdat_type_unit (comdat_type_node *);
 static const char *dwarf2_name (tree, int);
 static void add_pubname (tree, dw_die_ref);
+static void add_enumerator_pubname (const char *, dw_die_ref);
 static void add_pubname_string (const char *, dw_die_ref);
 static void add_pubtype (tree, dw_die_ref);
 static void output_pubnames (VEC (pubname_entry,gc) *);
@@ -3142,6 +3144,7 @@
 				     const char *, const char *);
 static void output_loc_list (dw_loc_list_ref);
 static char *gen_internal_sym (const char *);
+static bool want_pubnames (void);
 
 static void prune_unmark_dies (dw_die_ref);
 static void prune_unused_types_mark_generic_parms_dies (dw_die_ref);
@@ -5982,6 +5985,23 @@
 	       || c->die_tag == DW_TAG_type_unit);
 }
 
+/* Returns true iff C is a namespace DIE.  */
+
+static inline bool
+is_namespace_die (dw_die_ref c)
+{
+  return c && c->die_tag == DW_TAG_namespace;
+}
+
+/* Returns true iff C is a class or structure DIE.  */
+
+static inline bool
+is_class_die (dw_die_ref c)
+{
+  return c && (c->die_tag == DW_TAG_class_type
+               || c->die_tag == DW_TAG_structure_type);
+}
+
 static char *
 gen_internal_sym (const char *prefix)
 {
@@ -6568,6 +6588,7 @@
            declaration into the new type unit DIE, then remove this DIE
 	   from the main CU (or replace it with a skeleton if necessary).  */
 	replacement = remove_child_or_replace_with_skeleton (unit, c, prev);
+	type_node->skeleton_die = replacement;
 
         /* Break out nested types into their own type units.  */
         break_out_comdat_types (c);
@@ -8041,6 +8062,27 @@
     }
 }
 
+/* Whether to generate the DWARF accelerator tables in .debug_pubnames
+   and .debug_pubtypes.  This is configured per-target, but can be
+   overridden by the -gpubnames or -gno-pubnames options.  */
+
+static inline bool
+want_pubnames (void)
+{
+  return (debug_generate_pub_sections != -1
+	  ? debug_generate_pub_sections
+	  : targetm.want_debug_pub_sections);
+}
+
+/* Add the DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes attributes.  */
+
+static void
+add_AT_pubnames (dw_die_ref die)
+{
+  if (want_pubnames ())
+    add_AT_flag (die, DW_AT_GNU_pubnames, 1);
+}
+
 /* Output a comdat type unit DIE and its children.  */
 
 static void
@@ -8111,7 +8153,7 @@
 static void
 add_pubname_string (const char *str, dw_die_ref die)
 {
-  if (targetm.want_debug_pub_sections)
+  if (want_pubnames ())
     {
       pubname_entry e;
 
@@ -8124,14 +8166,32 @@
 static void
 add_pubname (tree decl, dw_die_ref die)
 {
-  if (targetm.want_debug_pub_sections && TREE_PUBLIC (decl))
+  if (!want_pubnames ())
+    return;
+
+  if ((TREE_PUBLIC (decl) && !is_class_die (die->die_parent))
+      || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
     {
       const char *name = dwarf2_name (decl, 1);
+
       if (name)
 	add_pubname_string (name, die);
     }
 }
 
+/* Add an enumerator to the pubnames section.  */
+
+static void
+add_enumerator_pubname (const char *scope_name, dw_die_ref die)
+{
+  pubname_entry e;
+
+  gcc_assert (scope_name);
+  e.name = concat (scope_name, get_AT_string (die, DW_AT_name), NULL);
+  e.die = die;
+  VEC_safe_push (pubname_entry, gc, pubname_table, &e);
+}
+
 /* Add a new entry to .debug_pubtypes if appropriate.  */
 
 static void
@@ -8139,40 +8199,55 @@
 {
   pubname_entry e;
 
-  if (!targetm.want_debug_pub_sections)
+  if (!want_pubnames ())
     return;
 
-  e.name = NULL;
   if ((TREE_PUBLIC (decl)
-       || is_cu_die (die->die_parent))
+       || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
       && (die->die_tag == DW_TAG_typedef || COMPLETE_TYPE_P (decl)))
     {
-      e.die = die;
-      if (TYPE_P (decl))
-	{
-	  if (TYPE_NAME (decl))
+      tree scope = NULL;
+      const char *scope_name = "";
+      const char *sep = is_cxx () ? "::" : ".";
+      const char *name;
+
+      scope = TYPE_P (decl) ? TYPE_CONTEXT (decl) : NULL;
+      if (scope && TREE_CODE (scope) == NAMESPACE_DECL)
 	    {
-	      if (TREE_CODE (TYPE_NAME (decl)) == IDENTIFIER_NODE)
-		e.name = IDENTIFIER_POINTER (TYPE_NAME (decl));
-	      else if (TREE_CODE (TYPE_NAME (decl)) == TYPE_DECL
-		       && DECL_NAME (TYPE_NAME (decl)))
-		e.name = IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (decl)));
+          scope_name = lang_hooks.dwarf_name (scope, 1);
+          if (scope_name != NULL && scope_name[0] != '\0')
+            scope_name = concat (scope_name, sep, NULL);
 	      else
-	       e.name = xstrdup ((const char *) get_AT_string (die, DW_AT_name));
-	    }
+            scope_name = "";
 	}
+
+      if (TYPE_P (decl))
+        name = type_tag (decl);
       else
-	{
-	  e.name = dwarf2_name (decl, 1);
-	  if (e.name)
-	    e.name = xstrdup (e.name);
-	}
+        name = lang_hooks.dwarf_name (decl, 1);
 
       /* If we don't have a name for the type, there's no point in adding
 	 it to the table.  */
-      if (e.name && e.name[0] != '\0')
+      if (name != NULL && name[0] != '\0')
+        {
+          e.die = die;
+          e.name = concat (scope_name, name, NULL);
 	VEC_safe_push (pubname_entry, gc, pubtype_table, &e);
     }
+
+      /* Although it might be more consistent to add the pubinfo for the
+         enumerators as their dies are created, they should only be added if the
+         enum type meets the criteria above.  So rather than re-check the parent
+         enum type whenever an enumerator die is created, just output them all
+         here.  This isn't protected by the name conditional because anonymous
+         enums don't have names.  */
+      if (die->die_tag == DW_TAG_enumeration_type)
+        {
+          dw_die_ref c;
+
+          FOR_EACH_CHILD (die, c, add_enumerator_pubname (scope_name, c));
+        }
+    }
 }
 
 /* Output the public names table used to speed up access to externally
@@ -8185,6 +8260,12 @@
   unsigned long pubnames_length = size_of_pubnames (names);
   pubname_ref pub;
 
+  if (!want_pubnames () || !info_section_emitted)
+    return;
+  if (names == pubname_table)
+    switch_to_section (debug_pubnames_section);
+  else
+    switch_to_section (debug_pubtypes_section);
   if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
     dw2_asm_output_data (4, 0xffffffff,
       "Initial length escape value indicating 64-bit DWARF extension");
@@ -8212,9 +8293,22 @@
 	  || pub->die->die_offset != 0
 	  || !flag_eliminate_unused_debug_types)
 	{
-	  dw2_asm_output_data (DWARF_OFFSET_SIZE, pub->die->die_offset,
-			       "DIE offset");
+	  dw_offset die_offset = pub->die->die_offset;
 
+	  /* If we're putting types in their own .debug_types sections,
+	     the .debug_pubtypes table will still point to the compile
+	     unit (not the type unit), so we want to use the offset of
+	     the skeleton DIE (if there is one).  */
+	  if (pub->die->comdat_type_p && names == pubtype_table)
+	    {
+	      comdat_type_node_ref type_node = pub->die->die_id.die_type_node;
+
+	      if (type_node != NULL && type_node->skeleton_die != NULL)
+		die_offset = type_node->skeleton_die->die_offset;
+	    }
+
+	  dw2_asm_output_data (DWARF_OFFSET_SIZE, die_offset, "DIE offset");
+
 	  dw2_asm_output_nstring (pub->name, -1, "external name");
 	}
     }
@@ -9098,6 +9192,7 @@
   add_AT_unsigned (base_type_result, DW_AT_byte_size,
 		   int_size_in_bytes (type));
   add_AT_unsigned (base_type_result, DW_AT_encoding, encoding);
+  add_pubtype (type, base_type_result);
 
   return base_type_result;
 }
@@ -16180,7 +16275,6 @@
   else
     add_AT_flag (type_die, DW_AT_declaration, 1);
 
-  if (get_AT (type_die, DW_AT_name))
     add_pubtype (type, type_die);
 
   return type_die;
@@ -16844,6 +16938,7 @@
 	{
 	  subr_die = new_die (DW_TAG_subprogram, context_die, decl);
 	  add_AT_specification (subr_die, old_die);
+          add_pubname (decl, subr_die);
 	  if (get_AT_file (old_die, DW_AT_decl_file) != file_index)
 	    add_AT_file (subr_die, DW_AT_decl_file, file_index);
 	  if (get_AT_unsigned (old_die, DW_AT_decl_line) != (unsigned) s.line)
@@ -16858,6 +16953,7 @@
 	add_AT_flag (subr_die, DW_AT_external, 1);
 
       add_name_and_src_coords_attributes (subr_die, decl);
+      add_pubname (decl, subr_die);
       if (debug_info_level > DINFO_LEVEL_TERSE)
 	{
 	  add_prototyped_attribute (subr_die, TREE_TYPE (decl));
@@ -16969,7 +17065,6 @@
       }
 #endif
 
-	  add_pubname (decl, subr_die);
 	}
       else
 	{
@@ -16990,7 +17085,6 @@
 		  add_ranges_by_labels (subr_die, fde->dw_fde_second_begin,
 					fde->dw_fde_second_end,
 					&range_list_added);
-		  add_pubname (decl, subr_die);
 		  if (range_list_added)
 		    add_ranges (NULL);
 		}
@@ -17012,8 +17106,6 @@
 				 fde->dw_fde_begin);
 		  add_AT_lbl_id (subr_die, DW_AT_high_pc,
 				 fde->dw_fde_end);
-		  /* Add it.   */
-		  add_pubname (decl, subr_die);
 
 		  /* Build a minimal DIE for the secondary section.  */
 		  seg_die = new_die (DW_TAG_subprogram,
@@ -17049,7 +17141,6 @@
 	    {
 	      add_AT_lbl_id (subr_die, DW_AT_low_pc, fde->dw_fde_begin);
 	      add_AT_lbl_id (subr_die, DW_AT_high_pc, fde->dw_fde_end);
-	      add_pubname (decl, subr_die);
 	    }
 	}
 
@@ -19090,6 +19181,8 @@
       add_AT_die_ref (namespace_die, DW_AT_import, origin_die);
       equate_decl_number_to_die (decl, namespace_die);
     }
+  /* Bypass dwarf2_name's check for DECL_NAMELESS.  */
+  add_pubname_string (lang_hooks.dwarf_name (decl, 1), namespace_die);
 }
 
 /* Generate Dwarf debug information for a decl described by DECL.
@@ -22365,6 +22458,8 @@
     }
   htab_delete (comdat_type_table);
 
+  add_AT_pubnames (comp_unit_die ());
+
   /* Output the main compilation unit if non-empty or if .debug_macinfo
      or .debug_macro will be emitted.  */
   output_comp_unit (comp_unit_die (), have_macinfo);
@@ -22388,42 +22483,12 @@
       output_location_lists (comp_unit_die ());
     }
 
-  /* Output public names table if necessary.  */
-  if (!VEC_empty (pubname_entry, pubname_table))
-    {
-      gcc_assert (info_section_emitted);
-      switch_to_section (debug_pubnames_section);
-      output_pubnames (pubname_table);
-    }
-
-  /* Output public types table if necessary.  */
+  /* Output public names and types tables if necessary.  */
+  output_pubnames (pubname_table);
   /* ??? Only defined by DWARF3, but emitted by Darwin for DWARF2.
      It shouldn't hurt to emit it always, since pure DWARF2 consumers
      simply won't look for the section.  */
-  if (!VEC_empty (pubname_entry, pubtype_table))
-    {
-      bool empty = false;
-      
-      if (flag_eliminate_unused_debug_types)
-	{
-	  /* The pubtypes table might be emptied by pruning unused items.  */
-	  unsigned i;
-	  pubname_ref p;
-	  empty = true;
-	  FOR_EACH_VEC_ELT (pubname_entry, pubtype_table, i, p)
-	    if (p->die->die_offset != 0)
-	      {
-		empty = false;
-		break;
-	      }
-	}
-      if (!empty)
-	{
-	  gcc_assert (info_section_emitted);
-	  switch_to_section (debug_pubtypes_section);
-	  output_pubnames (pubtype_table);
-	}
-    }
+  output_pubnames (pubtype_table);
 
   /* Output the address range information if a CU (.debug_info section)
      was emitted.  We output an empty table even if we had no functions
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 188622)
+++ gcc/common.opt	(working copy)
@@ -2239,6 +2239,14 @@
 Common JoinedOrMissing
 Generate debug information in default extended format
 
+gno-pubnames
+Common RejectNegative Var(debug_generate_pub_sections, 0) Init(-1)
+Don't generate DWARF pubnames and pubtypes sections.
+
+gpubnames
+Common RejectNegative Var(debug_generate_pub_sections, 1)
+Generate DWARF pubnames and pubtypes sections.
+
 gno-record-gcc-switches
 Common RejectNegative Var(dwarf_record_gcc_switches,0) Init(1)
 Don't record gcc command line switches in DWARF DW_AT_producer.

--
This patch is available for review at http://codereview.appspot.com/6197069

Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue6197069)

[Sterling Augustine]

On Thu, May 10, 2012 at 9:08 AM, Sterling Augustine
<saugustine@google.com> wrote:
> The enclosed patch fixes many issues with pubnames and pubtypes. It generates
> them for many more variables and with mostly correct and canonical dwarf names.
>
> This patch should not affect any target that does not use pubnames.
>
> The exceptions to the canonical names are addressed in a separate patch in
> to the front end under review at http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00512.html.
>
> Tested with bootstrap and running the test_pubnames_and_indices.py script
> recently contributed to the GDB project.
>
> OK for mainline?
>
> Sterling
>
> 2012-05-10   Sterling Augustine  <saugustine@google.com>
>
>        * dwarf2out.c (DEBUG_PUBNAMES_SECTION_LABEL,
>        DEBUG_PUBTYPES_SECTION_LABEL): New macros.
>        (debug_pubnames_section_label, debug_pubtypes_section_label): New
>        globals.
>        (is_cu_die, is_namespace_die, is_class_die, add_AT_pubnames,
>        add_enumerator_pubname): New functions.
>        (add_pubname): Rework logic.  Call is_class_die, is_cu_die and
>        is_namespace_die.  Fix minor style violation.
>        (add_pubtype): Rework logic for calculating type name.  Call
>        is_namespace_die.
>        (output_pubnames): Move conditional logic deciding when to produce the
>        section from dwarf2out_finish.  Output debug_pubnames_section_label
>        and debug_pubtypes_section_label.
>        (base_type_die): Call add_pubtype.
>        (gen_enumeration_type_die): Unconditionally call add_pubtype.
>        (gen_namespace_die): Call add_pubname_string.
>        (dwarf2out_init): Generate debug_pubnames_section_label and
>        debug_pubtypes_section_label from DEBUG_PUBNAMES_SECTION_LABEL and
>        DEBUG_PUBTYPES_SECTION_LABEL respectively.
>        (dwarf2out_finish): Call add_AT_pubnames; Move logic on when to
>        produce pubnames and pubtypes sections to output_pubnames.
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c     (revision 187271)
> +++ gcc/dwarf2out.c     (working copy)
> @@ -3007,6 +3007,7 @@ static void output_comp_unit (dw_die_ref, int);
>  static void output_comdat_type_unit (comdat_type_node *);
>  static const char *dwarf2_name (tree, int);
>  static void add_pubname (tree, dw_die_ref);
> +static void add_enumerator_pubname (const char *, dw_die_ref);
>  static void add_pubname_string (const char *, dw_die_ref);
>  static void add_pubtype (tree, dw_die_ref);
>  static void output_pubnames (VEC (pubname_entry,gc) *);
> @@ -3210,6 +3211,12 @@ static void gen_scheduled_generic_parms_dies (void
>  #ifndef COLD_TEXT_SECTION_LABEL
>  #define COLD_TEXT_SECTION_LABEL         "Ltext_cold"
>  #endif
> +#ifndef DEBUG_PUBNAMES_SECTION_LABEL
> +#define DEBUG_PUBNAMES_SECTION_LABEL       "Ldebug_pubnames"
> +#endif
> +#ifndef DEBUG_PUBTYPES_SECTION_LABEL
> +#define DEBUG_PUBTYPES_SECTION_LABEL       "Ldebug_pubtypes"
> +#endif
>  #ifndef DEBUG_LINE_SECTION_LABEL
>  #define DEBUG_LINE_SECTION_LABEL       "Ldebug_line"
>  #endif
> @@ -3246,6 +3253,8 @@ static char cold_end_label[MAX_ARTIFICIAL_LABEL_BY
>  static char abbrev_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
>  static char debug_info_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
>  static char debug_line_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
> +static char debug_pubnames_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
> +static char debug_pubtypes_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
>  static char macinfo_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
>  static char loc_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
>  static char ranges_section_label[2 * MAX_ARTIFICIAL_LABEL_BYTES];
> @@ -5966,6 +5975,22 @@ is_cu_die (dw_die_ref c)
>   return c && c->die_tag == DW_TAG_compile_unit;
>  }
>
> +/* Returns true iff C is a namespace DIE.  */
> +
> +static inline bool
> +is_namespace_die (dw_die_ref c)
> +{
> +  return c && c->die_tag == DW_TAG_namespace;
> +}
> +
> +/* Returns true iff C is a class DIE.  */
> +
> +static inline bool
> +is_class_die (dw_die_ref c)
> +{
> +  return c && c->die_tag == DW_TAG_class_type;
> +}
> +
>  static char *
>  gen_internal_sym (const char *prefix)
>  {
> @@ -8033,6 +8058,20 @@ output_comp_unit (dw_die_ref die, int output_if_em
>     }
>  }
>
> +/* Add the DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes attributes.  */
> +
> +static void
> +add_AT_pubnames (dw_die_ref die)
> +{
> +  if (targetm.want_debug_pub_sections)
> +    {
> +      /* FIXME: Should use add_AT_pubnamesptr.  This works because most targets
> +         don't care what the base section is.  */
> +      add_AT_lineptr (die, DW_AT_GNU_pubnames, debug_pubnames_section_label);
> +      add_AT_lineptr (die, DW_AT_GNU_pubtypes, debug_pubtypes_section_label);
> +    }
> +}
> +
>  /* Output a comdat type unit DIE and its children.  */
>
>  static void
> @@ -8116,14 +8155,32 @@ add_pubname_string (const char *str, dw_die_ref di
>  static void
>  add_pubname (tree decl, dw_die_ref die)
>  {
> -  if (targetm.want_debug_pub_sections && TREE_PUBLIC (decl))
> +  if (!targetm.want_debug_pub_sections)
> +    return;
> +
> +  if ((TREE_PUBLIC (decl) && !is_class_die (die->die_parent))
> +      || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
>     {
>       const char *name = dwarf2_name (decl, 1);
> +
>       if (name)
>        add_pubname_string (name, die);
>     }
>  }
>
> +/* Add an enumerator to the pubnames section.  */
> +
> +static void
> +add_enumerator_pubname (const char *scope_name, dw_die_ref die)
> +{
> +  pubname_entry e;
> +
> +  gcc_assert (scope_name);
> +  e.name = concat (scope_name, get_AT_string (die, DW_AT_name), NULL);
> +  e.die = die;
> +  VEC_safe_push (pubname_entry, gc, pubtype_table, &e);
> +}
> +
>  /* Add a new entry to .debug_pubtypes if appropriate.  */
>
>  static void
> @@ -8134,37 +8191,52 @@ add_pubtype (tree decl, dw_die_ref die)
>   if (!targetm.want_debug_pub_sections)
>     return;
>
> -  e.name = NULL;
>   if ((TREE_PUBLIC (decl)
> -       || is_cu_die (die->die_parent))
> +       || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
>       && (die->die_tag == DW_TAG_typedef || COMPLETE_TYPE_P (decl)))
>     {
> -      e.die = die;
> -      if (TYPE_P (decl))
> -       {
> -         if (TYPE_NAME (decl))
> +      tree scope = NULL;
> +      const char *scope_name = "";
> +      const char *sep = is_cxx () ? "::" : ".";
> +      const char *name;
> +
> +      scope = TYPE_P (decl) ? TYPE_CONTEXT (decl) : NULL;
> +      if (scope && TREE_CODE (scope) == NAMESPACE_DECL)
>            {
> -             if (TREE_CODE (TYPE_NAME (decl)) == IDENTIFIER_NODE)
> -               e.name = IDENTIFIER_POINTER (TYPE_NAME (decl));
> -             else if (TREE_CODE (TYPE_NAME (decl)) == TYPE_DECL
> -                      && DECL_NAME (TYPE_NAME (decl)))
> -               e.name = IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (decl)));
> +          scope_name = lang_hooks.dwarf_name (scope, 1);
> +          if (scope_name != NULL && scope_name[0] != '\0')
> +            scope_name = concat (scope_name, sep, NULL);
>              else
> -              e.name = xstrdup ((const char *) get_AT_string (die, DW_AT_name));
> -           }
> +            scope_name = "";
>        }
> +
> +      if (TYPE_P (decl))
> +        name = type_tag (decl);
>       else
> -       {
> -         e.name = dwarf2_name (decl, 1);
> -         if (e.name)
> -           e.name = xstrdup (e.name);
> -       }
> +        name = lang_hooks.dwarf_name (decl, 1);
>
>       /* If we don't have a name for the type, there's no point in adding
>         it to the table.  */
> -      if (e.name && e.name[0] != '\0')
> +      if (name != NULL && name[0] != '\0')
> +        {
> +          e.die = die;
> +          e.name = concat (scope_name, name, NULL);
>        VEC_safe_push (pubname_entry, gc, pubtype_table, &e);
>     }
> +
> +      /* Although it might be more consistent to add the pubinfo for the
> +         enumerators as their dies are created, they should only be added if the
> +         enum type meets the criteria above.  So rather than re-check the parent
> +         enum type whenever an enumerator die is created, just output them all
> +         here.  This isn't protected by the name conditional because anoymous
> +         enums don't have names.  */
> +      if (die->die_tag == DW_TAG_enumeration_type)
> +        {
> +          dw_die_ref c;
> +
> +          FOR_EACH_CHILD (die, c, add_enumerator_pubname (scope_name, c));
> +        }
> +    }
>  }
>
>  /* Output the public names table used to speed up access to externally
> @@ -8177,6 +8249,18 @@ output_pubnames (VEC (pubname_entry, gc) * names)
>   unsigned long pubnames_length = size_of_pubnames (names);
>   pubname_ref pub;
>
> +  if (!targetm.want_debug_pub_sections || !info_section_emitted)
> +    return;
> +  if (names == pubname_table)
> +    {
> +      switch_to_section (debug_pubnames_section);
> +      ASM_OUTPUT_LABEL (asm_out_file, debug_pubnames_section_label);
> +    }
> +  else
> +    {
> +      switch_to_section (debug_pubtypes_section);
> +      ASM_OUTPUT_LABEL (asm_out_file, debug_pubtypes_section_label);
> +    }
>   if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
>     dw2_asm_output_data (4, 0xffffffff,
>       "Initial length escape value indicating 64-bit DWARF extension");
> @@ -9090,6 +9174,7 @@ base_type_die (tree type)
>   add_AT_unsigned (base_type_result, DW_AT_byte_size,
>                   int_size_in_bytes (type));
>   add_AT_unsigned (base_type_result, DW_AT_encoding, encoding);
> +  add_pubtype (type, base_type_result);
>
>   return base_type_result;
>  }
> @@ -16176,7 +16261,6 @@ gen_enumeration_type_die (tree type, dw_die_ref co
>   else
>     add_AT_flag (type_die, DW_AT_declaration, 1);
>
> -  if (get_AT (type_die, DW_AT_name))
>     add_pubtype (type, type_die);
>
>   return type_die;
> @@ -18923,6 +19007,8 @@ gen_namespace_die (tree decl, dw_die_ref context_d
>       add_AT_die_ref (namespace_die, DW_AT_import, origin_die);
>       equate_decl_number_to_die (decl, namespace_die);
>     }
> +  /* Bypass dwarf2_name's check for DECL_NAMELESS.  */
> +  add_pubname_string (lang_hooks.dwarf_name (decl, 1), namespace_die);
>  }
>
>  /* Generate Dwarf debug information for a decl described by DECL.
> @@ -20585,6 +20671,10 @@ dwarf2out_init (const char *filename ATTRIBUTE_UNU
>
>   ASM_GENERATE_INTERNAL_LABEL (debug_info_section_label,
>                               DEBUG_INFO_SECTION_LABEL, 0);
> +  ASM_GENERATE_INTERNAL_LABEL (debug_pubnames_section_label,
> +                              DEBUG_PUBNAMES_SECTION_LABEL, 0);
> +  ASM_GENERATE_INTERNAL_LABEL (debug_pubtypes_section_label,
> +                              DEBUG_PUBTYPES_SECTION_LABEL, 0);
>   ASM_GENERATE_INTERNAL_LABEL (debug_line_section_label,
>                               DEBUG_LINE_SECTION_LABEL, 0);
>   ASM_GENERATE_INTERNAL_LABEL (ranges_section_label,
> @@ -22183,6 +22273,8 @@ dwarf2out_finish (const char *filename)
>     }
>   htab_delete (comdat_type_table);
>
> +  add_AT_pubnames (comp_unit_die ());
> +
>   /* Output the main compilation unit if non-empty or if .debug_macinfo
>      will be emitted.  */
>   output_comp_unit (comp_unit_die (), debug_info_level >= DINFO_LEVEL_VERBOSE);
> @@ -22206,42 +22298,12 @@ dwarf2out_finish (const char *filename)
>       output_location_lists (comp_unit_die ());
>     }
>
> -  /* Output public names table if necessary.  */
> -  if (!VEC_empty (pubname_entry, pubname_table))
> -    {
> -      gcc_assert (info_section_emitted);
> -      switch_to_section (debug_pubnames_section);
> -      output_pubnames (pubname_table);
> -    }
> -
> -  /* Output public types table if necessary.  */
> +  /* Output public names and types tables if necessary.  */
> +  output_pubnames (pubname_table);
>   /* ??? Only defined by DWARF3, but emitted by Darwin for DWARF2.
>      It shouldn't hurt to emit it always, since pure DWARF2 consumers
>      simply won't look for the section.  */
> -  if (!VEC_empty (pubname_entry, pubtype_table))
> -    {
> -      bool empty = false;
> -
> -      if (flag_eliminate_unused_debug_types)
> -       {
> -         /* The pubtypes table might be emptied by pruning unused items.  */
> -         unsigned i;
> -         pubname_ref p;
> -         empty = true;
> -         FOR_EACH_VEC_ELT (pubname_entry, pubtype_table, i, p)
> -           if (p->die->die_offset != 0)
> -             {
> -               empty = false;
> -               break;
> -             }
> -       }
> -      if (!empty)
> -       {
> -         gcc_assert (info_section_emitted);
> -         switch_to_section (debug_pubtypes_section);
> -         output_pubnames (pubtype_table);
> -       }
> -    }
> +  output_pubnames (pubtype_table);
>
>   /* Output the address range information if a CU (.debug_info section)
>      was emitted.  We output an empty table even if we had no functions
>
> --
> This patch is available for review at http://codereview.appspot.com/6197069

Ping? Also, I have a couple more for debug fission that apply over
this one coming.

[Dwarf Patch] Improve pubnames and pubtypes generation. (issue6197069)

[Sterling Augustine]

The enclosed patch fixes many issues with pubnames and pubtypes. It generates
them for many more variables and with mostly correct and canonical dwarf names.

This patch should not affect any target that does not use pubnames.

The exceptions to the canonical names are addressed in a separate patch in
to the front end under review at http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00512.html.

Tested with bootstrap and running the test_pubnames_and_indices.py script
recently contributed to the GDB project.

OK for mainline?

Sterling

2012-05-10   Sterling Augustine  <saugustine@google.com>

	* dwarf2out.c (DEBUG_PUBNAMES_SECTION_LABEL,
	DEBUG_PUBTYPES_SECTION_LABEL): New macros.
	(debug_pubnames_section_label, debug_pubtypes_section_label): New
	globals.
	(is_cu_die, is_namespace_die, is_class_die, add_AT_pubnames,
	add_enumerator_pubname): New functions.
	(add_pubname): Rework logic.  Call is_class_die, is_cu_die and
	is_namespace_die.  Fix minor style violation.
	(add_pubtype): Rework logic for calculating type name.  Call
	is_namespace_die.
	(output_pubnames): Move conditional logic deciding when to produce the
	section from dwarf2out_finish.  Output debug_pubnames_section_label
	and debug_pubtypes_section_label.
	(base_type_die): Call add_pubtype.
	(gen_enumeration_type_die): Unconditionally call add_pubtype.
	(gen_namespace_die): Call add_pubname_string.
	(dwarf2out_init): Generate debug_pubnames_section_label and
	debug_pubtypes_section_label from DEBUG_PUBNAMES_SECTION_LABEL and
	DEBUG_PUBTYPES_SECTION_LABEL respectively.
	(dwarf2out_finish): Call add_AT_pubnames; Move logic on when to
	produce pubnames and pubtypes sections to output_pubnames.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 187271)
+++ gcc/dwarf2out.c	(working copy)
@@ -3007,6 +3007,7 @@ static void output_comp_unit (dw_die_ref, int);
 static void output_comdat_type_unit (comdat_type_node *);
 static const char *dwarf2_name (tree, int);
 static void add_pubname (tree, dw_die_ref);
+static void add_enumerator_pubname (const char *, dw_die_ref);
 static void add_pubname_string (const char *, dw_die_ref);
 static void add_pubtype (tree, dw_die_ref);
 static void output_pubnames (VEC (pubname_entry,gc) *);
@@ -3210,6 +3211,12 @@ static void gen_scheduled_generic_parms_dies (void
 #ifndef COLD_TEXT_SECTION_LABEL
 #define COLD_TEXT_SECTION_LABEL         "Ltext_cold"
 #endif
+#ifndef DEBUG_PUBNAMES_SECTION_LABEL
+#define DEBUG_PUBNAMES_SECTION_LABEL	    "Ldebug_pubnames"
+#endif
+#ifndef DEBUG_PUBTYPES_SECTION_LABEL
+#define DEBUG_PUBTYPES_SECTION_LABEL	    "Ldebug_pubtypes"
+#endif
 #ifndef DEBUG_LINE_SECTION_LABEL
 #define DEBUG_LINE_SECTION_LABEL	"Ldebug_line"
 #endif
@@ -3246,6 +3253,8 @@ static char cold_end_label[MAX_ARTIFICIAL_LABEL_BY
 static char abbrev_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
 static char debug_info_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
 static char debug_line_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
+static char debug_pubnames_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
+static char debug_pubtypes_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
 static char macinfo_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
 static char loc_section_label[MAX_ARTIFICIAL_LABEL_BYTES];
 static char ranges_section_label[2 * MAX_ARTIFICIAL_LABEL_BYTES];
@@ -5966,6 +5975,22 @@ is_cu_die (dw_die_ref c)
   return c && c->die_tag == DW_TAG_compile_unit;
 }
 
+/* Returns true iff C is a namespace DIE.  */
+
+static inline bool
+is_namespace_die (dw_die_ref c)
+{
+  return c && c->die_tag == DW_TAG_namespace;
+}
+
+/* Returns true iff C is a class DIE.  */
+
+static inline bool
+is_class_die (dw_die_ref c)
+{
+  return c && c->die_tag == DW_TAG_class_type;
+}
+
 static char *
 gen_internal_sym (const char *prefix)
 {
@@ -8033,6 +8058,20 @@ output_comp_unit (dw_die_ref die, int output_if_em
     }
 }
 
+/* Add the DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes attributes.  */
+
+static void
+add_AT_pubnames (dw_die_ref die)
+{
+  if (targetm.want_debug_pub_sections)
+    {
+      /* FIXME: Should use add_AT_pubnamesptr.  This works because most targets
+         don't care what the base section is.  */
+      add_AT_lineptr (die, DW_AT_GNU_pubnames, debug_pubnames_section_label);
+      add_AT_lineptr (die, DW_AT_GNU_pubtypes, debug_pubtypes_section_label);
+    }
+}
+
 /* Output a comdat type unit DIE and its children.  */
 
 static void
@@ -8116,14 +8155,32 @@ add_pubname_string (const char *str, dw_die_ref di
 static void
 add_pubname (tree decl, dw_die_ref die)
 {
-  if (targetm.want_debug_pub_sections && TREE_PUBLIC (decl))
+  if (!targetm.want_debug_pub_sections)
+    return;
+
+  if ((TREE_PUBLIC (decl) && !is_class_die (die->die_parent))
+      || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
     {
       const char *name = dwarf2_name (decl, 1);
+
       if (name)
 	add_pubname_string (name, die);
     }
 }
 
+/* Add an enumerator to the pubnames section.  */
+
+static void
+add_enumerator_pubname (const char *scope_name, dw_die_ref die)
+{
+  pubname_entry e;
+
+  gcc_assert (scope_name);
+  e.name = concat (scope_name, get_AT_string (die, DW_AT_name), NULL);
+  e.die = die;
+  VEC_safe_push (pubname_entry, gc, pubtype_table, &e);
+}
+
 /* Add a new entry to .debug_pubtypes if appropriate.  */
 
 static void
@@ -8134,37 +8191,52 @@ add_pubtype (tree decl, dw_die_ref die)
   if (!targetm.want_debug_pub_sections)
     return;
 
-  e.name = NULL;
   if ((TREE_PUBLIC (decl)
-       || is_cu_die (die->die_parent))
+       || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
       && (die->die_tag == DW_TAG_typedef || COMPLETE_TYPE_P (decl)))
     {
-      e.die = die;
-      if (TYPE_P (decl))
-	{
-	  if (TYPE_NAME (decl))
+      tree scope = NULL;
+      const char *scope_name = "";
+      const char *sep = is_cxx () ? "::" : ".";
+      const char *name;
+
+      scope = TYPE_P (decl) ? TYPE_CONTEXT (decl) : NULL;
+      if (scope && TREE_CODE (scope) == NAMESPACE_DECL)
 	    {
-	      if (TREE_CODE (TYPE_NAME (decl)) == IDENTIFIER_NODE)
-		e.name = IDENTIFIER_POINTER (TYPE_NAME (decl));
-	      else if (TREE_CODE (TYPE_NAME (decl)) == TYPE_DECL
-		       && DECL_NAME (TYPE_NAME (decl)))
-		e.name = IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (decl)));
+          scope_name = lang_hooks.dwarf_name (scope, 1);
+          if (scope_name != NULL && scope_name[0] != '\0')
+            scope_name = concat (scope_name, sep, NULL);
 	      else
-	       e.name = xstrdup ((const char *) get_AT_string (die, DW_AT_name));
-	    }
+            scope_name = "";
 	}
+
+      if (TYPE_P (decl))
+        name = type_tag (decl);
       else
-	{
-	  e.name = dwarf2_name (decl, 1);
-	  if (e.name)
-	    e.name = xstrdup (e.name);
-	}
+        name = lang_hooks.dwarf_name (decl, 1);
 
       /* If we don't have a name for the type, there's no point in adding
 	 it to the table.  */
-      if (e.name && e.name[0] != '\0')
+      if (name != NULL && name[0] != '\0')
+        {
+          e.die = die;
+          e.name = concat (scope_name, name, NULL);
 	VEC_safe_push (pubname_entry, gc, pubtype_table, &e);
     }
+
+      /* Although it might be more consistent to add the pubinfo for the
+         enumerators as their dies are created, they should only be added if the
+         enum type meets the criteria above.  So rather than re-check the parent
+         enum type whenever an enumerator die is created, just output them all
+         here.  This isn't protected by the name conditional because anoymous
+         enums don't have names.  */
+      if (die->die_tag == DW_TAG_enumeration_type)
+        {
+          dw_die_ref c;
+
+          FOR_EACH_CHILD (die, c, add_enumerator_pubname (scope_name, c));
+        }
+    }
 }
 
 /* Output the public names table used to speed up access to externally
@@ -8177,6 +8249,18 @@ output_pubnames (VEC (pubname_entry, gc) * names)
   unsigned long pubnames_length = size_of_pubnames (names);
   pubname_ref pub;
 
+  if (!targetm.want_debug_pub_sections || !info_section_emitted)
+    return;
+  if (names == pubname_table)
+    {
+      switch_to_section (debug_pubnames_section);
+      ASM_OUTPUT_LABEL (asm_out_file, debug_pubnames_section_label);
+    }
+  else
+    {
+      switch_to_section (debug_pubtypes_section);
+      ASM_OUTPUT_LABEL (asm_out_file, debug_pubtypes_section_label);
+    }
   if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
     dw2_asm_output_data (4, 0xffffffff,
       "Initial length escape value indicating 64-bit DWARF extension");
@@ -9090,6 +9174,7 @@ base_type_die (tree type)
   add_AT_unsigned (base_type_result, DW_AT_byte_size,
 		   int_size_in_bytes (type));
   add_AT_unsigned (base_type_result, DW_AT_encoding, encoding);
+  add_pubtype (type, base_type_result);
 
   return base_type_result;
 }
@@ -16176,7 +16261,6 @@ gen_enumeration_type_die (tree type, dw_die_ref co
   else
     add_AT_flag (type_die, DW_AT_declaration, 1);
 
-  if (get_AT (type_die, DW_AT_name))
     add_pubtype (type, type_die);
 
   return type_die;
@@ -18923,6 +19007,8 @@ gen_namespace_die (tree decl, dw_die_ref context_d
       add_AT_die_ref (namespace_die, DW_AT_import, origin_die);
       equate_decl_number_to_die (decl, namespace_die);
     }
+  /* Bypass dwarf2_name's check for DECL_NAMELESS.  */
+  add_pubname_string (lang_hooks.dwarf_name (decl, 1), namespace_die);
 }
 
 /* Generate Dwarf debug information for a decl described by DECL.
@@ -20585,6 +20671,10 @@ dwarf2out_init (const char *filename ATTRIBUTE_UNU
 
   ASM_GENERATE_INTERNAL_LABEL (debug_info_section_label,
 			       DEBUG_INFO_SECTION_LABEL, 0);
+  ASM_GENERATE_INTERNAL_LABEL (debug_pubnames_section_label,
+			       DEBUG_PUBNAMES_SECTION_LABEL, 0);
+  ASM_GENERATE_INTERNAL_LABEL (debug_pubtypes_section_label,
+			       DEBUG_PUBTYPES_SECTION_LABEL, 0);
   ASM_GENERATE_INTERNAL_LABEL (debug_line_section_label,
 			       DEBUG_LINE_SECTION_LABEL, 0);
   ASM_GENERATE_INTERNAL_LABEL (ranges_section_label,
@@ -22183,6 +22273,8 @@ dwarf2out_finish (const char *filename)
     }
   htab_delete (comdat_type_table);
 
+  add_AT_pubnames (comp_unit_die ());
+
   /* Output the main compilation unit if non-empty or if .debug_macinfo
      will be emitted.  */
   output_comp_unit (comp_unit_die (), debug_info_level >= DINFO_LEVEL_VERBOSE);
@@ -22206,42 +22298,12 @@ dwarf2out_finish (const char *filename)
       output_location_lists (comp_unit_die ());
     }
 
-  /* Output public names table if necessary.  */
-  if (!VEC_empty (pubname_entry, pubname_table))
-    {
-      gcc_assert (info_section_emitted);
-      switch_to_section (debug_pubnames_section);
-      output_pubnames (pubname_table);
-    }
-
-  /* Output public types table if necessary.  */
+  /* Output public names and types tables if necessary.  */
+  output_pubnames (pubname_table);
   /* ??? Only defined by DWARF3, but emitted by Darwin for DWARF2.
      It shouldn't hurt to emit it always, since pure DWARF2 consumers
      simply won't look for the section.  */
-  if (!VEC_empty (pubname_entry, pubtype_table))
-    {
-      bool empty = false;
-      
-      if (flag_eliminate_unused_debug_types)
-	{
-	  /* The pubtypes table might be emptied by pruning unused items.  */
-	  unsigned i;
-	  pubname_ref p;
-	  empty = true;
-	  FOR_EACH_VEC_ELT (pubname_entry, pubtype_table, i, p)
-	    if (p->die->die_offset != 0)
-	      {
-		empty = false;
-		break;
-	      }
-	}
-      if (!empty)
-	{
-	  gcc_assert (info_section_emitted);
-	  switch_to_section (debug_pubtypes_section);
-	  output_pubnames (pubtype_table);
-	}
-    }
+  output_pubnames (pubtype_table);
 
   /* Output the address range information if a CU (.debug_info section)
      was emitted.  We output an empty table even if we had no functions

--
This patch is available for review at http://codereview.appspot.com/6197069