[PATCH 1/2] gdb: Forward VALUE_LVAL when avoiding side effects for STRUCTOP_PTR

[PATCH 1/2] gdb: Forward VALUE_LVAL when avoiding side effects for STRUCTOP_PTR

[Andrew Burgess]

Assume that we have a C program like this:

  struct foo_type
  {
    int var;
  } foo;

  struct foo_type *foo_ptr = &foo;

  int
  main ()
  {
    return foo_ptr->var;
  }

Then GDB should be able to evaluate the following, however, it currently
does not:

  (gdb) start
  ...
  (gdb) whatis &(foo_ptr->var)
  Attempt to take address of value not located in memory.

The problem is that in EVAL_AVOID_SIDE_EFFECTS mode,
eval.c:evaluate_subexp_standard always returns a not_lval value as the
result for a STRUCTOP_PTR operation. As a consequence, the rest of
the code believes that one cannot take the address of the returned
value.

This patch fixes STRUCTOP_PTR handling so that the VALUE_LVAL
attribute for the returned value is properly initialized.  After this
change, the above session becomes:

  (gdb) start
  ...
  (gdb) whatis &(foo_ptr->var)
  type = int *

This commit is largely the same as commit 2520f728b710 but applied to
STRUCTOP_PTR rather than STRUCTOP_STRUCT.  Both of these commits are
building on top of commit ac1ca910d74d.

gdb/ChangeLog:

	* eval.c (evaluate_subexp_standard): If EVAL_AVOID_SIDE_EFFECTS
	mode, forward the VALUE_LVAL attribute to the returned value in
	the STRUCTOP_PTR case.

gdb/testsuite/ChangeLog:

	* gdb.base/whatis.c: Extend the test case.
	* gdb.base/whatis.exp: Add additional tests.
---
 gdb/ChangeLog                     |  6 +++
 gdb/eval.c                        |  2 +-
 gdb/testsuite/ChangeLog           |  5 +++
 gdb/testsuite/gdb.base/whatis.c   |  8 ++--
 gdb/testsuite/gdb.base/whatis.exp | 83 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 46f5b42..7644c08 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-05-24  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* eval.c (evaluate_subexp_standard): If EVAL_AVOID_SIDE_EFFECTS
+	mode, forward the VALUE_LVAL attribute to the returned value in
+	the STRUCTOP_PTR case.
+
 2016-05-24  Yan-Ting Lin  <currygt52@gmail.com>
 
 	* MAINTAINERS (Write After Approval): Add "Yan-Ting Lin".
diff --git a/gdb/eval.c b/gdb/eval.c
index 3f8ca66..de1c663 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1917,7 +1917,7 @@ evaluate_subexp_standard (struct type *expect_type,
       arg3 = value_struct_elt (&arg1, NULL, &exp->elts[pc + 2].string,
 			       NULL, "structure pointer");
       if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	arg3 = value_zero (value_type (arg3), not_lval);
+	arg3 = value_zero (value_type (arg3), VALUE_LVAL (arg3));
       return arg3;
 
     case STRUCTOP_MEMBER:
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1f0e3f9..7825241 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-05-04  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/whatis.c: Extend the test case.
+	* gdb.base/whatis.exp: Add additional tests.
+
 2016-05-23  Tom Tromey  <tom@tromey.com>
 
 	PR python/19438, PR python/18393:
diff --git a/gdb/testsuite/gdb.base/whatis.c b/gdb/testsuite/gdb.base/whatis.c
index d60afa2..e5cfa51 100644
--- a/gdb/testsuite/gdb.base/whatis.c
+++ b/gdb/testsuite/gdb.base/whatis.c
@@ -135,7 +135,7 @@ struct t_struct {
 #endif
     float	v_float_member;
     double	v_double_member;
-} v_struct1;
+} v_struct1, *v_struct_ptr1;
 
 struct {
     char	v_char_member;
@@ -147,7 +147,7 @@ struct {
 #endif
     float	v_float_member;
     double	v_double_member;
-} v_struct2;
+} v_struct2, *v_struct_ptr2;
 
 /**** unions *******/
 
@@ -161,7 +161,7 @@ union t_union {
 #endif
     float	v_float_member;
     double	v_double_member;
-} v_union;
+} v_union, *v_union_ptr;
 
 union {
     char	v_char_member;
@@ -173,7 +173,7 @@ union {
 #endif
     float	v_float_member;
     double	v_double_member;
-} v_union2;
+} v_union2, *v_union_ptr2;
 
 /*** Functions returning type ********/
 
