On Friday, July 09, 2010 03:12:01 pm Ulrich Weigand wrote:
> 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

Hello,
thanks for the analysis and sorry for the confusion. I accidentally removed 
the else body.

But while we are at it. Are there any objections on copying the contents to 
the destination instead of creating a pointer?

The attached patch removes both calls with no regressions on powerpc64-*-
linux-gnu and i686-*-linux-gnu.

Regards
-ken