Re: [RFC] Special casing dtors?

Keith Seitz
2009-04-01T23:48:42Z

[-- Attachment #1: Type: text/plain, Size: 4674 bytes --]

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.

[-- Attachment #2: ignore-__base_dtor-ctor.patch --]
[-- Type: text/plain, Size: 745 bytes --]

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);