From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3202 invoked by alias); 9 Jun 2014 10:26:29 -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 3188 invoked by uid 89); 9 Jun 2014 10:26:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-gw1-out.broadcom.com Received: from mail-gw1-out.broadcom.com (HELO mail-gw1-out.broadcom.com) (216.31.210.62) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Jun 2014 10:26:27 +0000 Received: from irvexchcas08.broadcom.com (HELO IRVEXCHCAS08.corp.ad.broadcom.com) ([10.9.208.57]) by mail-gw1-out.broadcom.com with ESMTP; 09 Jun 2014 03:38:51 -0700 Received: from IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) by IRVEXCHCAS08.corp.ad.broadcom.com (10.9.208.57) with Microsoft SMTP Server (TLS) id 14.3.174.1; Mon, 9 Jun 2014 03:26:24 -0700 Received: from mail-irva-13.broadcom.com (10.10.10.20) by IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) with Microsoft SMTP Server id 14.3.174.1; Mon, 9 Jun 2014 03:26:24 -0700 Received: from [10.177.73.41] (unknown [10.177.73.41]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id 0B9189FA87; Mon, 9 Jun 2014 03:26:18 -0700 (PDT) Message-ID: <53958BC9.9060107@broadcom.com> Date: Mon, 09 Jun 2014 10:26:00 -0000 From: Andrew Burgess User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Gary Benson CC: , Doug Evans , Eli Zaretskii , Florian Weimer , Mark Kettenis , Pedro Alves , Tom Tromey Subject: Re: [PATCH 3/3 v4] Demangler crash handler References: <20140605130140.GA20572@blade.nx> <20140605130358.GD20572@blade.nx> <53922EBD.7030300@broadcom.com> <20140609090123.GA30086@blade.nx> In-Reply-To: <20140609090123.GA30086@blade.nx> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg00365.txt.bz2 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. >> 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... > >> 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. > 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. Thanks, Andrew