From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28976 invoked by alias); 22 Nov 2008 18:16:05 -0000 Received: (qmail 28871 invoked by uid 22791); 22 Nov 2008 18:16:04 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 22 Nov 2008 18:15:29 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 801D92A9648; Sat, 22 Nov 2008 13:15:27 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id rIwd16UgxSkp; Sat, 22 Nov 2008 13:15:27 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 20B1A2A9647; Sat, 22 Nov 2008 13:15:27 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 0A28AE7ACD; Sat, 22 Nov 2008 10:15:25 -0800 (PST) Date: Mon, 24 Nov 2008 02:25:00 -0000 From: Joel Brobecker To: Eli Zaretskii Cc: Jan Kratochvil , gdb-patches@sourceware.org Subject: Re: [patch] ia64: Fix breakpoints memory shadow Message-ID: <20081122181525.GG4318@adacore.com> References: <20081028172816.GA1284@host0.dyn.jankratochvil.net> <20081029210242.GA3635@adacore.com> <20081030144841.GA26606@host0.dyn.jankratochvil.net> <20081101185410.GB15606@adacore.com> <20081111131726.GA3272@host0.dyn.jankratochvil.net> <20081113053100.GL5112@adacore.com> <20081120144936.GA25926@host0.dyn.jankratochvil.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.2i 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 X-SW-Source: 2008-11/txt/msg00616.txt.bz2 > > + instr_breakpoint = slotN_contents (bundle_mem, slotnum); > > + if (instr_breakpoint != IA64_BREAKPOINT) > > + warning (_("Breakpoint removal cannot find the placed breakpoint at %s"), > > + paddr_nz (bp_tgt->placed_address)); > > Can this happen as part of normal GDB behavior? If not, we should > make this internal_error, I think. If indeed this is a warning, we > should tell users what to do with such a warning, either as part of > the message or at least in the manual. Maybe we can imagine some far-fetched scenario where the user is in non-stop mode, meaning that the breakpoints are always inserted, and that the user somehow overwrites the breakpoint instruction manually before switching back to all-stop (is that even possible?). In that case, that would be a valid warning. Regardless of that, I thought that the warning was a judicious choice because the debugger might be able to recover from this situation. Whether or not there was a breakpoint there, it can still restore the instruction at that address and hopefully continue working normally. Changing the warning into an internal_error would cause the debugger to abruptly abort the current operation (after which the user might choose to end the debugging session). I thought that attempting to recover would bring more benefit in this case. I would agree that some extra documentation explaining this warning might be useful. That being said, I'm not opposed to changing the warning into an internal_error, if you still think it's better. > > + if (slotnum > 2) > > + error (_("Can't insert breakpoint for slot numbers greater than 2.")); > > Similarly here: assuming slot numbers are not something exposed to the > user, I'd be bewildered if presented with such a message. If this is > an internal GDB error (a.k.a. bug), let's treat it like one. In this case, the invalid address can occur as a result of bad user input: (top-gdb) b *0x4000000000054c73 Breakpoint 4 at 0x4000000000054c73: file gdb.c, line 28. (top-gdb) c Continuing. Can't insert breakpoint for slot numbers greater than 2. So this is not necessarily an internal error. I wish GDB would catch the invalid address a little earlier, though. -- Joel