[PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Hannes Domani]

Considering these variables:
int i = 3;
int &iref = i;

It's not possible to do any pointer arithmetic with iref:
(gdb) p &i+iref
Argument to arithmetic operation not a number or boolean.

So this adds checks for references to integers in pointer arithmetic.

gdb/ChangeLog:

2020-04-01  Hannes Domani  <ssbssa@yahoo.de>

	PR gdb/24789
	* eval.c (is_integral_or_integral_reference): New function.
	(evaluate_subexp_standard): Allow integer references in
	pointer arithmetic.

gdb/testsuite/ChangeLog:

2020-04-01  Hannes Domani  <ssbssa@yahoo.de>

	PR gdb/24789
	* gdb.cp/misc.cc: Add integer reference variable.
	* gdb.cp/misc.exp: Add test.
---
v2:
- no assignment in expression, new helper function instead
- add test cases
---
 gdb/eval.c                    | 20 +++++++++++++++++---
 gdb/testsuite/gdb.cp/misc.cc  |  3 +++
 gdb/testsuite/gdb.cp/misc.exp |  8 ++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 17af1b51df..0425c59f8d 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1248,6 +1248,20 @@ skip_undetermined_arglist (int nargs, struct expression *exp, int *pos,
     evaluate_subexp (NULL_TYPE, exp, pos, noside);
 }
 
