Re: [PATCH 3/3 v4] Demangler crash handler

[Gary Benson <gbenson at redhat dot com>]

Andrew Burgess wrote:
> On 05/06/2014 2:03 PM, Gary Benson wrote:
> > diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> > index 91533e8..f4dde70 100644
> > --- a/gdb/cp-support.c
> > +++ b/gdb/cp-support.c
> 
> > +
> > +/* Signal handler for gdb_demangle.  */
> > +
> > +static void
> > +gdb_demangle_signal_handler (int signo)
> > +{
> > +  if (gdb_demangle_attempt_core_dump)
> > +    {
> > +      if (fork () == 0)
> > +	dump_core ();
> 
> This worries me a little, when a problem case occurs gdb will dump
> core regardless of the users ulimit setting, without first asking
> the user, and doesn't tell the user that a core file was created.
> 
> This feels quite unexpected behaviour to me, especially the bit
> about disregarding the ulimit setting without first asking for
> permission.
> 
> Catching the crash feels like a good idea, but I'd prefer that gdb
> ask before circumventing the ulimit and dumping core.

This part of the same patch:

+  if (core_dump_allowed == -1)
+    {
+      core_dump_allowed = can_dump_core ();
+
+      if (!core_dump_allowed)
+        gdb_demangle_attempt_core_dump = 0;
+    }

calls this:

  int
  can_dump_core (void)
  {
  #ifdef HAVE_GETRLIMIT
    struct rlimit rlim;
  
    /* Be quiet and assume we can dump if an error is returned.  */
    if (getrlimit (RLIMIT_CORE, &rlim) != 0)
      return 1;
  
    if (rlim.rlim_max == 0)
      return 0;
  #endif /* HAVE_GETRLIMIT */
  
    return 1;
  }
		  
which inhibits the core dump if the user's ulimit is 0.

> Alternatively we could just not dump core from gdb, report the bad
> symbol and let the user file a bug.  With the demangler being so
> deterministic it should be possible to reproduce, if not, then we
> just ask the user to turn off the crash catch, adjust their ulimit
> (like we would with any other gdb SEGV crash), and rerun the test.

That was and is my preferred solution, but Mark Kettenis indicated
that he would not accept the patch unless a meaningful core file was
created.

> If we really want to create the core file by default, but aren't
> going to ask, then I'd propose we honour the ulimit setting, and
> make sure that the user is told that a core file was just written.

The problem with asking is that you'd have to ask within the signal
handler, and no code that prints to the screen is safe to call from
within a signal handler.

Even indicating that a core file was written is probably impossible:
you just have to abort and hope for the best.  The nearest I could
do is set a flag in the signal handler and have the code it returns
to print "Attempting to dump core" or some such thing.

Thanks,
Gary

-- 
http://gbenson.net/