diff mbox

Kill TYPE_METHODS debug 1/9

Message ID e48f02e0-6b29-e339-621a-72047e16db11@acm.org
State New
Headers show

Commit Message

Nathan Sidwell July 20, 2017, 9 p.m. UTC
A bit more poking showed that dbx's desire to put all methods of the 
same name in a single record was significant -- gdb requires that to 
build an overload set.

So, this version of the dbxout changes is much smaller, in that we 
iterate the TYPE_FIELDS twice.  There's the original scan, which just 
needs to now skip FUNCTION_DECLs as well.  The method emission routine 
now iterates over TYPE_FIELDS again, skipping everything that is not a 
FUNCTION_DECL.  I preserve the previous behaviour of merging decls with 
the same name -- of course it'll still fail in the same manner if you 
declare overloads discontigously.

With this patch the gdb stabs test results are still awful, but they are 
unchanged awfulness.

nathan

Comments

Jim Wilson July 20, 2017, 10:03 p.m. UTC | #1
On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
> With this patch the gdb stabs test results are still awful, but they are
> unchanged awfulness.

Yes, the stabs support for C++ is poor.  That is one of the reasons
why almost everyone has switched to dwarf2.

I wasn't sure what to make of your last message, so I tried to see if
I could build a toolchain that defaults to stabs so I could look at
this.  I discovered that -freorder-functions doesn't work with stabs
on an elf target, as we get a cross section label subtraction that the
assembler can't compute.  I had to manually disable that optimization.
I also had to disable the x86 specific change to enable
-freorder-blocks-and-partition at -O2.  I managed to get it working
for x86_64-linux and i686-pc-cygwin.  But the fact that -O2 and stabs
don't work together by default anymore suggest that there are no
serious stabs users except on old legacy systems that don't support
named sections, and hence don't support -freorder-functions.

I also noticed that there is a config/i386/gstabs.h file that has been
unused since the openbsd 2 and 3 support was removed last year, and
should be deleted.

Anyways, your new dbxout.c patch looks good.  And maybe we should
think about deprecating the stabs support some day.

Jim
Nathan Sidwell July 21, 2017, 12:29 a.m. UTC | #2
On 07/20/2017 06:03 PM, Jim Wilson wrote:

> I wasn't sure what to make of your last message, so I tried to see if
> I could build a toolchain that defaults to stabs so I could look at
> this.  I discovered that -freorder-functions doesn't work with stabs
> on an elf target, as we get a cross section label subtraction that the
> assembler can't compute.  I had to manually disable that optimization.
> I also had to disable the x86 specific change to enable
> -freorder-blocks-and-partition at -O2.  I managed to get it working
> for x86_64-linux and i686-pc-cygwin.  But the fact that -O2 and stabs
> don't work together by default anymore suggest that there are no
> serious stabs users except on old legacy systems that don't support
> named sections, and hence don't support -freorder-functions.

Sorry if I was unclear.  Indeed a complete build failed building libraries as 
you describe.  But for testing purposes I copied stabs-preferring xg++/xgcc and 
cc1/cc1plus to an dwarf install and pointed gdb at that.  Plus some manual testing.

> Anyways, your new dbxout.c patch looks good.  And maybe we should
> think about deprecating the stabs support some day.

agreed.

I have committed the patch series, thanks all for comments.

