Re: FYI: fix big-endian bug with DWARF_VALUE_STACK

["Ulrich Weigand" <uweigand@de.ibm.com>]

Tom Tromey wrote:

> 2010-02-23  Tom Tromey  <tromey@redhat.com>
> 
> 	* dwarf2loc.c (read_pieced_value) <DWARF_VALUE_STACK>: Correctly
> 	handle big-endian values.
> 	(dwarf2_evaluate_loc_desc) <DWARF_VALUE_STACK>: Likewise.

I'd recently been wondering about this piece of code (and in particular
its use of gdbarch_addr_bit) as well ...


> @@ -298,16 +298,14 @@ read_pieced_value (struct value *v)
>  
>  	case DWARF_VALUE_STACK:
>  	  {
> -	    gdb_byte bytes[sizeof (ULONGEST)];
>  	    size_t n;
>  	    int addr_size = gdbarch_addr_bit (c->arch) / 8;
> -	    store_unsigned_integer (bytes, addr_size,
> -				    gdbarch_byte_order (c->arch),
> -				    p->v.expr.value);
>  	    n = p->size;
>  	    if (n > addr_size)
>  	      n = addr_size;
> -	    memcpy (contents + offset, bytes, n);
> +	    store_unsigned_integer (contents + offset, n,
> +				    gdbarch_byte_order (c->arch),
> +				    p->v.expr.value);
>  	  }
>  	  break;

The original code looks broken to me too.  But even the new code
doesn't seem quite right: if the requested piece size p->size
is larger than addr_size, the remaining bytes of the output value
are left undefined (well, probably zeroed -- but that still doesn't
look correct on a big-endian machine ...).

Why would that code look at gdbarch_addr_bit at all?  I think this
should simply do something like:

	    store_unsigned_integer (contents + offset, p->size,
				    gdbarch_byte_order (c->arch),
				    p->v.expr.value);

> @@ -476,19 +474,17 @@ dwarf2_evaluate_loc_desc (struct symbol *var, struct frame_info *frame,
>  
>  	case DWARF_VALUE_STACK:
>  	  {
> -	    gdb_byte bytes[sizeof (ULONGEST)];
>  	    ULONGEST value = (ULONGEST) dwarf_expr_fetch (ctx, 0);
>  	    bfd_byte *contents;
>  	    size_t n = ctx->addr_size;
>  
> -	    store_unsigned_integer (bytes, ctx->addr_size,
> -				    gdbarch_byte_order (ctx->gdbarch),
> -				    value);
>  	    retval = allocate_value (SYMBOL_TYPE (var));
>  	    contents = value_contents_raw (retval);
>  	    if (n > TYPE_LENGTH (SYMBOL_TYPE (var)))
>  	      n = TYPE_LENGTH (SYMBOL_TYPE (var));
> -	    memcpy (contents, bytes, n);
> +	    store_unsigned_integer (contents, n,
> +				    gdbarch_byte_order (ctx->gdbarch),
> +				    value);
>  	  }
>  	  break;

Similiary here; why does this not simply use value_from_longest?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com