Re: [patch] fix pre-/post- in-/decrement

[Ken Werner <ken@linux.vnet.ibm.com>]

[-- Attachment #1: Type: Text/Plain, Size: 843 bytes --]

On Monday, October 04, 2010 9:46:50 pm Ken Werner wrote:
> The attached patch introduces a new function called value_non_lval that
> returns a non-lval version of the given value. This function is called
> prior the simple assignment, compound assignment and pre/postfix routines
> return. Any comments are appreciated.

I overlooked that in case of C++ the various assignment, pre-increment and  
pre-decrement operators return lvalues while they return non-lvalues for C.
I've updated the patch to respect this. 
The patch also depends on non-lazy return values:
http://sourceware.org/ml/gdb-patches/2010-10/msg00082.html since calls of 
value_non_lval may trigger unnecessary reads from the target as mentioned 
here:
http://sourceware.org/ml/gdb-patches/2010-10/msg00030.html .
Tested on i686-*-linux-gnu with no regressions.

Regards
Ken

[-- Attachment #2: pre_post_in_decrement.patch --]
[-- Type: text/x-patch, Size: 5860 bytes --]

ChangeLog:

2010-10-07  Ken Werner  <ken.werner@de.ibm.com>

	* value.h (value_non_lval): Declare.
	* value.c (value_non_lval): New function.
	* eval.c (evaluate_subexp_standard) <BINOP_ASSIGN, BINOP_ASSIGN_MODIFY,
	UNOP_PREINCREMENT, UNOP_PREDECREMENT>: Call value_non_lval to ensure to
	return a non-lvalue if the source language is not C++.
	UNOP_POSTINCREMENT, UNOP_POSTDECREMENT>: Call value_non_lval to ensure
	to return a non-lvalue.

testsuite/ChangeLog:

2010-10-07  Ken Werner  <ken.werner@de.ibm.com>

	* gdb.base/exprs.exp: Add tests for pre-/post- in-/decrement operators.


Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.139
diff -p -u -r1.139 eval.c
--- gdb/eval.c	11 Aug 2010 16:48:26 -0000	1.139
+++ gdb/eval.c	7 Oct 2010 09:22:40 -0000
@@ -2009,8 +2009,10 @@ evaluate_subexp_standard (struct type *e
 	return arg1;
       if (binop_user_defined_p (op, arg1, arg2))
 	return value_x_binop (arg1, arg2, op, OP_NULL, noside);
-      else
+      else if (exp->language_defn->la_language == language_cplus)
 	return value_assign (arg1, arg2);
+      else
+	return value_non_lval (value_assign (arg1, arg2));
 
     case BINOP_ASSIGN_MODIFY:
       (*pos) += 2;
@@ -2043,7 +2045,10 @@ evaluate_subexp_standard (struct type *e
 
 	  arg2 = value_binop (tmp, arg2, op);
 	}
-      return value_assign (arg1, arg2);
+      if (exp->language_defn->la_language == language_cplus)
+	return value_assign (arg1, arg2);
+      else
+	return value_non_lval (value_assign (arg1, arg2));
 
     case BINOP_ADD:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
@@ -2702,7 +2707,10 @@ evaluate_subexp_standard (struct type *e
 	      arg2 = value_binop (tmp, arg2, BINOP_ADD);
 	    }
 
-	  return value_assign (arg1, arg2);
+	  if (exp->language_defn->la_language == language_cplus)
+	    return value_assign (arg1, arg2);
+	  else
+	    return value_non_lval (value_assign (arg1, arg2));
 	}
 
     case UNOP_PREDECREMENT:
@@ -2726,7 +2734,10 @@ evaluate_subexp_standard (struct type *e
 	      arg2 = value_binop (tmp, arg2, BINOP_SUB);
 	    }
 
-	  return value_assign (arg1, arg2);
+	  if (exp->language_defn->la_language == language_cplus)
+	    return value_assign (arg1, arg2);
+	  else
+	    return value_non_lval (value_assign (arg1, arg2));
 	}
 
     case UNOP_POSTINCREMENT:
@@ -2739,6 +2750,8 @@ evaluate_subexp_standard (struct type *e
 	}
       else
 	{
+	  arg3 = value_non_lval (arg1);
+	  
 	  if (ptrmath_type_p (exp->language_defn, value_type (arg1)))
 	    arg2 = value_ptradd (arg1, 1);
 	  else
@@ -2751,7 +2764,7 @@ evaluate_subexp_standard (struct type *e
 	    }
 
 	  value_assign (arg1, arg2);
-	  return arg1;
+	  return arg3;
 	}
 
     case UNOP_POSTDECREMENT:
@@ -2764,6 +2777,8 @@ evaluate_subexp_standard (struct type *e
 	}
       else
 	{
+	  arg3 = value_non_lval (arg1);
+	  
 	  if (ptrmath_type_p (exp->language_defn, value_type (arg1)))
 	    arg2 = value_ptradd (arg1, -1);
 	  else
@@ -2776,7 +2791,7 @@ evaluate_subexp_standard (struct type *e
 	    }
 
 	  value_assign (arg1, arg2);
-	  return arg1;
+	  return arg3;
 	}
 
     case OP_THIS:
Index: gdb/value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.113
diff -p -u -r1.113 value.c
--- gdb/value.c	30 Sep 2010 18:58:07 -0000	1.113
+++ gdb/value.c	7 Oct 2010 09:22:40 -0000
@@ -826,6 +826,26 @@ value_copy (struct value *arg)
   return val;
 }
 