diff --git a/gdb/testsuite/gdb.base/whatis.exp b/gdb/testsuite/gdb.base/whatis.exp
index 93a4d44..0cdcf5b 100644
--- a/gdb/testsuite/gdb.base/whatis.exp
+++ b/gdb/testsuite/gdb.base/whatis.exp
@@ -294,6 +294,47 @@ gdb_test "whatis v_struct2" \
     "type = struct \{\.\.\.\}" \
     "whatis unnamed structure"
 
+gdb_test "whatis &v_struct1" \
+    "type = struct t_struct \\*"
+
+gdb_test "whatis &v_struct2" \
+    "type = struct {\\.\\.\\.} \\*"
+
+gdb_test "whatis v_struct_ptr1" \
+    "type = struct t_struct \\*"
+
+gdb_test "whatis v_struct_ptr2" \
+    "type = struct {\\.\\.\\.} \\*"
+
+gdb_test "whatis &v_struct_ptr1" \
+    "type = struct t_struct \\*\\*"
+
+gdb_test "whatis &v_struct_ptr2" \
+    "type = struct {\\.\\.\\.} \\*\\*"
+
+gdb_test "whatis v_struct1.v_char_member" \
+    "type = char"
+
+gdb_test "whatis v_struct2.v_char_member" \
+    "type = char"
+
+gdb_test "whatis v_struct_ptr1->v_char_member" \
+    "type = char"
+
+gdb_test "whatis v_struct_ptr2->v_char_member" \
+    "type = char"
+
+gdb_test "whatis &(v_struct1.v_char_member)" \
+    "type = char \\*"
+
+gdb_test "whatis &(v_struct2.v_char_member)" \
+    "type = char \\*"
+
+gdb_test "whatis &(v_struct_ptr1->v_char_member)" \
+    "type = char \\*"
+
+gdb_test "whatis &(v_struct_ptr2->v_char_member)" \
+    "type = char \\*"
 
 # test whatis command with union types
 gdb_test "whatis v_union" \
@@ -308,6 +349,48 @@ gdb_test "whatis v_union2" \
     "type = union \{\.\.\.\}" \
     "whatis unnamed union"
 
+gdb_test "whatis &v_union" \
+    "type = union t_union \\*"
+
+gdb_test "whatis &v_union2" \
+    "type = union {\\.\\.\\.} \\*"
+
+gdb_test "whatis v_union_ptr" \
+    "type = union t_union \\*"
+
+gdb_test "whatis v_union_ptr2" \
+    "type = union {\\.\\.\\.} \\*"
+
+gdb_test "whatis &v_union_ptr" \
+    "type = union t_union \\*\\*"
+
+gdb_test "whatis &v_union_ptr2" \
+    "type = union {\\.\\.\\.} \\*\\*"
+
+gdb_test "whatis v_union.v_char_member" \
+    "type = char"
+
+gdb_test "whatis v_union2.v_char_member" \
+    "type = char"
+
+gdb_test "whatis v_union_ptr->v_char_member" \
+    "type = char"
+
+gdb_test "whatis v_union_ptr2->v_char_member" \
+    "type = char"
+
+gdb_test "whatis &(v_union.v_char_member)" \
+    "type = char \\*"
+
+gdb_test "whatis &(v_union2.v_char_member)" \
+    "type = char \\*"
+
+gdb_test "whatis &(v_union_ptr->v_char_member)" \
+    "type = char \\*"
+
+gdb_test "whatis &(v_union_ptr2->v_char_member)" \
+    "type = char \\*"
+
 
 # Using stabs we will mark these functions as prototyped.  This
 # is harmless but causes an extra VOID to be printed.
-- 
2.5.1

[PATCH 2/2] gdb: Forward VALUE_LVAL when avoiding side effects for STRUCTOP_STRUCT

[Andrew Burgess]

When evaluating an expression with EVAL_AVOID_SIDE_EFFECTS if the value
we return is forced to be of type not_lval then GDB will be unable to
take the address of the returned value.

Instead, we should properly initialise the LVAL of the returned value.

This commit builds on two previous commits 2520f728b710 and
c244f977c7b8, which in turn build on ac1ca910d74d.  This commit is
currently untested due to my lack of access to an OpenCL compiler,
however, if follows the same pattern as the previous two commits and so
I believe that it is correct.

