From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31149 invoked by alias); 11 May 2014 20:23:37 -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 31131 invoked by uid 89); 11 May 2014 20:23:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: glazunov.sibelius.xs4all.nl Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sun, 11 May 2014 20:23:30 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id s4BKNM5h023960; Sun, 11 May 2014 22:23:22 +0200 (CEST) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id s4BKNL3v024248; Sun, 11 May 2014 22:23:21 +0200 (CEST) Date: Sun, 11 May 2014 20:23:00 -0000 Message-Id: <201405112023.s4BKNL3v024248@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: gbenson@redhat.com CC: gdb-patches@sourceware.org In-reply-to: <20140509153305.GA13345@blade.nx> (message from Gary Benson on Fri, 9 May 2014 16:33:06 +0100) Subject: Re: [PATCH 0/2] Demangler crash handler References: <20140509100656.GA4760@blade.nx> <201405091120.s49BKO1f010622@glazunov.sibelius.xs4all.nl> <20140509153305.GA13345@blade.nx> X-SW-Source: 2014-05/txt/msg00129.txt.bz2 > Date: Fri, 9 May 2014 16:33:06 +0100 > From: Gary Benson > > Mark Kettenis wrote: > > > A number of bugs have been filed recently because of segmentation > > > faults in the demangler. While such crashes are a problem for all > > > demangler consumers, they are particularly nasty for GDB because > > > they prevent the user from debugging their program at all. > > > > > > This patch series arranges for GDB to catch segmentation faults > > > in the demangler and recover from them gracefully. A warning is > > > printed the first time a fault occurs. Example sessions with and > > > without these patches are included below. > > > > > > None of the wrapped code uses cleanups, so each caught failure > > > will leak a small amount of memory. This is undesirable but I > > > think the benefits here outweigh this drawback. > > > > > > Ok to commit? > > > > 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 would normally agree with you 100% on this issue Mark, but in this > case I think a handler is justified. If the demangler crashes because > of a symbol in the users program then the user cannot debug their > program at all. If the demangler were simple and well understood then > that would be fine but it's not: the demangler is complex, the > specification it's following is complex, and everything's complicated > further because you can't allocate heap and you have to roll your own > data structures. The reality is that the libiberty demangler is a > breeding ground for segfaults, and GDB needs to be able to deal with > this. There are entire subsystems in GDB that are a breeding ground for segfaults. Should we therefore wrap evrything? It is obvious that the demangler is a breeding ground for segmentation faults. It uses strcpy, strcat and sprintf. So it's probably full of buffer overflows. I bet that if those are fixed, the SIGSEGVs are gone. Note that only some of those buffer overflows will generate a SIGSEGV. Others will corrupt random memory. And you can't patch those up with a signal handler. > It's true that you don't get core dumps with this patch, but what you > do get in return is a printed warning that includes the symbol that > caused the crash. That's all you need in most cases. The five recent > demangler crashes (14963, 16593, 16752, 16817 and 16845) all required > digging by either the reporter or a GDB developer to uncover the > failing symbol. Printing the offending symbol means this work is > already done. > > If the lack of core dumps is a showstopper for you then I can > update the patch to allow disabling the handler with > "maint set handle-demangler-crashes 0" or some similar thing. Not acceptable. Unless you make it the default...