From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9440 invoked by alias); 9 Jul 2010 13:12:12 -0000 Received: (qmail 9431 invoked by uid 22791); 9 Jul 2010 13:12:11 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate2.de.ibm.com (HELO mtagate2.de.ibm.com) (195.212.17.162) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 09 Jul 2010 13:12:06 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate2.de.ibm.com (8.13.1/8.13.1) with ESMTP id o69DC3qk019485 for ; Fri, 9 Jul 2010 13:12:03 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 o69DC3PW1790068 for ; Fri, 9 Jul 2010 15:12:03 +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 o69DC2nc013535 for ; Fri, 9 Jul 2010 15:12:03 +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 o69DC1TX013390; Fri, 9 Jul 2010 15:12:01 +0200 Message-Id: <201007091312.o69DC1TX013390@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 09 Jul 2010 15:12:01 +0200 Subject: Re: [patch] Small fix for assigning values to vectors To: ken@linux.vnet.ibm.com (Ken Werner) Date: Fri, 09 Jul 2010 13:12:00 -0000 From: "Ulrich Weigand" Cc: dan@codesourcery.com (Daniel Jacobowitz), gdb-patches@sourceware.org In-Reply-To: <201007091238.57599.ken@linux.vnet.ibm.com> from "Ken Werner" at Jul 09, 2010 12:38:57 PM 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-07/txt/msg00180.txt.bz2 Ken Werner wrote: > Ok, the attached patch removes coerce_array call as well. Tested on powerpc64- > *-linux-gnu and i686-*-linux-gnu, no regressions. Hmm, actually I was thinking of the value_coerce_to_target call on the *destination*, not the coerce_array on the source. However, it is in fact interesting to see that the coerce_array call seems to have no effects on the testsuite either ... The current code reads: type = value_type (toval); if (VALUE_LVAL (toval) != lval_internalvar) { toval = value_coerce_to_target (toval); fromval = value_cast (type, fromval); } else { /* Coerce arrays and functions to pointers, except for arrays which only live in GDB's storage. */ if (!value_must_coerce_to_target (fromval)) fromval = coerce_array (fromval); } which distinguishes between assignments to a GDB internal variable and some other assignment destination. 1.) For usual destinations (except GDB internal variables), the destination has a specified type, and we need to cast FROMVAL to that destination type before assignment. 2.) In addition, for usual destinations, if the destination currently resides in GDB memory only, it is forced to target memory. This is the call to value_coerce_to_target in the "if" part. 3.) Finally, if we assign to a GDB internal variable, the variable has no type in and of itself, but assumes the type of whatever is assigned to it; therefore we do not need to type-cast the source. However, the code will (sometimes) convert an array to a pointer. Now, I think (1) is clearly required. As to (2), this is what my original mail was refering to. If the destination is not an internal variable, but something that temporarily resides in GDB memory (e.g. as the result of GDB constructing a string literal), assigning to it would normally fail because the destination is not an lvalue. However, due to that value_coerce_to_target call, the old contents of the destination will be copied to some random location in target memory, and immediately afterwards overwritten by the new contents. (And then the pointer to that target memory will most likely be forgotten anyway.) This seems not really useful to me, therefore I'd suggested to remove that call. Now as to (3), which you just removed, this also seems questionable. The effect of the coerce_array call is that if you assign an array to an internal variable, the full array contents are not copied into the GDB representation of the internal variable; instead, only a pointer is stored. However, this seems to conflict with another goal of internal variables: see e.g. this comment in set_internalvar: /* Force the value to be fetched from the target now, to avoid problems later when this internalvar is referenced and the target is gone or has changed. */ The code in value_assign directly conflicts with the code in set_internalvar in that respect. It seems to me that it does indeed make sense to fetch internal variable contents completely, to allow to preserve them once the target goes away. (If the user want to specifically store a pointer in the internal variable for some reason, they're still free to explicitly use the & operator anyway.) So in summary, it's good that you verified (3) can go away with no testsuite regressions. Could you in addition check whether (2) can *also* go away? Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com