Re: [PATCH 0/2] Demangler crash handler

[Pedro Alves <palves at redhat dot com>]

On 05/10/2014 09:55 PM, Florian Weimer wrote:
> * Mark Kettenis:
> 
>> No.  It's this skind of duct-tape that will make sure that bugs in the
>> demangler won't get fixed.  Apart from removing the incentive to fix
>> the bugs, these SIGSEGV signal handlers make actually fixing the bugs
>> harder as you won't have core dumps.
> 
> I find this approach extremely odd as well.

I have to admit I'm not super keen on using signals for this either.
For one, not all bugs trigger segmentation faults.  Then stealing
a signal handler always has multi-threading considerations.  E.g.,
gdb Python code could well spawn a thread that happens to call
something that wants its own SIGSEGV handler...  Signal handlers
are per-process, not per-thread.

How about we instead add a new hook to the demangler interface,
that allows registering a callback that has the prototype of
gdb's internal_error?

 extern void internal_error (const char *file, int line, const char *, ...)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);

That's in the same spirit as bfd_set_error_handler, but for
internal errors.  The default implementation could just abort.

 /* This is a function pointer to the routine which should handle BFD
    error messages.  It is called when a BFD routine encounters an
    error for which it wants to print a message.  Going through a
    function pointer permits a program linked against BFD to intercept
    the messages and deal with them itself.  */

 bfd_error_handler_type _bfd_error_handler = _bfd_default_error_handler;

 bfd_error_handler_type
 bfd_set_error_handler (bfd_error_handler_type pnew)
 {
   bfd_error_handler_type pold;

   pold = _bfd_error_handler;
   _bfd_error_handler = pnew;
   return pold;
 }

GDB would install a hook that would end up in internal_error, or
warning, or whatever we decide is best.

As bonus, we get the file/line number that triggered the bug.

If libgcc/libstd++ don't want any sort of overhead, then we could make the
assertions be nops under #if defined(IN_LIBGCC2) || defined(IN_GLIBCPP_V3)
That is, the same conditions __cxa_demangle is under.

Then we'd add a demangle_assert macro to the demangler, similar to
gdb_assert, that calls that hook if the assertion fails.  And then
we could sprinkle the demangler with assertions.

I think that'd be easy to do, and I'd think it's much cleaner
and robust.

-- 
Pedro Alves