Re: [patch] Small fix for assigning values to vectors

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

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