Re: [PATCH] PR debug/45682 - wrong struct DIE nesting with -fdebug-types-section

[Cary Coutant <ccoutant at google dot com>]

> I think the real problem is that the skeleton declaration DIE for
> class Executor is being inserted under the wrong DIE -- probably in
> remove_child_or_replace_with_skeleton().

The problem is that we're inserting the skeleton DIE for class
Executor in place of the original specification DIE at the top level
under the compile_unit DIE. The original had a DW_AT_specification
pointing to a declaration DIE under namespace thread, but the skeleton
is a declaration, so it needs to be directly under the namespace DIE.
The original declaration DIE gets pruned because there are no
references left that point to it.

The patch below fixes that by noting that the DIE we're replacing had
a specification DIE, and when it's time to replace it with the
skeleton DIE, we need to insert it under the original declaration's
parent.

Take a look at this and see if it makes sense; if it does, I'll add a
ChangeLog and a testcase on Monday, bootstrap, test, and submit. I did
verify that gdb can read the resulting debug info for the test case in
the original PR.

-cary


diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 695b7b1..f1632ea 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3307,11 +3307,12 @@ static int should_move_die_to_comdat (dw_die_ref);
 static dw_die_ref clone_as_declaration (dw_die_ref);
 static dw_die_ref clone_die (dw_die_ref);
 static dw_die_ref clone_tree (dw_die_ref);
-static void copy_declaration_context (dw_die_ref, dw_die_ref);
+static dw_die_ref copy_declaration_context (dw_die_ref, dw_die_ref);
 static void generate_skeleton_ancestor_tree (skeleton_chain_node *);
 static void generate_skeleton_bottom_up (skeleton_chain_node *);
 static dw_die_ref generate_skeleton (dw_die_ref);
 static dw_die_ref remove_child_or_replace_with_skeleton (dw_die_ref,
+                                                         dw_die_ref,
                                                          dw_die_ref);
 static void break_out_comdat_types (dw_die_ref);
 static dw_die_ref copy_ancestor_tree (dw_die_ref, dw_die_ref, htab_t);
@@ -7160,11 +7161,12 @@ clone_as_declaration (dw_die_ref die)
    AT_specification attribute, it also includes attributes and children
    attached to the specification.  */

-static void
+static dw_die_ref
 copy_declaration_context (dw_die_ref unit, dw_die_ref die)
 {
   dw_die_ref decl;
   dw_die_ref new_decl;
+  dw_die_ref new_parent = NULL;

   decl = get_AT_ref (die, DW_AT_specification);
   if (decl == NULL)
@@ -7175,6 +7177,10 @@ copy_declaration_context (dw_die_ref unit,
dw_die_ref die)
       dw_die_ref c;
       dw_attr_ref a;

+      /* The original DIE will be changed to a declaration, and must
+         be moved to be a child of the original declaration DIE.  */
+      new_parent = decl->die_parent;
+
       /* Copy the type node pointer from the new DIE to the original
          declaration DIE so we can forward references later.  */
       decl->die_id.die_type_node = die->die_id.die_type_node;
@@ -7203,6 +7209,8 @@ copy_declaration_context (dw_die_ref unit, dw_die_ref die)
           add_AT_specification (die, new_decl);
         }
     }
+
+  return new_parent;
 }

 /* Generate the skeleton ancestor tree for the given NODE, then clone
@@ -7286,7 +7294,7 @@ generate_skeleton (dw_die_ref die)
   return node.new_die;
 }

-/* Remove the DIE from its parent, possibly replacing it with a cloned
+/* Remove the CHILD DIE from its parent, possibly replacing it with a cloned
    declaration.  The original DIE will be moved to a new compile unit
    so that existing references to it follow it to the new location.  If
    any of the original DIE's descendants is a declaration, we need to
@@ -7294,7 +7302,8 @@ generate_skeleton (dw_die_ref die)
    declarations back into the skeleton tree.  */

 static dw_die_ref
-remove_child_or_replace_with_skeleton (dw_die_ref child, dw_die_ref prev)
+remove_child_or_replace_with_skeleton (dw_die_ref child, dw_die_ref prev,
+				       dw_die_ref new_parent)
 {
   dw_die_ref skeleton;

@@ -7304,7 +7313,16 @@ remove_child_or_replace_with_skeleton
(dw_die_ref child, dw_die_ref prev)
   else
     {
       skeleton->die_id.die_type_node = child->die_id.die_type_node;
-      replace_child (child, skeleton, prev);
+
+      /* If the original DIE was a specification, we need to put
+         the skeleton under the parent DIE of the declaration.  */
+      if (new_parent != NULL)
+	{
+	  remove_child_with_prev (child, prev);
+	  add_child_die (new_parent, skeleton);
+	}
+      else
+	replace_child (child, skeleton, prev);
     }

   return skeleton;
@@ -7332,7 +7350,7 @@ break_out_comdat_types (dw_die_ref die)
     next = (c == first ? NULL : c->die_sib);
     if (should_move_die_to_comdat (c))
       {
-        dw_die_ref replacement;
+        dw_die_ref replacement, new_parent;
 	comdat_type_node_ref type_node;

         /* Create a new type unit DIE as the root for the new tree, and
@@ -7350,10 +7368,11 @@ break_out_comdat_types (dw_die_ref die)

         /* Copy the declaration context, attributes, and children of the
            declaration into the new compile unit DIE.  */
-	copy_declaration_context (unit, c);
+	new_parent = copy_declaration_context (unit, c);

         /* Remove this DIE from the main CU.  */
-	replacement = remove_child_or_replace_with_skeleton (c, prev);
+	replacement = remove_child_or_replace_with_skeleton (c, prev,
+							     new_parent);

         /* Break out nested types into their own type units.  */
         break_out_comdat_types (c);
@@ -7705,7 +7724,7 @@ size_of_die (dw_die_ref die)
   unsigned long size = 0;
   dw_attr_ref a;
   unsigned ix;
-  dwarf_form form;
+  enum dwarf_form form;

   size += size_of_uleb128 (die->die_abbrev);
   FOR_EACH_VEC_ELT (dw_attr_node, die->die_attr, ix, a)