Re: [PATCH] Diagnose invalid pointer arithmetic on gdb.Value

[Jonathan Wakely <jwakely at redhat dot com>]

On 20/09/16 17:42 +0100, Pedro Alves wrote:
> On 09/20/2016 05:19 PM, Jonathan Wakely wrote:
> > On 20/09/16 16:41 +0100, Pedro Alves wrote:
> > > On 09/20/2016 03:46 PM, Jonathan Wakely wrote:
> > > > Instead of passing invalid arguments to value_binop and getting a
> > > > misleading error, raise a TypeError directly in valpy_binop_throw.
> > >
> > > Did you try changing value_binop instead?  The error string seems
> > > misleading even in C:
> > >
> > > (gdb) p ptr
> > > $1 = 0x601040 <buf> ""
> > > (gdb) p ptr + 1
> > > $2 = 0x601041 <buf+1> ""
> > > (gdb) p ptr + 1.0
> > > Argument to arithmetic operation not a number or boolean.
> > > (gdb)
> >
> > Ah, so it's not specific to the Python API, in which case changing
> > value_binop (or scalar_binop more accurately) might make sense.
> > The check in scalar_binop *does* check for numbers, so the error is
> > accurate. The argument that triggers the errors is actually the
> > pointer argument, not the float one:
> >
> >  if ((TYPE_CODE (type1) != TYPE_CODE_FLT
> >       && TYPE_CODE (type1) != TYPE_CODE_DECFLOAT
> >       && !is_integral_type (type1))
> >      || (TYPE_CODE (type2) != TYPE_CODE_FLT
> >       && TYPE_CODE (type2) != TYPE_CODE_DECFLOAT
> >       && !is_integral_type (type2)))
> >    error (_("Argument to arithmetic operation not a number or boolean."));
> >
> >
> > The problem is that pointer arithmetic with invalid operands ends up
> > here, because value_ptr{add,sub,diff} don't get called for invalid
> > arguments.
> >
> > So maybe a better fix is to first check if either argument is a
> > pointer and give a more specific error, as attached (untested).
>
> Makes sense to me.

Here's a patch to do that. None of the existing tests needed to change
as far as I can see. gdb.base/pointers.exp and gdb.base/completion.exp
have tests that try to match the old error, but they're commented out.

I kept my additions to py-value.exp that do invalid pointer
arithmetic, updating the expected text. Should I move that to
gdb.base/pointers.exp instead? Or check it in both?

Attachment: pr20622.txt
Description: Text document