diff mbox

Kill TYPE_METHODS debug 1/9

Message ID a8067467-a452-efe1-751d-5f86882d8a06@acm.org
State New
Headers show

Commit Message

Nathan Sidwell July 14, 2017, 4:48 p.m. UTC
This changes dbxout and dwarf2out.

Rather than iterate over the TYPE_METHODS, they now need to deal with member fns 
in the regular TYPE_FIELDS iteration.
dbxout was a little weirdly convoluted, apparently presuming that functions with 
the same name are all together.  That's not true, so other than maybe slight 
debug bloat in cases when they happen to be adjacent, it seems more sensible to 
handle each member function as a separate item.

The dwarf2out changes are just moving the processing to the TYPE_FIELDS loop, 
and thereby deleting some duplicate code.

I'd appreciate a review of this patch.

Oh, the patch series survived a bootstrap on x86_64-linux.

nathan

Comments

Jim Wilson July 18, 2017, 5:24 p.m. UTC | #1
On 07/14/2017 09:48 AM, Nathan Sidwell wrote:
> This changes dbxout and dwarf2out.

> Oh, the patch series survived a bootstrap on x86_64-linux.

Changes to the debug info files requires a gdb make check with and 
without the patch to check for regressions.  Since you are changing both 
dbxout and dwarf2out, you would need to do this twice, once for each 
debug info type.  Testing dbxout may be a little tricky, since few 
systems still use this by default.  Maybe you can hack an x86_64-linux 
build to use dbxout by default to test it.

Otherwise, this looks OK.

Jim
Nathan Sidwell July 20, 2017, 11:31 a.m. UTC | #2
On 07/18/2017 01:24 PM, Jim Wilson wrote:

> Changes to the debug info files requires a gdb make check with and 
> without the patch to check for regressions.  Since you are changing both 
> dbxout and dwarf2out, you would need to do this twice, once for each 
> debug info type.  Testing dbxout may be a little tricky, since few 
> systems still use this by default.  Maybe you can hack an x86_64-linux 
> build to use dbxout by default to test it.

DWARF results are unchanged.

DBX results after hacking x86-64 to prefer stabs are too horrible to 
tell.  However, a manual check on a simple program shows that gdb 
couldn't locate member functions anyway even before the patch and an 
unhacked g++.

g++ -gstabs:
(gdb) ptype X
type = struct X {
     int m;
}

g++ -gdwarf:
(gdb) ptype X
type = struct X {
     int m;
   public:
     int frob(int);
     X(int);
}

So I don't think I've made it worse there.   thoughts?

nathan
Nathan Sidwell July 20, 2017, 12:13 p.m. UTC | #3
On 07/20/2017 07:31 AM, Nathan Sidwell wrote:

> So I don't think I've made it worse there.   thoughts?

aha, gstabs+ for gnu extensions. With a small fix I get the following 
before and after (I changed the example to add another frob member).
(gdb) ptype X
type = struct X {
   public:
     int frob(int);
     int frob(float);
     X(int);
     X(int);
}

The fragment I'd missed is losing:
   /* Also ignore abstract methods; those are only interesting to
      the DWARF backends.  */
   if (DECL_IGNORED_P (decl) || DECL_ABSTRACT_P (decl))
     return;

which inhibits the abstract ctor.

nathan
diff mbox

Patch

Index: gcc/dbxout.c
===================================================================
--- gcc/dbxout.c	(revision 250160)
+++ gcc/dbxout.c	(working copy)
@@ -311,8 +311,7 @@  static void dbxout_typedefs (tree);
 static void dbxout_type_index (tree);
 static void dbxout_args (tree);
 static void dbxout_type_fields (tree);
-static void dbxout_type_method_1 (tree);
-static void dbxout_type_methods (tree);
+static void dbxout_type_fn_member (tree);
 static void dbxout_range_type (tree, tree, tree);
 static void dbxout_type (tree, int);
 static bool print_int_cst_bounds_in_octal_p (tree, tree, tree);
@@ -1493,6 +1492,8 @@  dbxout_type_fields (tree type)
 		  || ! tree_fits_uhwi_p (DECL_SIZE (tem)))))
 	continue;
 