+/* Return a version of ARG that is non-lvalue.  */
+
+struct value *
+value_non_lval (struct value *arg)
+{
+  if (VALUE_LVAL (arg) != not_lval)
+    {
+      struct type *enc_type = value_enclosing_type (arg);
+      struct value *val = allocate_value (enc_type);
+  
+      memcpy (value_contents_all_raw (val), value_contents_all (arg),
+	      TYPE_LENGTH (enc_type));
+      val->type = arg->type;
+      set_value_embedded_offset (val, value_embedded_offset (arg));
+      set_value_pointed_to_offset (val, value_pointed_to_offset (arg));
+      return val;
+    }
+   return arg;
+}
+
 void
 set_value_component_location (struct value *component,
 			      const struct value *whole)
Index: gdb/value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.161
diff -p -u -r1.161 value.h
--- gdb/value.h	7 Jul 2010 16:15:18 -0000	1.161
+++ gdb/value.h	7 Oct 2010 09:22:41 -0000
@@ -710,6 +710,8 @@ extern void preserve_values (struct objf
 
 extern struct value *value_copy (struct value *);
 
+extern struct value *value_non_lval (struct value *);
+
 extern void preserve_one_value (struct value *, struct objfile *, htab_t);
 
 /* From valops.c */
Index: gdb/testsuite/gdb.base/exprs.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/exprs.exp,v
retrieving revision 1.19
diff -p -u -r1.19 exprs.exp
--- gdb/testsuite/gdb.base/exprs.exp	10 Jun 2010 19:48:19 -0000	1.19
+++ gdb/testsuite/gdb.base/exprs.exp	7 Oct 2010 09:22:41 -0000
@@ -253,3 +253,14 @@ gdb_test "set output-radix 8" ".*"
 test_expr "print red" "\\$\[0-9\]* = red"
 test_expr "print/d red" "\\$\[0-9\]* = 0"
 gdb_test "set output-radix 10" ".*"
+
+# Pre-/post in-/decrement tests.
+gdb_test "set variable v_int = 1" ""
+gdb_test "print v_int++" "\\$\[0-9\]* = 1"
+gdb_test "print ++v_int" "\\$\[0-9\]* = 3"
+gdb_test "print v_int--" "\\$\[0-9\]* = 3"
+gdb_test "print --v_int" "\\$\[0-9\]* = 1"
+gdb_test "print v_int++ = 5" "Left operand of assignment is not an lvalue."
+gdb_test "print ++v_int = 5" "Left operand of assignment is not an lvalue."
+gdb_test "print v_int-- = 5" "Left operand of assignment is not an lvalue."
+gdb_test "print --v_int = 5" "Left operand of assignment is not an lvalue."