This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[rfc] Fix value_assign return value (Re: [patch] fix pre-/post- in-/decrement)
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: dan at codesourcery dot com (Daniel Jacobowitz)
- Cc: ken at linux dot vnet dot ibm dot com (Ken Werner), gdb-patches at sourceware dot org, brobecker at adacore dot com (Joel Brobecker), vladimir at codesourcery dot com (Vladimir Prus)
- Date: Wed, 6 Oct 2010 20:59:18 +0200 (CEST)
- Subject: [rfc] Fix value_assign return value (Re: [patch] fix pre-/post- in-/decrement)
Daniel Jacobowitz wrote:
> On Tue, Oct 05, 2010 at 03:28:19PM +0200, Ulrich Weigand wrote:
> > "Assignments are also expressions and have an rvalue. However when assigning
> > to a scalar volatile, the volatile object is not reread, regardless of whether
> > the assignment expression's rvalue is used or not. If the assignment's rvalue
> > is used, the value is that assigned to the volatile object. [...] If you need
> > to read the volatile object after an assignment has occurred, you must use a
> > separate expression with an intervening sequence point."
>
> > To reduce the potential for confusion, it seems to me GDB ought to mirror
> > that behavior as well ...
>
> Hmm.
>
> It seems to me that this is a disruptive change for us, because the "a
> = b" case is more likely to be used than anything fancy (a += b, a = b
> = c). If we change it, we should document the change.
Sure. The patch below implements fixes to the various problems w.r.t.
the return value of value_assign that I noticed:
- Make sure the returned value is non-lazy (this is what we discussed above).
This includes a proposed NEWS entry -- feel free to suggest improved
wordings. I don't think we need to update the actual documentation,
which already reads (in 15.4.1.1): "The value of an assignment expression
is the value assigned."
- In the case of assignments to a value of C++ class type, the code sets
the enclosing type / embedded offset of the result value to reflect
FROMVAL. This is simply wrong; assignment to an object does not change
its dynamic type. The patch removes this, but keeps changing the
enclosing type / pointed-to offset in the case of C++ object *pointers*.
- In the vase of assignment to internal variables, the existing code was
simply incorrect (it returned a copy of "from", which means subsequent
assignments no longer change the internal variable). The patch fixes
this by simply creating a new value for the updated internalvar using
value_of_internalvar.
Tested on i386-linux with no regressions.
Any comments are welcome ...
Bye,
Ulrich
ChangeLog:
* valops.c (value_assign): Returned value is never lazy. If a
C++ class type is returned, fix incorrect enclosing type / embedded
offset. If internal variable is returned, allocate new internalvar
value using value_of_internalvar.
* NEWS: Document changes in behavior of "print x = 0" and similar
expressions.
Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.405
diff -u -p -r1.405 NEWS
--- gdb/NEWS 13 Sep 2010 20:37:34 -0000 1.405
+++ gdb/NEWS 6 Oct 2010 18:41:26 -0000
@@ -23,6 +23,12 @@
feature requires proper debuginfo support from the compiler; it
was added to GCC 4.5.
+* GDB now follows GCC's rules on accessing volatile objects when
+ reading or writing target state during expression evaluation.
+ One notable difference to prior behavior is that "print x = 0"
+ no longer generates a read of x; the value of the assignment is
+ now always taken directly from the value being assigned.
+
* GDB now has some support for using labels in the program's source in
linespecs. For instance, you can use "advance label" to continue
execution to a label.
Index: gdb/valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.251
diff -u -p -r1.251 valops.c
--- gdb/valops.c 24 Sep 2010 14:47:53 -0000 1.251
+++ gdb/valops.c 6 Oct 2010 18:41:26 -0000
@@ -1099,13 +1099,8 @@ value_assign (struct value *toval, struc
{
case lval_internalvar:
set_internalvar (VALUE_INTERNALVAR (toval), fromval);
- val = value_copy (fromval);
- val = value_change_enclosing_type (val,
- value_enclosing_type (fromval));
- set_value_embedded_offset (val, value_embedded_offset (fromval));
- set_value_pointed_to_offset (val,
- value_pointed_to_offset (fromval));
- return val;
+ return value_of_internalvar (get_type_arch (type),
+ VALUE_INTERNALVAR (toval));
case lval_internalvar_component:
set_internalvar_component (VALUE_INTERNALVAR (toval),
@@ -1291,14 +1286,23 @@ value_assign (struct value *toval, struc
fromval = value_from_longest (type, fieldval);
}
+ /* The return value is a copy of TOVAL so it shares its location
+ information, but its contents are updated from FROMVAL. This
+ implies the returned value is not lazy, even if TOVAL was. */
val = value_copy (toval);
+ set_value_lazy (val, 0);
memcpy (value_contents_raw (val), value_contents (fromval),
TYPE_LENGTH (type));
- deprecated_set_value_type (val, type);
- val = value_change_enclosing_type (val,
- value_enclosing_type (fromval));
- set_value_embedded_offset (val, value_embedded_offset (fromval));
- set_value_pointed_to_offset (val, value_pointed_to_offset (fromval));
+
+ /* We copy over the enclosing type and pointed-to offset from FROMVAL
+ in the case of pointer types. For object types, the enclosing type
+ and embedded offset must *not* be copied: the target object refered
+ to by TOVAL retains its original dynamic type after assignment. */
+ if (TYPE_CODE (type) == TYPE_CODE_PTR)
+ {
+ val = value_change_enclosing_type (val, value_enclosing_type (fromval));
+ set_value_pointed_to_offset (val, value_pointed_to_offset (fromval));
+ }
return val;
}
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com