Re: [patch] Add proper error message instead of gdb_assert

[Michael Snyder <msnyder@specifix.com>]

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.  */