+/* Return true if type is integral or reference to integral */
+
+static bool
+is_integral_or_integral_reference (struct type *type)
+{
+  if (is_integral_type (type))
+    return true;
+
+  type = check_typedef (type);
+  return type != nullptr
+    && TYPE_IS_REFERENCE (type)
+    && is_integral_type (TYPE_TARGET_TYPE (type));
+}
+
 struct value *
 evaluate_subexp_standard (struct type *expect_type,
 			  struct expression *exp, int *pos,
@@ -2208,10 +2222,10 @@ evaluate_subexp_standard (struct type *expect_type,
       if (binop_user_defined_p (op, arg1, arg2))
 	return value_x_binop (arg1, arg2, op, OP_NULL, noside);
       else if (ptrmath_type_p (exp->language_defn, value_type (arg1))
-	       && is_integral_type (value_type (arg2)))
+	       && is_integral_or_integral_reference (value_type (arg2)))
 	return value_ptradd (arg1, value_as_long (arg2));
       else if (ptrmath_type_p (exp->language_defn, value_type (arg2))
-	       && is_integral_type (value_type (arg1)))
+	       && is_integral_or_integral_reference (value_type (arg1)))
 	return value_ptradd (arg2, value_as_long (arg1));
       else
 	{
@@ -2234,7 +2248,7 @@ evaluate_subexp_standard (struct type *expect_type,
 	  return value_from_longest (type, value_ptrdiff (arg1, arg2));
 	}
       else if (ptrmath_type_p (exp->language_defn, value_type (arg1))
-	       && is_integral_type (value_type (arg2)))
+	       && is_integral_or_integral_reference (value_type (arg2)))
 	return value_ptradd (arg1, - value_as_long (arg2));
       else
 	{
diff --git a/gdb/testsuite/gdb.cp/misc.cc b/gdb/testsuite/gdb.cp/misc.cc
index d461d6de61..41fb9d2f2d 100644
--- a/gdb/testsuite/gdb.cp/misc.cc
+++ b/gdb/testsuite/gdb.cp/misc.cc
@@ -24,6 +24,9 @@ bool            v_bool_array[2];
 typedef struct fleep fleep;
 struct fleep { int a; } s;
 
+int number;
+int &number_ref = number;
+
 // ====================== simple class structures  =======================
 
 struct default_public_struct {
diff --git a/gdb/testsuite/gdb.cp/misc.exp b/gdb/testsuite/gdb.cp/misc.exp
index bceb73ef87..cd6f0f7070 100644
--- a/gdb/testsuite/gdb.cp/misc.exp
+++ b/gdb/testsuite/gdb.cp/misc.exp
@@ -110,3 +110,11 @@ gdb_test "print (int)false" "\\$\[0-9\]* = 0" "(int)false"
 
 gdb_test "print 'misc.cc'::v_bool" " = true" \
     "expression using block qualifier"
+
+# pointer arithmetic
+gdb_test "print *(v_bool_array + number_ref)" "\\$\[0-9\]* = false" \
+    "pointer addition with integer reference"
+gdb_test "print *(number_ref + v_bool_array)" "\\$\[0-9\]* = false" \
+    "pointer addition with integer reference"
+gdb_test "print *(v_bool_array - number_ref)" "\\$\[0-9\]* = false" \
+    "pointer subtraction with integer reference"
-- 
2.26.0

Re: [PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> 2020-04-01  Hannes Domani  <ssbssa@yahoo.de>

Hannes> 	PR gdb/24789
Hannes> 	* eval.c (is_integral_or_integral_reference): New function.
Hannes> 	(evaluate_subexp_standard): Allow integer references in
Hannes> 	pointer arithmetic.

Thank you.

Hannes> +/* Return true if type is integral or reference to integral */
Hannes> +
Hannes> +static bool
Hannes> +is_integral_or_integral_reference (struct type *type)
Hannes> +{
Hannes> +  if (is_integral_type (type))
Hannes> +    return true;
Hannes> +
Hannes> +  type = check_typedef (type);
Hannes> +  return type != nullptr
Hannes> +    && TYPE_IS_REFERENCE (type)
Hannes> +    && is_integral_type (TYPE_TARGET_TYPE (type));

This needs parens around the multi-line expression, like:

  return (type != nullptr
	  && TYPE_IS_REFERENCE (type)
	  && is_integral_type (TYPE_TARGET_TYPE (type)));

The patch is ok with this tweak.

Tom

Re: [PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Hannes Domani]

 Am Mittwoch, 1. April 2020, 18:36:47 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Hannes> 2020-04-01  Hannes Domani  <ssbssa@yahoo.de>
>
> Hannes>     PR gdb/24789
> Hannes>     * eval.c (is_integral_or_integral_reference): New function.
> Hannes>     (evaluate_subexp_standard): Allow integer references in
> Hannes>     pointer arithmetic.
>
> Thank you.
>
> Hannes> +/* Return true if type is integral or reference to integral */
> Hannes> +
> Hannes> +static bool
> Hannes> +is_integral_or_integral_reference (struct type *type)
> Hannes> +{
> Hannes> +  if (is_integral_type (type))
> Hannes> +    return true;
> Hannes> +
> Hannes> +  type = check_typedef (type);
> Hannes> +  return type != nullptr
> Hannes> +    && TYPE_IS_REFERENCE (type)
> Hannes> +    && is_integral_type (TYPE_TARGET_TYPE (type));
>
> This needs parens around the multi-line expression, like:
>
>
>   return (type != nullptr
>       && TYPE_IS_REFERENCE (type)
>       && is_integral_type (TYPE_TARGET_TYPE (type)));
>
>
> The patch is ok with this tweak.

Pushed with this change, thanks.


Hannes

Re: [PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Hannes Domani]

 Am Mittwoch, 1. April 2020, 19:17:58 MESZ hat Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> Am Mittwoch, 1. April 2020, 18:36:47 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
>
> > >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> >
> > Hannes> 2020-04-01  Hannes Domani  <ssbssa@yahoo.de>
> >
> > Hannes>     PR gdb/24789
> > Hannes>     * eval.c (is_integral_or_integral_reference): New function.
> > Hannes>     (evaluate_subexp_standard): Allow integer references in
> > Hannes>     pointer arithmetic.
> >
> > Thank you.
> >
> > Hannes> +/* Return true if type is integral or reference to integral */
> > Hannes> +
> > Hannes> +static bool
> > Hannes> +is_integral_or_integral_reference (struct type *type)
> > Hannes> +{
> > Hannes> +  if (is_integral_type (type))
> > Hannes> +    return true;
> > Hannes> +
> > Hannes> +  type = check_typedef (type);
> > Hannes> +  return type != nullptr
> > Hannes> +    && TYPE_IS_REFERENCE (type)
> > Hannes> +    && is_integral_type (TYPE_TARGET_TYPE (type));
> >
> > This needs parens around the multi-line expression, like:
> >
> >
> >   return (type != nullptr
> >       && TYPE_IS_REFERENCE (type)
> >       && is_integral_type (TYPE_TARGET_TYPE (type)));
> >
> >
> > The patch is ok with this tweak.
>
> Pushed with this change, thanks.

That leads me to another question, who/when are PRs closed after pushing?

So in this case this one:
https://sourceware.org/bugzilla/show_bug.cgi?id=24789

And these are 2 other which I think should have been closed some time ago
after someone implemented it:
https://sourceware.org/bugzilla/show_bug.cgi?id=8684
https://sourceware.org/bugzilla/show_bug.cgi?id=14529

And these problems can no longer be reproduced since some versions:
https://sourceware.org/bugzilla/show_bug.cgi?id=17667
https://sourceware.org/bugzilla/show_bug.cgi?id=22541
https://sourceware.org/bugzilla/show_bug.cgi?id=17521


Hannes

Re: [PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> That leads me to another question, who/when are PRs closed after
Hannes> pushing?

Someone has to do it manually.  If I fix a bug, normally I do it myself
after pushing.

If you do this, be sure to set the target milestone on the bug.
That makes it easy to search for bugs closed in a given release.
If I don't know the release, I tend to leave it blank, though it's also
ok if you want to dig through the git log to find the answer.

I'll look at the ones you mentioned.

Tom

Re: [PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Hannes Domani]

 Am Mittwoch, 1. April 2020, 20:11:30 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> If you want bugzilla rights to close bugs, let me know.
> I don't know how to get you set up with this, but we can find out.

I think that would be good, I just wanted to reply to you that I can't close
any bugs.


Hannes

Re: [PATCH v2][PR gdb/24789] Allow pointer arithmetic with integer references

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

>> If you want bugzilla rights to close bugs, let me know.
>> I don't know how to get you set up with this, but we can find out.

Hannes> I think that would be good, I just wanted to reply to you that I can't close
Hannes> any bugs.

Frank reminded me that normally this is tied to the email address on the
account; and if you use your sourceware.org address to make a bugzilla
account, then it should probably work.

Tom