[rfc] debugging anonymous unions

[rfc] debugging anonymous unions

[Richard Henderson]

At the moment, we emit a variable with no name, and an associated type:

	float foo(int i)
	{
	  union {
	    int ui;
	    float uf;
	  };
	  ui = i;
	  return uf;
	}

        .uleb128 0x7    # (DIE (0xc2) DW_TAG_variable)
        .long   0xa5    # DW_AT_type
        .byte   0x2     # DW_AT_location
        .byte   0x91    # DW_OP_fbreg
        .sleb128 -4
        .byte   0x0     # end of children of DIE 0x9c

        .uleb128 0x5    # (DIE (0xa5) DW_TAG_union_type)
        .long   0xc2    # DW_AT_sibling
        .byte   0x4     # DW_AT_byte_size
        .byte   0x1     # DW_AT_decl_file
        .byte   0x3     # DW_AT_decl_line
        .uleb128 0x6    # (DIE (0xad) DW_TAG_member)
        .ascii "ui\0"   # DW_AT_name
        .byte   0x1     # DW_AT_decl_file
        .byte   0x4     # DW_AT_decl_line
        .long   0xd5    # DW_AT_type
        .uleb128 0x6    # (DIE (0xb7) DW_TAG_member)
        .ascii "uf\0"   # DW_AT_name
        .byte   0x1     # DW_AT_decl_file
        .byte   0x5     # DW_AT_decl_line
        .long   0xcc    # DW_AT_type
        .byte   0x0     # end of children of DIE 0xa5

GDB skips this variable, so we get absolutely nothing useful to debug.

I have a moderately hacky patch below works for simple cases like this,
but doesn't work for cases like

	int foo(int x)
	{
	  union {
	    int i;
	    struct {
	      short l;
	      short h;
	    };
	  };
	  i = x;
	  return l + h;
	}

Hum.  For some reason the C++ front end doesn't like this test, giving

	zz.c:9: anonymous struct not inside named type

Why?  Anonymous structs by themselves are useless.  Nested inside of
anonymous unions they are extremely useful.  If we're going to have 
them as an extension at all...  Anyway, I digress.

Back to the main point.  How should this be represented?

The status quo is more or less an exact representation of what really
happens in the source code.  One could argue that the debugger ought to
recognize this case and handle it.

There's my patch, which tries to recognize this case and hack around
it as we're emitting debugging info.

Finally, the C++ front end has an ALIAS_DECL node, which might be 
useful in cases even more general than this.  We could promote that
to a language-independent entity and support that in the debug emitter
directly.  If I'm not mistaken, this would be even less work than my
patch, at least in the dwarf2 emitter.

Thoughts?


r~


Index: dwarf2out.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/dwarf2out.c,v
retrieving revision 1.379.2.32
diff -u -p -r1.379.2.32 dwarf2out.c
--- dwarf2out.c	25 Nov 2003 02:09:42 -0000	1.379.2.32
+++ dwarf2out.c	28 Nov 2003 21:19:19 -0000
@@ -3761,6 +3761,7 @@ static void gen_unspecified_parameters_d
 static void gen_formal_types_die (tree, dw_die_ref);
 static void gen_subprogram_die (tree, dw_die_ref);
 static void gen_variable_die (tree, dw_die_ref);
+static void gen_anonaggr_variable_die (tree, dw_die_ref);
 static void gen_label_die (tree, dw_die_ref);
 static void gen_lexical_block_die (tree, dw_die_ref, int);
 static void gen_inlined_subroutine_die (tree, dw_die_ref, int);
@@ -6794,13 +6795,10 @@ dwarf2_name (tree decl, int scope)
 /* Add a new entry to .debug_pubnames if appropriate.  */
 
 static void
