Re: [PATCH 0/2 v3] Demangler crash handler

[Gary Benson <gbenson at redhat dot com>]

Mark Kettenis wrote:
> > From: Gary Benson <gbenson@redhat.com>
> > Is this ok to commit?
> 
> Not for me.  You're running too much code between entering the
> SIGSEGV handler and dumping core for my taste.

I don't understand.  This is the signal handler:

  static void
  gdb_demangle_signal_handler (int signo)
  {
    static int core_dumped = 0;
  
    if (!core_dumped)
      {
        if (fork () == 0)
          dump_core ();
  
        core_dumped = 1;
      }
  
    SIGLONGJMP (gdb_demangle_jmp_buf, signo);
  }

and this is dump_core:

  void
  dump_core (void)
  {
  #ifdef HAVE_SETRLIMIT
    struct rlimit rlim = { RLIM_INFINITY, RLIM_INFINITY };
  
    setrlimit (RLIMIT_CORE, &rlim);
  #endif /* HAVE_SETRLIMIT */
  
    abort ();	/* NOTE: GDB has only three calls to abort().  */
  }

This is what happens:

  1. the signal handler is entered
  2. fork
  3. setrlimit
  4. abort

I could remove the setrlimit but presumably somebody added that to
make a successful core dump more likely.

Would you accept this patch if I changed the dump_core to abort
in gdb_demangle_signal_handler?  (and updated all the comments
about having only three calls to abort)?

> I still very much agree with Pedro; this should not be necessary.

"Should" is the operative word here.  It *should* not be necessary
because the demangler *should* never crash.  But this isn't utopia.
The demangler is code, and code has bugs.  People make mistakes.
Things are valid now that may not be valid in the future.  And GDB
should not just crash if some symbol in the inferior isn't handled
or doesn't make sense or whatever.

> Drop this.  Spend your time on fixing the actual bugs.

Are you trying to tell me you will never approve a patch that installs
a SIGSEGV handler across demangler calls?

Thanks,
Gary

-- 
http://gbenson.net/