From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8925 invoked by alias); 9 Jun 2014 11:48:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 8894 invoked by uid 89); 9 Jun 2014 11:48:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Jun 2014 11:48:51 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s59Bmjde016660 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 9 Jun 2014 07:48:45 -0400 Received: from blade.nx (ovpn-116-92.ams2.redhat.com [10.36.116.92]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s59BmhVl019689; Mon, 9 Jun 2014 07:48:44 -0400 Received: by blade.nx (Postfix, from userid 1000) id 2B24D262409; Mon, 9 Jun 2014 12:48:43 +0100 (BST) Date: Mon, 09 Jun 2014 11:48:00 -0000 From: Gary Benson To: Andrew Burgess Cc: gdb-patches@sourceware.org, Doug Evans , Eli Zaretskii , Florian Weimer , Mark Kettenis , Pedro Alves , Tom Tromey Subject: Re: [PATCH 3/3 v4] Demangler crash handler Message-ID: <20140609114843.GA32608@blade.nx> References: <20140605130140.GA20572@blade.nx> <20140605130358.GD20572@blade.nx> <53922EBD.7030300@broadcom.com> <20140609090123.GA30086@blade.nx> <53958BC9.9060107@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53958BC9.9060107@broadcom.com> X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg00368.txt.bz2 Andrew Burgess wrote: > On 09/06/2014 10:01 AM, Gary Benson wrote: > > 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. > > Ahh, yes I see. > > So the problem here is this function is geared towards the /old/ use > of the function where we are about to ask the user if we should dump > core. For that, this function was correct, we check the hard limit > of the resource. If the hard limit is high then we ask the user, > and dump core. > > However, in doing so we circumvent the soft limit rlim.rlim_cur. So > I think my point still stands. The user has said "no core files > please", and we create one without asking. If we must go down this > road then I think we need two functions to check the two different > limits. Ah, I didn't realize the code in dump_core was to override the user's soft limit. I will update the patch. > > > 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. > > I don't understand that position, but I'd hope he'd agree that we > should respect the user ulimit over creating a core file... Yes, this seems reasonable. > > > 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. > > Indeed. I did wonder about some horrible synchronisation scheme > where the "master" gdb process queries the user then signals the > fork()ed child to indicate if it should dump core or not .... but > it felt like huge overkill. Yeah, I thought down this road too :) > > 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. > > I think an "attempting ..." style message would be enough, the > gdb_demangle_attempt_core_dump flag could be used to indicate > if we've tried to dump core or not. I will add this to the updated patch. Thanks, Gary -- http://gbenson.net/