* [PATCH] Diagnose invalid pointer arithmetic on gdb.Value
@ 2016-09-20 15:06 Jonathan Wakely
2016-09-20 16:19 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2016-09-20 15:06 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 519 bytes --]
Instead of passing invalid arguments to value_binop and getting a
misleading error, raise a TypeError directly in valpy_binop_throw.
gdb/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20622
* python/py-value.c (valpy_binop_throw): Throw TypeError for pointer
arithmetic with invalid types instead of passing arguments to
value_binop.
gdb/testsuite/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20622
* gdb.python/py-value.exp: Test invalid pointer arithmetic.
[-- Attachment #2: pr20622.txt --]
[-- Type: text/plain, Size: 4750 bytes --]
commit 17ee03b254118af0108a976125984e7f5ff3858d
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Sep 20 15:39:44 2016 +0100
gdb: Diagnose invalid pointer arithmetic on gdb.Value
Instead of passing invalid arguments to value_binop and getting a
misleading error, raise a TypeError directly in valpy_binop_throw.
gdb/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20622
* python/py-value.c (valpy_binop_throw): Throw TypeError for pointer
arithmetic with invalid types instead of passing arguments to
value_binop.
gdb/testsuite/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20622
* gdb.python/py-value.exp: Test invalid pointer arithmetic.
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 21e9247..560f56a 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1064,18 +1064,28 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
rtype = check_typedef (rtype);
rtype = STRIP_REFERENCE (rtype);
- handled = 1;
- if (TYPE_CODE (ltype) == TYPE_CODE_PTR
- && is_integral_type (rtype))
- res_val = value_ptradd (arg1, value_as_long (arg2));
- else if (TYPE_CODE (rtype) == TYPE_CODE_PTR
- && is_integral_type (ltype))
- res_val = value_ptradd (arg2, value_as_long (arg1));
+ handled = 1;
+ if (TYPE_CODE (ltype) == TYPE_CODE_PTR)
+ {
+ if (is_integral_type (rtype))
+ res_val = value_ptradd (arg1, value_as_long (arg2));
+ else
+ PyErr_SetString (PyExc_TypeError, _("pointer arithmetic with "
+ "non-integer argument."));
+ }
+ else if (TYPE_CODE (rtype) == TYPE_CODE_PTR)
+ {
+ if (is_integral_type (ltype))
+ res_val = value_ptradd (arg2, value_as_long (arg1));
+ else
+ PyErr_SetString (PyExc_TypeError, _("pointer arithmetic with "
+ "non-integer argument."));
+ }
else
- {
- handled = 0;
- op = BINOP_ADD;
- }
+ {
+ handled = 0;
+ op = BINOP_ADD;
+ }
}
break;
case VALPY_SUB:
@@ -1094,9 +1104,14 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
/* A ptrdiff_t for the target would be preferable here. */
res_val = value_from_longest (builtin_type_pyint,
value_ptrdiff (arg1, arg2));
- else if (TYPE_CODE (ltype) == TYPE_CODE_PTR
- && is_integral_type (rtype))
- res_val = value_ptradd (arg1, - value_as_long (arg2));
+ else if (TYPE_CODE (ltype) == TYPE_CODE_PTR)
+ {
+ if (is_integral_type (rtype))
+ res_val = value_ptradd (arg1, - value_as_long (arg2));
+ else
+ PyErr_SetString (PyExc_TypeError, _("pointer arithmetic with "
+ "non-integer argument."));
+ }
else
{
handled = 0;
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 57a9ba1..6ad3658 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -144,6 +144,24 @@ proc test_value_numeric_ops {} {
-re "result = .*$gdb_prompt $" {fail "catch throw of GDB error"}
-re "$gdb_prompt $" {fail "catch throw of GDB error"}
}
+
+ gdb_test_multiple "python print ('result = ' + str(a+0.5))" "catch error in python type conversion" {
+ -re "pointer arithmetic with non-integer argument.*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
+
+ gdb_test_multiple "python print ('result = ' + str(0.5+a))" "catch error in python type conversion" {
+ -re "pointer arithmetic with non-integer argument.*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
+
+ gdb_test_multiple "python print ('result = ' + str(a-0.5))" "catch error in python type conversion" {
+ -re "pointer arithmetic with non-integer argument.*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
}
proc test_value_boolean {} {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Diagnose invalid pointer arithmetic on gdb.Value
2016-09-20 15:06 [PATCH] Diagnose invalid pointer arithmetic on gdb.Value Jonathan Wakely
@ 2016-09-20 16:19 ` Pedro Alves
2016-09-20 16:36 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2016-09-20 16:19 UTC (permalink / raw)
To: Jonathan Wakely, gdb-patches
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)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Diagnose invalid pointer arithmetic on gdb.Value
2016-09-20 16:19 ` Pedro Alves
@ 2016-09-20 16:36 ` Jonathan Wakely
2016-09-20 16:57 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2016-09-20 16:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]
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).
[-- Attachment #2: ptrarith.txt --]
[-- Type: text/plain, Size: 682 bytes --]
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Diagnose invalid pointer arithmetic on gdb.Value
2016-09-20 16:36 ` Jonathan Wakely
@ 2016-09-20 16:57 ` Pedro Alves
2016-09-21 11:29 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2016-09-20 16:57 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Diagnose invalid pointer arithmetic on gdb.Value
2016-09-20 16:57 ` Pedro Alves
@ 2016-09-21 11:29 ` Jonathan Wakely
2016-09-21 16:30 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2016-09-21 11:29 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]
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 #2: pr20622.txt --]
[-- Type: text/plain, Size: 3142 bytes --]
commit 39448eaecd5f2e97c6f2c1566b7bae40fa63b2ff
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Sep 20 15:39:44 2016 +0100
gdb: Better error message for invalid pointer arithmetic
The value_ptr{add,sub,diff} functions are only used when the arguments
are suitable types, otherwise we end up in scalar_binop so add a more
helpful error message there.
gdb/ChangeLog:
2016-09-21 Jonathan Wakely <jwakely@redhat.com>
PR python/20622
* valarith.c (scalar_binop): If either argument is a pointer
give a more descriptive error message.
gdb/testsuite/ChangeLog:
2016-09-21 Jonathan Wakely <jwakely@redhat.com>
PR python/20622
* gdb.python/py-value.exp: Test invalid pointer arithmetic.
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 57a9ba1..70732cc 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -144,6 +144,24 @@ proc test_value_numeric_ops {} {
-re "result = .*$gdb_prompt $" {fail "catch throw of GDB error"}
-re "$gdb_prompt $" {fail "catch throw of GDB error"}
}
+
+ gdb_test_multiple "python print ('result = ' + str(a+0.5))" "catch error in python type conversion" {
+ -re "Invalid arguments to pointer arithmetic operation..*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
+
+ gdb_test_multiple "python print ('result = ' + str(0.5+a))" "catch error in python type conversion" {
+ -re "Invalid arguments to pointer arithmetic operation..*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
+
+ gdb_test_multiple "python print ('result = ' + str(a-0.5))" "catch error in python type conversion" {
+ -re "Invalid arguments to pointer arithmetic operation..*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
}
proc test_value_boolean {} {
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Diagnose invalid pointer arithmetic on gdb.Value
2016-09-21 11:29 ` Jonathan Wakely
@ 2016-09-21 16:30 ` Pedro Alves
0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2016-09-21 16:30 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: gdb-patches
On 09/21/2016 12:07 PM, Jonathan Wakely wrote:
> 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?
>
If we keep only one set, then I think moving is appropriate.
Though I guess having both would be nice, as potentially Python
layer changes can mess this up too. Belt and suspenders.
If you could do that, it'd be great.
+ gdb_test_multiple "python print ('result = ' + str(a+0.5))" "catch error in python type conversion" {
+ -re "Invalid arguments to pointer arithmetic operation..*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
+
+ gdb_test_multiple "python print ('result = ' + str(0.5+a))" "catch error in python type conversion" {
+ -re "Invalid arguments to pointer arithmetic operation..*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
+
+ gdb_test_multiple "python print ('result = ' + str(a-0.5))" "catch error in python type conversion" {
+ -re "Invalid arguments to pointer arithmetic operation..*$gdb_prompt $" {pass "catch error in python type conversion"}
+ -re "result = .*$gdb_prompt $" {fail "catch error in python type conversion"}
+ -re "$gdb_prompt $" {fail "catch error in python type conversion"}
+ }
}
BTW, I noticed that this using the same test message in all cases,
which is a problem we've been slowing fixing:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique
I see that the test is already violating that, but I'd prefer to
not make it worse. (see below).
Also, while at it, and I know its a preexisting pattern in the
test that you're copying, but there's really no reason to have the
three regexs per test. The latter two in all cases are unnecessary,
as gdb_test_multiple has a fallback fail case for "$gdb_prompt $".
So if the first -re line doesn't match we'd get a fail anyway.
What this means is that the tests could be simplified to use gdb_test
instead. Say, like:
gdb_test "python print ('result = ' + str(a+0.5))" \
"Invalid arguments to pointer arithmetic operation..*" \
"catch error in python type conversion: a+0.5"
gdb_test "python print ('result = ' + str(0.5+a))" \
"Invalid arguments to pointer arithmetic operation..*" \
"catch error in python type conversion: 0.5+a"
gdb_test "python print ('result = ' + str(a-0.5))" \
"Invalid arguments to pointer arithmetic operation..*" \
"catch error in python type conversion: a-0.5"
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-21 16:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20 15:06 [PATCH] Diagnose invalid pointer arithmetic on gdb.Value Jonathan Wakely
2016-09-20 16:19 ` Pedro Alves
2016-09-20 16:36 ` Jonathan Wakely
2016-09-20 16:57 ` Pedro Alves
2016-09-21 11:29 ` Jonathan Wakely
2016-09-21 16:30 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).