Diagnose invalid pointer arithmetic on gdb.Value
Commit Message
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).
Comments
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.
Thanks,
Pedro Alves
@@ -951,7 +951,9 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
type1 = check_typedef (value_type (arg1));
type2 = check_typedef (value_type (arg2));
- if ((TYPE_CODE (type1) != TYPE_CODE_FLT
+ if (TYPE_CODE (type1) == TYPE_CODE_PTR || TYPE_CODE (type2) == TYPE_CODE_PTR)
+ error (_("Invalid arguments to pointer arithmetic operation."));
+ else if ((TYPE_CODE (type1) != TYPE_CODE_FLT
&& TYPE_CODE (type1) != TYPE_CODE_DECFLOAT
&& !is_integral_type (type1))
|| (TYPE_CODE (type2) != TYPE_CODE_FLT