-add_pubname (tree decl, dw_die_ref die)
+add_pubname_1 (tree decl, dw_die_ref die)
 {
   pubname_ref p;
 
-  if (! TREE_PUBLIC (decl))
-    return;
-
   if (pubname_table_in_use == pubname_table_allocated)
     {
       pubname_table_allocated += PUBNAME_TABLE_INCREMENT;
@@ -6816,6 +6814,13 @@ add_pubname (tree decl, dw_die_ref die)
   p->name = xstrdup (dwarf2_name (decl, 1));
 }
 
+static void
+add_pubname (tree decl, dw_die_ref die)
+{
+  if (TREE_PUBLIC (decl))
+    add_pubname_1 (decl, die);
+}
+
 /* Output the public names table used to speed up access to externally
    visible names.  For now, only generate entries for externally
    visible procedures.  */
@@ -8574,8 +8579,7 @@ loc_descriptor_from_tree (tree loc, int 
 	    add_loc_descr (&ret, new_loc_descr (DW_OP_plus, 0, 0));
 	  }
 
-	if (!addressp)
-	  indirect_p = 1;
+	indirect_p = 1;
 
 	bytepos = bitpos / BITS_PER_UNIT;
 	if (bytepos > 0)
@@ -10875,15 +10879,10 @@ gen_subprogram_die (tree decl, dw_die_re
 /* Generate a DIE to represent a declared data object.  */
 
 static void
-gen_variable_die (tree decl, dw_die_ref context_die)
+gen_variable_die_1 (tree origin, tree var, tree decl, dw_die_ref var_die,
+		    dw_die_ref old_die, dw_die_ref context_die,
+		    bool declaration)
 {
-  tree origin = decl_ultimate_origin (decl);
-  dw_die_ref var_die = new_die (DW_TAG_variable, context_die, decl);
-
-  dw_die_ref old_die = lookup_decl_die (decl);
-  int declaration = (DECL_EXTERNAL (decl)
-		     || class_scope_p (context_die));
-
   if (origin != NULL)
     add_abstract_origin_attribute (var_die, origin);
 
@@ -10902,28 +10901,28 @@ gen_variable_die (tree decl, dw_die_ref 
       add_AT_specification (var_die, old_die);
       if (DECL_NAME (decl))
 	{
-	  unsigned file_index = lookup_filename (DECL_SOURCE_FILE (decl));
+	  unsigned file_index = lookup_filename (DECL_SOURCE_FILE (var));
 
 	  if (get_AT_unsigned (old_die, DW_AT_decl_file) != file_index)
 	    add_AT_unsigned (var_die, DW_AT_decl_file, file_index);
 
 	  if (get_AT_unsigned (old_die, DW_AT_decl_line)
-	      != (unsigned) DECL_SOURCE_LINE (decl))
+	      != (unsigned) DECL_SOURCE_LINE (var))
 
 	    add_AT_unsigned (var_die, DW_AT_decl_line,
-			     DECL_SOURCE_LINE (decl));
+			     DECL_SOURCE_LINE (var));
 	}
     }
   else
     {
-      add_name_and_src_coords_attributes (var_die, decl);
-      add_type_attribute (var_die, TREE_TYPE (decl), TREE_READONLY (decl),
-			  TREE_THIS_VOLATILE (decl), context_die);
+      add_name_and_src_coords_attributes (var_die, var);
+      add_type_attribute (var_die, TREE_TYPE (var), TREE_READONLY (var),
+			  TREE_THIS_VOLATILE (var), context_die);
 
       if (TREE_PUBLIC (decl))
 	add_AT_flag (var_die, DW_AT_external, 1);
 
-      if (DECL_ARTIFICIAL (decl))
+      if (DECL_ARTIFICIAL (var))
 	add_AT_flag (var_die, DW_AT_artificial, 1);
 
       if (TREE_PROTECTED (decl))
@@ -10936,7 +10935,31 @@ gen_variable_die (tree decl, dw_die_ref 
     add_AT_flag (var_die, DW_AT_declaration, 1);
 
   if (class_scope_p (context_die) || DECL_ABSTRACT (decl))
-    equate_decl_number_to_die (decl, var_die);
+    equate_decl_number_to_die (var, var_die);
+}
+
+static void
+gen_variable_die (tree decl, dw_die_ref context_die)
+{
+  tree origin;
+  dw_die_ref var_die, old_die;
+  bool declaration;
+
+  if (DECL_NAME (decl) == NULL_TREE
+      && AGGREGATE_TYPE_P (TREE_TYPE (decl))
+      /* && lang_hooks.tree_inlining.anon_aggr_type_p (TREE_TYPE (decl)) */)
+    {
+      gen_anonaggr_variable_die (decl, context_die);
+      return;
+    }
+
+  origin = decl_ultimate_origin (decl);
+  var_die = new_die (DW_TAG_variable, context_die, decl);
+  old_die = lookup_decl_die (decl);
+  declaration = (DECL_EXTERNAL (decl) || class_scope_p (context_die));
+
+  gen_variable_die_1 (origin, decl, decl, var_die, old_die,
+		      context_die, declaration);
 
   if (! declaration && ! DECL_ABSTRACT (decl))
     {
@@ -10945,6 +10968,48 @@ gen_variable_die (tree decl, dw_die_ref 
     }
   else
     tree_add_const_value_attribute (var_die, decl);
+}
+
+static void
+gen_anonaggr_variable_die (tree decl, dw_die_ref context_die)
+{
+  tree origin, field;
+  dw_die_ref var_die, old_die;
+  bool declaration;
+  rtx base_rtl;
+
+  origin = decl_ultimate_origin (decl);
+  declaration = (DECL_EXTERNAL (decl) || class_scope_p (context_die));
+
+  /* We can only handle emitting locations for memories.  */
+  base_rtl = rtl_for_decl_location (decl);
+  if (GET_CODE (base_rtl) != MEM)
+    base_rtl = NULL;
+
+  field = TYPE_FIELDS (TREE_TYPE (decl));
+  for (; field ; field = TREE_CHAIN (field))
+    {
+      if (TREE_CODE (field) != FIELD_DECL)
+	continue;
+
+      var_die = new_die (DW_TAG_variable, context_die, field);
+      old_die = lookup_decl_die (field);
+
+      gen_variable_die_1 (origin, field, decl, var_die, old_die,
+			  context_die, declaration);
+
+      if (! declaration && ! DECL_ABSTRACT (decl))
+	{
+	  if (base_rtl)
+	    {
+	      tree t = build (COMPONENT_REF, TREE_TYPE (field), decl, field);
+	      dw_loc_descr_ref l = loc_descriptor_from_tree (t, 1);
+	      add_AT_location_description (var_die, DW_AT_location, l);
+	    }
+	  if (TREE_PUBLIC (decl))
+	    add_pubname_1 (field, var_die);
+	}
+    }
 }
 
 /* Generate a DIE to represent a label identifier.  */

Re: [rfc] debugging anonymous unions

[Jim Blandy]

In principle, having the debug info reflect the structure of the code
is a good thing.  But I can't see what it buys us in this case, over
simply emitting all the (effectively) top-level members as separate
variables, whose locations happen to overlap.  Is there anything
interesting you can think of that GDB could do only if it knew there
was an anonymous union involved?

Re: [rfc] debugging anonymous unions

[Daniel Jacobowitz]

On Sun, Nov 30, 2003 at 12:42:59PM -0800, Richard Henderson wrote:
> On Sat, Nov 29, 2003 at 09:47:28PM -0500, Jim Blandy wrote:
> > Is there anything
> > interesting you can think of that GDB could do only if it knew there
> > was an anonymous union involved?
> 
> The only sligtly interesting thing is not having to replicate a large
> location list, with small or no perturbations based on offset.  Otherwise
> it makes sense to present gdb with multiple top-level variables.

For a single location expression, sure, but if there's a location list
couldn't you reference the same one?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer