Re: [rfc] Fix bitfield regressions on 64-bit big-endian targets

[Daniel Jacobowitz <drow at false dot org>]

On Sun, Sep 27, 2009 at 11:48:34PM +0200, Ulrich Weigand wrote:
> Daniel Jacobowitz wrote:
> 
> > value_offset for a bitfield is an offset to be added into the parent's
> > contents.  So including the parent's offset is incorrect; the attached
> > test shows that we were reading the wrong location.  With large enough
> > offsets, we could wander off to another page of memory and fault.
> 
> This changes the value_offset for a bitfield, but does not adapt all
> places where the offset is used; in particular value_assign still does
> not take the parent's offset into account.
> 
> This causes a significant number of store.exp testsuite failures on
> s390x-linux and ppc64-linux (and presumably other 64-bit big-endian
> platforms).
> 
> The following patch updates value_assign to respect the parent offset,
> which fixes all those failures.
> 
> Tested on s390(x)-linux and ppc(64)-linux with no regressions.
> 
> Does this look OK to you?

It looks like the code you're fixing was completely bogus.

> !             && ((LONGEST) value_address (toval) % TYPE_LENGTH (type)) == 0)

What does that even mean?  We set v->offset, both before and after the
patch you're replying to, but we never set value->location.address.
Are we only testing this in registers somehow where no address was
required?  Or am I missing where the location was set?

Your patch looks fine to me.

-- 
Daniel Jacobowitz
CodeSourcery