gdb/ChangeLog:

	* opencl-lang.c (evaluate_subexp_opencl): If
	EVAL_AVOID_SIDE_EFFECTS mode, forward the VALUE_LVAL attribute to
	the returned value in the STRUCTOP_STRUCT case.
---
 gdb/ChangeLog     | 6 ++++++
 gdb/opencl-lang.c | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7644c08..b9005a3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-05-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* opencl-lang.c (evaluate_subexp_opencl): If
+	EVAL_AVOID_SIDE_EFFECTS mode, forward the VALUE_LVAL attribute to
+	the returned value in the STRUCTOP_STRUCT case.
+
+2016-05-24  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* eval.c (evaluate_subexp_standard): If EVAL_AVOID_SIDE_EFFECTS
 	mode, forward the VALUE_LVAL attribute to the returned value in
 	the STRUCTOP_PTR case.
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 7799735..767d3bc 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -984,7 +984,7 @@ Cannot perform conditional operation on vectors with different sizes"));
 						"structure");
 
 	    if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	      v = value_zero (value_type (v), not_lval);
+	      v = value_zero (value_type (v), VALUE_LVAL (v));
 	    return v;
 	  }
       }
-- 
2.5.1

[PATCH 0/2] Forward VALUE_LVAL for STRUCT/STRUCT-PTR expressions

[Andrew Burgess]

These two commits fix some regressions relating to expression
evaluation involving operations of type STRUCTOP_PTR and
STRUCTOP_STRUCT.

The regressions were introduced in commit ac1ca910d74d, where values
with an lval type of not_lval are created in 3 places.  A later
commit, 2520f728b710, fixed one of these places, setting an
appropriate lval value, but left the other two.  These two commits fix
the last two occurences.

I've split the changes into two patches.  The first change is in
generic expression evaluation, and is covered by the new tests I've
added in that patch.  The tests will also cover the case fixed by
commit 2520f728b710 as well.

The second patch fixes what I am convinced will be a similar case in
OpenCL code.  However, I don't have access to an OpenCL compiler at
the moment, so I have not confirmed the bug, nor have I written any
tests.  If the second patch is rejected as untested that's fine, or if
someone with an OpenCL compiler would like to write a test, that would
be great.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb: Forward VALUE_LVAL when avoiding side effects for STRUCTOP_PTR
  gdb: Forward VALUE_LVAL when avoiding side effects for STRUCTOP_STRUCT

 gdb/ChangeLog                     | 12 ++++++
 gdb/eval.c                        |  2 +-
 gdb/opencl-lang.c                 |  2 +-
 gdb/testsuite/ChangeLog           |  5 +++
 gdb/testsuite/gdb.base/whatis.c   |  8 ++--
 gdb/testsuite/gdb.base/whatis.exp | 83 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 106 insertions(+), 6 deletions(-)

-- 
2.5.1

Re: [PATCH 0/2] Forward VALUE_LVAL for STRUCT/STRUCT-PTR expressions

[Pedro Alves]

On 05/25/2016 09:10 AM, Andrew Burgess wrote:
> These two commits fix some regressions relating to expression
> evaluation involving operations of type STRUCTOP_PTR and
> STRUCTOP_STRUCT.
> 
> The regressions were introduced in commit ac1ca910d74d, where values
> with an lval type of not_lval are created in 3 places.  A later
> commit, 2520f728b710, fixed one of these places, setting an
> appropriate lval value, but left the other two.  These two commits fix
> the last two occurences.
> 
> I've split the changes into two patches.  The first change is in
> generic expression evaluation, and is covered by the new tests I've
> added in that patch.  The tests will also cover the case fixed by
> commit 2520f728b710 as well.
> 
> The second patch fixes what I am convinced will be a similar case in
> OpenCL code.  However, I don't have access to an OpenCL compiler at
> the moment, so I have not confirmed the bug, nor have I written any
> tests.  If the second patch is rejected as untested that's fine, or if
> someone with an OpenCL compiler would like to write a test, that would
> be great.

Both patches are OK.  

One nit -- when referring to a git commit hash, please write it
like '... commit 2520f728b710 ($commit-subject) ...' so that we know
what you're talking about without having to go look it up.  Please
update the commit logs, and you're good to go.

Thanks,
Pedro Alves