+      else if (TREE_CODE (tem) == FUNCTION_DECL)
+	dbxout_type_fn_member (tem);
       else if (TREE_CODE (tem) != CONST_DECL)
 	{
 	  /* Continue the line if necessary,
@@ -1542,14 +1543,23 @@  dbxout_type_fields (tree type)
     }
 }
 
-/* Subroutine of `dbxout_type_methods'.  Output debug info about the
-   method described DECL.  */
+/* Subroutine of `dbxout_type'.  Output debug info about the
+   function member DECL.  */
 
 static void
-dbxout_type_method_1 (tree decl)
+dbxout_type_fn_member (tree decl)
 {
+  if (!use_gnu_debug_info_extensions)
+    return;
+
   char c1 = 'A', c2;
 
+  CONTIN;
+  stabstr_I (DECL_NAME (decl));
+  stabstr_S ("::");
+
+  dbxout_type (TREE_TYPE (decl), 0);
+
   if (TREE_CODE (TREE_TYPE (decl)) == FUNCTION_TYPE)
     c2 = '?';
   else /* it's a METHOD_TYPE.  */
@@ -1586,72 +1596,7 @@  dbxout_type_method_1 (tree decl)
       dbxout_type (DECL_CONTEXT (decl), 0);
       stabstr_C (';');
     }
-}
-
-/* Subroutine of `dbxout_type'.  Output debug info about the methods defined
-   in TYPE.  */
-
-static void
-dbxout_type_methods (tree type)
-{
-  /* C++: put out the method names and their parameter lists */
-  tree methods = TYPE_METHODS (type);
-  tree fndecl;
-  tree last;
-
-  if (methods == NULL_TREE)
-    return;
-
-  if (TREE_CODE (methods) != TREE_VEC)
-    fndecl = methods;
-  else if (TREE_VEC_ELT (methods, 0) != NULL_TREE)
-    fndecl = TREE_VEC_ELT (methods, 0);
-  else
-    fndecl = TREE_VEC_ELT (methods, 1);
-
-  while (fndecl)
-    {
-      int need_prefix = 1;
-
-      /* Group together all the methods for the same operation.
-	 These differ in the types of the arguments.  */
-      for (last = NULL_TREE;
-	   fndecl && (last == NULL_TREE || DECL_NAME (fndecl) == DECL_NAME (last));
-	   fndecl = DECL_CHAIN (fndecl))
-	/* Output the name of the field (after overloading), as
-	   well as the name of the field before overloading, along
-	   with its parameter list */
-	{
-	  /* Skip methods that aren't FUNCTION_DECLs.  (In C++, these
-	     include TEMPLATE_DECLs.)  The debugger doesn't know what
-	     to do with such entities anyhow.  */
-	  if (TREE_CODE (fndecl) != FUNCTION_DECL)
-	    continue;
-
-	  CONTIN;
-
-	  last = fndecl;
-
-	  /* Also ignore abstract methods; those are only interesting to
-	     the DWARF backends.  */
-	  if (DECL_IGNORED_P (fndecl) || DECL_ABSTRACT_P (fndecl))
-	    continue;
-
-	  /* Redundantly output the plain name, since that's what gdb
-	     expects.  */
-	  if (need_prefix)
-	    {
-	      stabstr_I (DECL_NAME (fndecl));
-	      stabstr_S ("::");
-	      need_prefix = 0;
-	    }
-
-	  dbxout_type (TREE_TYPE (fndecl), 0);
-	  dbxout_type_method_1 (fndecl);
-	}
-      if (!need_prefix)
-	stabstr_C (';');
-    }
+  stabstr_C (';');
 }
 
 /* Emit a "range" type specification, which has the form:
@@ -2211,10 +2156,6 @@  dbxout_type (tree type, int full)
 
       /* Write out the field declarations.  */
       dbxout_type_fields (type);
-      if (use_gnu_debug_info_extensions && TYPE_METHODS (type) != NULL_TREE)
-	{
-	  dbxout_type_methods (type);
-	}
 
       stabstr_C (';');
 
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 250160)
+++ gcc/dwarf2out.c	(working copy)
@@ -24088,7 +24088,8 @@  gen_member_die (tree type, dw_die_ref co
 {
   tree member;
   tree binfo = TYPE_BINFO (type);
-  dw_die_ref child;
+
+  gcc_assert (TYPE_MAIN_VARIANT (type) == type);
 
   /* If this is not an incomplete type, output descriptions of each of its
      members. Note that as we output the DIEs necessary to represent the
@@ -24125,13 +24126,16 @@  gen_member_die (tree type, dw_die_ref co
 	   && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline)
 	       != -1));
 
+      /* Ignore clones.  */
+      if (DECL_ABSTRACT_ORIGIN (member))
+	continue;
+
       /* If we thought we were generating minimal debug info for TYPE
 	 and then changed our minds, some of the member declarations
 	 may have already been defined.  Don't define them again, but
 	 do put them in the right order.  */
 
-      child = lookup_decl_die (member);
-      if (child)
+      if (dw_die_ref child = lookup_decl_die (member))
 	{
 	  /* Handle inline static data members, which only have in-class
 	     declarations.  */
@@ -24159,6 +24163,7 @@  gen_member_die (tree type, dw_die_ref co
 		  static_inline_p = false;
 		}
 	    }
+
 	  if (child->die_tag == DW_TAG_variable
 	      && child->die_parent == comp_unit_die ()
 	      && ref == NULL)
@@ -24197,23 +24202,6 @@  gen_member_die (tree type, dw_die_ref co
 	  DECL_EXTERNAL (member) = old_extern;
 	}
     }
-
-  /* We do not keep type methods in type variants.  */
-  gcc_assert (TYPE_MAIN_VARIANT (type) == type);
-  /* Now output info about the function members (if any).  */
-  if (TYPE_METHODS (type) != error_mark_node)
-    for (member = TYPE_METHODS (type); member; member = DECL_CHAIN (member))
-      {
-	/* Don't include clones in the member list.  */
-	if (DECL_ABSTRACT_ORIGIN (member))
-	  continue;
-
-	child = lookup_decl_die (member);
-	if (child)
-	  splice_child_die (context_die, child);
-	else
-	  gen_decl_die (member, NULL, NULL, context_die);
-      }
 }
 
 /* Generate a DIE for a structure or union type.  If TYPE_DECL_SUPPRESS_DEBUG