Re: [patch] GNU vector binop support

[Tom Tromey <tromey@redhat.com>]

>>>>> "Ken" == Ken Werner <ken@linux.vnet.ibm.com> writes:

Ken> This patch implements some binary operations for GNU vectors by
Ken> traversing through the vector and calling scalar_binop (the former
Ken> value_binop) element wise.

Thanks for tackling this.

This patch looks good to me.  I noticed a few nits, and one slightly
more serious problem.

Ken>  struct value *
Ken> -value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
Ken> +scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)

Should be static.

Ken> +struct value *
Ken> +vector_binop (struct value *val1, struct value *val2, enum exp_opcode op)

Likewise.

Ken> +  if (TYPE_CODE (eltype1) != TYPE_CODE (eltype2))
Ken> +    error (_("The vectors have different types"));
Ken> +
Ken> +  elsize =  TYPE_LENGTH (eltype1);

Extra space after the "=".

Ken> +  n = TYPE_LENGTH (type1) / elsize;
Ken> +  val = allocate_value (type1);
Ken> +
Ken> +  mark = value_mark ();
Ken> +  for (i = 0; i < n; i++)
Ken> +    {
Ken> +      tmp = value_binop (value_subscript (val1, i),
Ken> +			 value_subscript (val2, i), op);
Ken> +      memcpy (value_contents_writeable (val) + i * elsize,
Ken> +	      value_contents_all (tmp),
Ken> +	      elsize);

It seems to me that this could use some more checking.

The TYPE_CODEs could be equal but still give the wrong answer here.  For
example, 'int' and 'long' will have the same TYPE_CODE, but different
sizes.  Depending on the order of the addition, this could cause the
memcpy to overflow the destination buffer.

Ken> +set testfile "gnu_vector"
Ken> +set srcfile ${testfile}.c
Ken> +set binfile ${objdir}/${subdir}/${testfile}
Ken> +
Ken> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
Ken> +    untested "Couldn't compile ${srcfile}"
Ken> +    return -1
Ken> +}
Ken> +
Ken> +if [get_compiler_info ${binfile}] {
Ken> +    return -1
Ken> +}
Ken> +
Ken> +gdb_exit
Ken> +gdb_start
Ken> +gdb_reinitialize_dir $srcdir/$subdir
Ken> +gdb_load $binfile

I think most of this (except the get_compiler_info bit) can be replaced
with prepare_for_testing.

Tom