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

[Ken Werner <ken@linux.vnet.ibm.com>]

[-- Attachment #1: Type: Text/Plain, Size: 1609 bytes --]

On Wednesday, July 07, 2010 08:26:10 pm Daniel Jacobowitz wrote:
> On Wed, Jul 07, 2010 at 07:50:14PM +0200, Ulrich Weigand wrote:
> > Ken Werner wrote:
> > > As can be seen the GDB behaves incorrect for vector types. A quick look
> > > to the valops.c:value_assign function shows that
> > > value_coerce_to_target creates a value with lval set to lval_memory
> > > for array types (including vectors). The code was introduced with the
> > > following patch: http://sourceware.org/ml/gdb-
> > > patches/2008-03/msg00079.html. I have to admit that I do not entirely
> > > understand why value_coerce_to_target is called here.
> > 
> > Dan, do you recall why you added a value_coerce_to_target for the
> > *destination* of an assignment?  It seems this shouldn't really be
> > necessary ...
> 
> I'm not sure now.  We could try dropping it.

Ok, the attached patch removes coerce_array call as well. Tested on powerpc64-
*-linux-gnu and i686-*-linux-gnu, no regressions.

> > > @@ -1424,6 +1424,9 @@ value_must_coerce_to_target (struct valu
> > > 
> > >    valtype = check_typedef (value_type (val));
> > > 
> > > +  if (TYPE_VECTOR (valtype))
> > > +    return 0;
> > > +
> > > 
> > >    switch (TYPE_CODE (valtype))
> > >    
> > >      {
> > 
> > >      case TYPE_CODE_ARRAY:
> > Ken, this doesn't look quite right: the TYPE_VECTOR flag is defined only
> > for TYPE_CODE_ARRAY types; you should never look at TYPE_VECTOR for any
> > other type.

Fixed, thanks.

> > Otherwise, this looks reasonable to me ...
> 
> It does seem reasonable that a vector does not have to live in target
> memory.

Regards
-ken

[-- Attachment #2: vec_lval.patch --]
[-- Type: text/x-patch, Size: 2559 bytes --]

Changelog:

2010-07-09  Ken Werner  <ken.werner@de.ibm.com>

	* valops.c (value_assign): Remove coerce_array call.
	(value_must_coerce_to_target): Return 0 in case of TYPE_VECTOR.

testsuite/ChangeLog:

2010-07-09  Ken Werner  <ken.werner@de.ibm.com>

	* gdb.arch/altivec-abi.exp: New tests.


Index: gdb/valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.248
diff -p -u -r1.248 valops.c
--- gdb/valops.c	28 Jun 2010 20:35:52 -0000	1.248
+++ gdb/valops.c	9 Jul 2010 08:12:16 -0000
@@ -1083,13 +1083,6 @@ value_assign (struct value *toval, struc
       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);
-    }
 
   CHECK_TYPEDEF (type);
 
@@ -1427,6 +1420,7 @@ value_must_coerce_to_target (struct valu
   switch (TYPE_CODE (valtype))
     {
     case TYPE_CODE_ARRAY:
+      return TYPE_VECTOR (valtype) ? 0 : 1;
     case TYPE_CODE_STRING:
       return 1;
     default:
Index: gdb/testsuite/gdb.arch/altivec-abi.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/altivec-abi.exp,v
retrieving revision 1.19
diff -u -p -r1.19 altivec-abi.exp
--- gdb/testsuite/gdb.arch/altivec-abi.exp	2 Jul 2010 18:02:19 -0000	1.19
+++ gdb/testsuite/gdb.arch/altivec-abi.exp	6 Jul 2010 12:47:01 -0000
@@ -98,6 +98,16 @@ proc altivec_abi_tests { extra_flags for
     gdb_test "p vec_func(vshort_d,vushort_d,vint_d,vuint_d,vchar_d,vuchar_d,vfloat_d,x_d,y_d,a_d,b_d,c_d,intv_on_stack_d)" \
 	".\[0-9\]+ = .0, 0, 0, 0." "call inferior function with vectors (2)"
 
+    # Attempt to take address of the return value of vec_func.
+    gdb_test "p &vec_func(vshort,vushort,vint,vuint,vchar,vuchar,vfloat,x,y,a,b,c,intv_on_stack)" \
+	"Attempt to take address of value not located in memory." \
+	"Attempt to take address of the return value of vec_func"
+
+    # Attempt to assing a value to the return value of vec_func.
+    gdb_test "set variable vec_func(vshort,vushort,vint,vuint,vchar,vuchar,vfloat,x,y,a,b,c,intv_on_stack) = {0,1,2,3}" \
+	"Left operand of assignment is not an lvalue." \
+	"Attempt to assing a value to the return value of vec_func"
+
     # Let's step into the function, to see if the args are printed correctly.
     gdb_test "step" \
 	$pattern1 \