Re: RFC: fix bug in pieced value with offset

[Tom Tromey <tromey@redhat.com>]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Thinking now if the BFD_ENDIAN_BIG patch by Ulrich Weigand
Jan> 	[rfc] Handle DWARF-2 value pieces residing in *parts* of a register
Jan> 	http://sourceware.org/ml/gdb-patches/2009-12/msg00305.html
Jan> should not have been applied also for DWARF_VALUE_STACK; but this
Jan> is outside of the scope of this patch.

I must be missing something... I don't see any change to
DWARF_VALUE_STACK there.

>> -             && p->size < register_size (arch, gdb_regnum))
>> +             && this_size + reg_offset <= register_size (arch, gdb_regnum))

Jan> I believe it should be instead:
Jan> # +	      reg_offset = (register_size (arch, gdb_regnum)
Jan> # +			    - this_size);

Jan> As we should ignore source_offset bytes from the start of register.
Jan> register_size = 8
Jan> p-> size = 4
Jan> bytes_to_skip = for example 1
Jan> =>
Jan> source_offset = 1
Jan> this_size = 3

Jan> From the register occupying bytes <0..7> we thus want to read-in
Jan> bytes <5..7>.

My thinking was to consider the resulting contents as a sequence of
bytes.  In this case the register would be laid out from high byte to
low byte.  The existing 'size' offsetting strips off high bytes (because
it is conceptually value-based); but then for 'source_offset' we want to
advance through the byte representation -- so, also skipping high bytes.

I can't tell if this makes sense or not.

>> case DWARF_VALUE_STACK:
>> {
>> struct gdbarch *gdbarch = get_type_arch (value_type (v));
>> -	    size_t n = p->size;
>> +	    size_t n = this_size;
>> if (n > c->addr_size)
>> n = c->addr_size;

Jan> Generally I would prefer more sanity checks there instead of quiet data
Jan> cutting.

In this particular case, I think this is just what DWARF specifies.
I think it makes sense to compute a value on the stack and then just
select some bits from it.

Maybe for DWARF_VALUE_LITERAL it would make sense to issue a complaint
if the piece is smaller than the literal.  That would be strange
compiler output.

Jan> There is also missing `- source_offset':
Jan> #  	    if (n > c->addr_size - source_offset)
Jan> #  	      n = c->addr_size - source_offset;

Thanks for this and the other similar things.

Jan> Why weren't just simple "main" and standard compilation used?  It
Jan> works for me.

I picked a bad example to copy.  I will fix this up.

Jan> Isn't missing the ubiquitous FSF copyleft header?

And this.

thanks,
Tom