From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22148 invoked by alias); 3 Mar 2008 20:25:11 -0000 Received: (qmail 22134 invoked by uid 22791); 3 Mar 2008 20:25:10 -0000 X-Spam-Check-By: sourceware.org Received: from bluesmobile.specifix.com (HELO bluesmobile.specifix.com) (216.129.118.140) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 03 Mar 2008 20:24:53 +0000 Received: from [127.0.0.1] (bluesmobile.specifix.com [216.129.118.140]) by bluesmobile.specifix.com (Postfix) with ESMTP id 9EA283BF5B; Mon, 3 Mar 2008 12:24:51 -0800 (PST) Subject: Re: [patch] Add proper error message instead of gdb_assert From: Michael Snyder To: Markus Deuling Cc: GDB Patches , Ulrich Weigand In-Reply-To: <47CC5332.3020700@de.ibm.com> References: <47CC5332.3020700@de.ibm.com> Content-Type: text/plain Date: Mon, 03 Mar 2008 20:25:00 -0000 Message-Id: <1204575891.19253.578.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.10.3 (2.10.3-7.fc7) Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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-03/txt/msg00018.txt.bz2 In general I don't like asserts, and especially I don't like them when the error is something local as opposed to something un-recoverable. Whenever it is NOT an unrecoverable error, I think that replacing an assert with an error message should be a good thing. I'm inviting comment. ;-) On Mon, 2008-03-03 at 20:36 +0100, Markus Deuling wrote: > Hi, > > when trying to put > 1 values into an array (fortran subrange) which > comes from a register, register_size is called with regnum == -1. > > The following example comes from SPU architecture. Currently GDB exits > with a gdb_assert going wrong: > > (gdb) set $r0%v2_int64(0:1)=(1,2) > ../../../gdb-6.7/gdb/regcache.c:177: internal-error: register_size: Assertion `regnum >= 0 && regnum < (gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch))' failed. > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) y > > This patch replaces that gdb_assert by a proper error message before > exiting: > > (gdb) set $r2%v2_int64(0:1)=(1,1) > ../../../gdb-6.7/gdb/regcache.c:185: internal-error: invalid register -1 > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) > > If this patch is ok it would be great to have it in gdb 6.8. Ok? > > ChangeLog: > > * regcache.c (register_size): Replace gdb_assert by a proper error > message. > > > Regards, > Markus > > > plain text document attachment (fix-assert) > diff -urpN gdb-6.7-orig/gdb/regcache.c gdb-6.7/gdb/regcache.c > --- gdb-6.7-orig/gdb/regcache.c 2008-03-03 19:30:54.000000000 +0100 > +++ gdb-6.7/gdb/regcache.c 2008-03-03 20:30:10.000000000 +0100 > @@ -172,11 +172,18 @@ register_size (struct gdbarch *gdbarch, > { > struct regcache_descr *descr = regcache_descr (gdbarch); > int size; > - gdb_assert (regnum >= 0 > - && regnum < (gdbarch_num_regs (gdbarch) > - + gdbarch_num_pseudo_regs (gdbarch))); > - size = descr->sizeof_register[regnum]; > - return size; > + > + > + if (regnum >= 0 > + && regnum < (gdbarch_num_regs (gdbarch) > + + gdbarch_num_pseudo_regs (gdbarch))) > + { > + size = descr->sizeof_register[regnum]; > + return size; > + } > + > + internal_error (__FILE__, __LINE__, _("invalid register %d"), regnum); > + return 0; > } > > /* The register cache for storing raw register values. */