Re: [RFC] Special casing dtors?

[Keith Seitz <keiths at redhat dot com>]

I apologize ahead of time for the lengthy treatise here... I am not 
trying to be argumentative or anything, I just want to explain as best 
and fully as I can what I've discovered. Thank you to anyone that 
actually reads all of this. :-)

Pedro Alves wrote:

> I mentioned that these tests pass on *4.2*, and that you see them
> failing on 4.3.  It isn't much of a surprise that they fail on
> gcc head.

Actually, they PASS on gcc HEAD, too. At least, they did here. O:-)

> I just tried the templates.exp test with both the top of the gcc 4.2
> branch, and with current gcc head, both on stabs, x86_64.  Here's the
> result difference:
[big snip]

I have run with gcc 4.2.0, 4.2.4, and gcc 4.3.2 releases (well, F10 
w/4.3.2), and I was finally able to reproduce the failures you mention 
with -gstabs+ (on the 4.2.x compilers). After digging into this for 
quite some time, I am convinced this happens not because of my patch, 
but because stabs is just really broken in gdb.

For example:
Consider the much simpler case of setting a breakpoint on a ctor/dtor 
(non-template) -- for which there is not ONE single test in gdb's testsuite:

$ cat simple.cc
#include <iostream>

using namespace std;

class aclass
{
 public:
   aclass (void) { cout << "ctor" << endl; };
   ~aclass (void) { cout << "dtor" << endl; };
};

int
main (int argc, char *argv[])
{
  aclass* ac = new aclass();
  delete ac;
  return 0;
}

Compile this with gcc-4.2.4 with -gstabs+ (or DWARF, it doesn't matter) 
and run this under FSF gdb CVS HEAD (from a few days ago) :

(gdb) break aclass::~aclass
the class `aclass' does not have destructor defined
Hint: try 'aclass::~aclass<TAB> or 'aclass::~aclass<ESC-?>
(Note leading single quote.)

Run this under my proposed patch:
(gdb) break aclass::~aclass
Breakpoint 1 at 0x80489fe: file simple.cc, line 9.

[Anecdote: With the templates.exp dtor test, while gdb is able to set a 
breakpoint, it never actually hits it. At all. It does with my patch.]

So clearly, things are as complicated as we've discovered. The actual 
templates.exp tests which are/are not failing are template ctor and dtor 
tests -- ones I could also not reduce to simpler test cases. So I had a 
look specifically at templates.cc -- it has been over a decade since I 
looked at stabs output! -- and I discovered that the stabs info DOES 
define TWO ctors and TWO dtors.

So I stepped the patched and unpatched gdbs side-by-side until I found 
something different. While I claim neither to understand everything 
behind this nor why fixing the difference I did find caused such a 
massive positive change in test results, it is clear that something 
changed in gcc somewhere, and the corresponding change never got to gdb.

I've attached a secondary patch for stabsread.c which not only fixes the 
failing tests you found (with -gstabs+), but also fixes tons of other 
failures in classes.exp and virtfunc.exp (and more). Again, don't ask me 
why. The fix seems innocently isolated enough, but it just reduces the 
number of failed tests from >400 to just over 300.

Can you apply the attached patch to your tree and try again?

> I don't know how much we care for stabs on 4.2.  Certainly on
> linux it isn't that important, but other platforms are still a
> bit stuck with it.  Note that I'm not objecting to you applying
> your patch.  The failure mode it introduces on stabs isn't that
> critical.

I think that what is simply happening here is that there is some severe 
stabs breakage in gdb (and no one noticed/tested/complained?). It just 
so happens that some of these failures were being hidden by what I 
suspect is very old code. By removing the old code, the (or a) real 
problem was exposed. I have no doubt that there are many more 
undiscovered bugs w.r.t. C++ and stabs.

> I was merely trying to be helpful, and I didn't
> expect that we'd find this.  In any case, it is always good to
> try out several compilers, not just the tip of the trunk.  GDB
> has to cope with code produced from older compilers as well.

I certainly understand (and appreciate) that you are trying to be 
helpful -- and you are! I never felt otherwise.

So, I guess my position is that since stabs is so darn broken any way, 
we might as well apply the patch and deal with the fallout. At least 
then we'll know (and hopefully document) WHY all this special casing of 
dtors is necessary.

Keith

PS. I still don't understand how you are getting DWARF2 failures. I've 
run these tests on gcc 4.2.0, 4.2.4, 4.3.2, and 4.4.0, and NONE of those 
compilers demonstrate any failures after the patch; they all fail BEFORE 
the patch is applied. I guess I'll deal with this next.
Index: stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.113
diff -u -p -r1.113 stabsread.c
--- stabsread.c	21 Feb 2009 16:14:49 -0000	1.113
+++ stabsread.c	1 Apr 2009 23:03:22 -0000
@@ -2408,8 +2408,8 @@ read_member_functions (struct field_info
 
       /* Skip GCC 3.X member functions which are duplicates of the callable
 	 constructor/destructor.  */
-      if (strcmp (main_fn_name, "__base_ctor") == 0
-	  || strcmp (main_fn_name, "__base_dtor") == 0
+      if (strcmp_iw (main_fn_name, "__base_ctor ") == 0
+	  || strcmp_iw (main_fn_name, "__base_dtor ") == 0
 	  || strcmp (main_fn_name, "__deleting_dtor") == 0)
 	{
 	  xfree (main_fn_name);