From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26569 invoked by alias); 6 Oct 2010 18:59:31 -0000 Received: (qmail 26554 invoked by uid 22791); 6 Oct 2010 18:59:30 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate5.de.ibm.com (HELO mtagate5.de.ibm.com) (195.212.17.165) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 06 Oct 2010 18:59:23 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate5.de.ibm.com (8.13.1/8.13.1) with ESMTP id o96IxKXL003070 for ; Wed, 6 Oct 2010 18:59:20 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o96IxKcO3948646 for ; Wed, 6 Oct 2010 20:59:20 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id o96IxJOx012689 for ; Wed, 6 Oct 2010 20:59:19 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id o96IxIMF012683; Wed, 6 Oct 2010 20:59:18 +0200 Message-Id: <201010061859.o96IxIMF012683@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 06 Oct 2010 20:59:18 +0200 Subject: [rfc] Fix value_assign return value (Re: [patch] fix pre-/post- in-/decrement) To: dan@codesourcery.com (Daniel Jacobowitz) Date: Wed, 06 Oct 2010 18:59:00 -0000 From: "Ulrich Weigand" Cc: ken@linux.vnet.ibm.com (Ken Werner), gdb-patches@sourceware.org, brobecker@adacore.com (Joel Brobecker), vladimir@codesourcery.com (Vladimir Prus) In-Reply-To: <20101005134200.GJ6985@caradoc.them.org> from "Daniel Jacobowitz" at Oct 05, 2010 09:42:01 AM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-10/txt/msg00082.txt.bz2 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