Re: [patch] Fix dwarf2read to crash again

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Therefore the patch of mine [readerror] turned CRASH->FAIL in some
Jan> cases. The FAIL case is difficult to understand and occurs only in
Jan> very rare C++ cases (one can -readnow libwebkit.so) so it could
Jan> hide for a long time the [delay] bug confusing C++ debugging.

It took me a while to really understand this rationale, but I get it
now.  For us as gdb developers, a crash is preferable because it is
simpler to notice.

I think, however, that the opposite is true for users.  I was talking
with Graydon recently about a different GDB dwarf reader bug he had run
into: if tag_type_to_type fails, then an entire objfile's worth of
debuginfo can be dropped, leading to weird behavior.  This is pretty
unfriendly when the type DIE in question is in the vendor range.

My conclusion from these cases is that gdb ought to fail gracefully.  I
suspect we should even go so far as to wrap process_die (or possibly
some more appropriate function) in a TRY_CATCH and skip DIEs that
confuse gdb.

For the testing case, I think the best thing would be to add a complaint
anywhere we encounter invalid DWARF.  Then, we can enable complaints
when testing -- either in the GDB test suite or whatever other QA
anybody does.

On the QA topic: the particular bug in question was found in a deployed
Fedora system.  We've been discussing an automated debuginfo smoke test,
consisting of running "gdb -readnow" on all the debuginfo in the distro.

The above is why I think this patch should no go in as-is.
I am interested in reactions to the above.

Tom