Re: [PATCH 0/2] Demangler crash handler

[Gary Benson <gbenson@redhat.com>]

Pedro Alves wrote:
> I don't believe anyone here likes that GDB crashes.  But this
> demangler is being turned into a scarecrow, for no good reason,
> IMO.  This is not how we handle all others sorts of crashes,
> even for other libraries in the tree (libbfd, libopcodes, etc.).
> 
> Here:
> 
>   https://sourceware.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&list_id=14858&product=gdb&query_format=advanced&short_desc=crash&short_desc_type=allwordssubstr
> 
> That shows 68 open GDB bugs with "crash" in the summary.  Some of
> them are likewise crashes that make GDB not even start.  Are you
> going to propose wrapping all of GDB's modules for those?  E.g.,
> some are (and I'm sure there are more) in the dwarf reader code.
> Are we wrapping that to dump the dwarf?

Mark Kettenis raised a similar point earlier in the thread [1] and
I replied [2].  This is a straw man.  It's irrelevant whether other
subsystems of GDB also have crash bugs filed against them or not.
I'm not proposing to wrap all or even any other subsystems of GDB.
I'm proposing to wrap this one specific subsystem which regularly has
bugs filed against it, bugs which completely prevent users from being
able to debug their programs with GDB.

> Seems like only one, maybe two are still relevant demangler crashes,
> even.

This is because I fixed all the open ones before proposing the wrapper.

> And I don't see a huge number of users adding themselves to CC on
> those bugs.

Every user matters.

> Looking at Tom's example crash from gcc 61233, the crash is due to
> stack exhaustion due to seemingly infinite recursion.  Even if the
> main bug is not fixed, I'd think it'd be easy to add a recursion
> limit to the demangler, and thus prevent the crash, until whatever
> c++11 feature that's still not well developed is handled properly.

A recursion-limiter would catch 61233, and some of the others, but
certainly not all of the bugs that were filed.  One failure was
because the demangler was trying to dereference 0x3.  And OTOH only
one of the bugs the fuzzer caught was an infinite recursion, the
others look like reading the wrong entry from a union.

> I just can't/won't believe there are that many unindentified
> bugs in the demangler.  If that were true, we'd hear _much_ more
> about them.

We _are_ hearing about them.  There's not been much traffic here, on
this list, because Keith has been quietly triaging every segfault and
I've been quietly fixing the bugs.  That all happens in GCC land.

> It's clear to me that we need to do something
> demangler-development-process-wise to prevent these sorts of bugs
> from manifesting only late in GDB, and I think we should definitely
> get in contact with the GCC folks about it.

Sure, but what do we say?  "Hey, you guys, it'd be great if you all
did some more testing on your code."  I can imagine the response.  I
don't have the expertise to write the kind of tests that are needed.

> I had another idea to aid demangler testing with g++.
> I had suggested before to run the demangler through the binaries
> produced by g++'s testsuite.  The whole point here is to validate
> the mangling as close as possible to the generator.
> The alternative idea is to teach g++ itself to try to demangle any
> symbol it generates/mangles (off by default, under a --param switch
> or some such).  A sort of making-it-very-easy-to-eat-your-own-dog-
> food methodology.
> We could then just run g++'s testsuite with that enabled.  No other
> infrastructure would be necessary.  And, we could ask g++
> maintainers to do that once in a while, and perhaps run it through
> whatever usual set of complicated C++ codebases they usually test
> g++ with.

That is a good idea (and something I could do myself) but you of all
people know I'm not free to spend the time it would take, so it will
remain a good idea indefinitely unless someone steps up to do the
work.

In any event, whatever future testing and/or bugfixing is performed,
the crash catcher still has merit.  I never plan on crashing my car,
but I still wear a seatbelt.

Cheers,
Gary

-- 
[1] https://sourceware.org/ml/gdb-patches/2014-05/msg00129.html
[2] https://sourceware.org/ml/gdb-patches/2014-05/msg00152.html