Re: [PATCH 0/2] Demangler crash handler

[Tom Tromey <tromey at redhat dot com>]

Stan> So it's a little strange to me that it's now become so troublesome
Stan> that it needs to be wrapped, or has been suggested, to be run in a
Stan> different process(!), and it reinforces Mark K's original point
Stan> about signal catchers masking more serious problems.

The typical failure mode here is that gdb crashes when you start it.
You cannot debug your program, and there is no reasonable workaround.
(There is an unreasonable one: edit your binary to remove all mentions
of the offending symbol.)

What problem might be masked that is more serious than this?

Stan> Complicated or not, the demangler is one of the most algorithmically
Stan> predictable components of GDB, and it is very easy to test
Stan> comprehensively; no races, no arcane target dependencies, textual
Stan> input and output.  So if it's becoming unreliable, perhaps there are
Stan> process flaws that we should be addressing.

I agree, that would be a good thing to do.

However, even if the process flaws are fixed, it remains that gdb is
unusually exposed to demangler bugs, and that the patch in question is
not likely to cause harm.  We know this because this technique is
commonplace in the managed runtime community.

And, supposing that the demangler is thoroughly de-bugged -- the patch
in question will still not cause any harm.

That is, addressing the process is not exclusive with this patch.


I realize there is an attractive quality to the "suffering is good for
us" idea.  It appeals to my inner spartan.  However, I disagree very
strongly with this.

As a thought experiment, consider applying this same logic to
internal_error.  The same considerations apply to making it simply
abort.  Yet, it would be a mistake to make this change -- because in the
end, a user's debug session is more important than any particular gdb
bug.


BTW you can try it out thanks to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61233

Compile this with g++ and try it with a recent gdb.

    extern "C"
    void _Z7ZipWithI7QStringS0_5QListZN4oral6detail16AdaptCreateTableI7AccountEES0_RKNS3_16CachedFieldsDataEEUlRKS0_SA_E_ET1_IDTclfp1_cvT__EcvT0__EEEERKT1_ISC_ERKT1_ISD_ET2_ ()
    {
    }

    int main()
    {
      return 0;
    }

Ouch:

    barimba. ./gdb/gdb --quiet /tmp/q
    Reading symbols from /tmp/q...Segmentation fault (core dumped)

Tom