nathan
Richard Biener July 21, 2017, 5:11 a.m. UTC | #3
On July 21, 2017 12:03:58 AM GMT+02:00, Jim Wilson <jim.wilson@linaro.org> wrote:
>On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> With this patch the gdb stabs test results are still awful, but they
>are
>> unchanged awfulness.
>
>Yes, the stabs support for C++ is poor.  That is one of the reasons
>why almost everyone has switched to dwarf2.
>
>I wasn't sure what to make of your last message, so I tried to see if
>I could build a toolchain that defaults to stabs so I could look at
>this.  I discovered that -freorder-functions doesn't work with stabs
>on an elf target, as we get a cross section label subtraction that the
>assembler can't compute.  I had to manually disable that optimization.
>I also had to disable the x86 specific change to enable
>-freorder-blocks-and-partition at -O2.  I managed to get it working
>for x86_64-linux and i686-pc-cygwin.  But the fact that -O2 and stabs
>don't work together by default anymore suggest that there are no
>serious stabs users except on old legacy systems that don't support
>named sections, and hence don't support -freorder-functions.
>
>I also noticed that there is a config/i386/gstabs.h file that has been
>unused since the openbsd 2 and 3 support was removed last year, and
>should be deleted.
>
>Anyways, your new dbxout.c patch looks good.  And maybe we should
>think about deprecating the stabs support some day.

I've suggested that multiple times also to be able to get rid of the debug hook interfacing in GCC and emit dwarf directly from FEs where that makes sense.  DWARF should be a superset of other debug formats so it should be possible to build stabs from dwarf.

Richard.

>Jim
Jeff Law July 24, 2017, 10:18 p.m. UTC | #4
On 07/20/2017 04:03 PM, Jim Wilson wrote:
> On Thu, Jul 20, 2017 at 2:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> With this patch the gdb stabs test results are still awful, but they are
>> unchanged awfulness.
> 
> Yes, the stabs support for C++ is poor.  That is one of the reasons
> why almost everyone has switched to dwarf2.
> 
> I wasn't sure what to make of your last message, so I tried to see if
> I could build a toolchain that defaults to stabs so I could look at
> this.  I discovered that -freorder-functions doesn't work with stabs
> on an elf target, 
Hmm, it's supposed to if we support named sections.  But clearly the
code has bit-rotted through the years.  One more strike against stabs ;-)


> 
> I also noticed that there is a config/i386/gstabs.h file that has been
> unused since the openbsd 2 and 3 support was removed last year, and
> should be deleted.
Pre-approved :-)

> 
> Anyways, your new dbxout.c patch looks good.  And maybe we should
> think about deprecating the stabs support some day.
Agreed.  I think the biggest concern is AIX.

jeff
diff mbox

Patch

2017-07-20  Nathan Sidwell  <nathan@acm.org>

	* dbxout.c (dbxout_type_fields): Ignore FUNCTION_DECLs.
	(dbxout_type_methods): Scan TYPE_FIELDS.
	(dbxout_type): Don't check TYPE_METHODS here.

Index: dbxout.c
===================================================================
--- dbxout.c	(revision 250318)
+++ dbxout.c	(working copy)
@@ -1481,6 +1481,8 @@  dbxout_type_fields (tree type)
       /* Omit here local type decls until we know how to support them.  */
       if (TREE_CODE (tem) == TYPE_DECL
 	  || TREE_CODE (tem) == TEMPLATE_DECL
+	  /* Member functions emitted after fields.  */
+	  || TREE_CODE (tem) == FUNCTION_DECL
 	  /* Omit here the nameless fields that are used to skip bits.  */
 	  || DECL_IGNORED_P (tem)
 	  /* Omit fields whose position or size are variable or too large to
@@ -1586,55 +1588,38 @@  dbxout_type_method_1 (tree decl)
     }
 }
 
-/* Subroutine of `dbxout_type'.  Output debug info about the methods defined
-   in TYPE.  */
+/* Subroutine of `dbxout_type'.  Output debug info about the member
+   functions 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)
+  for (tree fndecl = TYPE_FIELDS (type); 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;
+      for (tree 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.  */
+	  /* Skip non-functions.  */
 	  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;
 
+	  CONTIN;
+
+	  last = fndecl;
+
 	  /* Redundantly output the plain name, since that's what gdb
 	     expects.  */
 	  if (need_prefix)
@@ -2209,10 +2194,8 @@  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);
-	}
+      if (use_gnu_debug_info_extensions)
+	dbxout_type_methods (type);
 
       stabstr_C (';');