diff mbox

Diagnose invalid pointer arithmetic on gdb.Value

Message ID 20160920161936.GA5736@redhat.com
State New, archived
Headers

Commit Message

Jonathan Wakely Sept. 20, 2016, 4:19 p.m. UTC 
  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

Pedro Alves Sept. 20, 2016, 4:42 p.m. UTC | #1 
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
diff mbox

Patch

diff --git a/gdb/valarith.c b/gdb/valarith.c
index de6fcfd..546d4b6 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -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