Re: assertion failure in regcache.c

[Andrew Cagney <ac131313 at redhat dot com>]

> Kris Warkentin writes:
>  > >  > I'd start with the obvious thing - a simple tipo in the SH4 register
>  > >  > byte function.  The code was written long before these sanity checks
>  > >  > were added and ``the old way'' makes it very hard to notice that the
>  > >  > values are skewed.
>  > >  >
>  > >  > Andrew
>  > >  >
>  > >
>  > >
>  > > yes, look at sh_sh4_register_byte. Maybe FV0_REGNUM or FV_LAST_REGNUM
>  > > are not set correctly or fv_reg_base_num does something wrong. These
>  > > registers with (*1) are pseudo registers, so it's easy that the
>  > > calculations could have been screwed up.
>  > 
>  > Well, I found the disagreement.  It looks to me like
>  > regcache->descr->register_offset[] is pointing to an upwardly growing list
>  > of registers including the pseudo-registers.  So you get something like dr5
>  > being 260 in the register_offset array but sh4_register_byte will return 124
>  > which would be the offset of fr10 (taking into account that dr0 is overlaid
>  > on top of the fr regs).  I'm inclined to think that the regcache way is
>  > wrong since someone who updates dr0 and then reads fr0 will get conflicting
>  > values.  We shouldn't be storing extra copies of the same register.

Plan B.  Try not setting [deprecated_]register_byte.

> Looking at regcache.c I see that the long term goal is to not allocate
> space in the regcache for the PSEUDOs. But in the meantime, 
>         descr->register_offset[i] = REGISTER_BYTE (i); 
> in the legacy init function, while
> 	descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]);
> 	descr->register_offset[i] = offset;
> 	offset += descr->sizeof_register[i];
> in the new version of the function.
>
> So the mismatch seems to come from the TYPE_LENGTH() on the type of a
> pseudo, because that's always a positive quantity, while the
> REGISTER_BYTE points 'backwards'. Maybe we should be using the legacy
> version of the regcache init function? Is that doable?

Are we all sitting comfortably?  I'll try to explain the history to this 
... :-)

Long ago, the code for reading/writing register values looked like (this 
was burried in read_register_bytes):

  extract_integer (&registers[value->address], value->length);

That is, given $dr5, gdb would create a value the location of which was 
described by:

	location = lval_register;
	regnum = $dr5 regnum
	address = REGISTER_BYTE ($dr5 regnum)

and then, assuming that the register cache was one big byte array, and 
that the value was already in that array, and that they array only held 
raw registers, use the ADDRESS and not the REGNUM to extract the value's 
raw bytes.

This forced the SH (and a few other architectures) to directly map 
pseudo registers onto the raw register range (to guarentee that the 
value read was always valid).

Fortunatly, since then problems like this have (hopefully) all been 
identified and fixed.  It uses a read / modify / write to access the values:

	- map the [value->address, value->length) back to a sequence of 
registers (identified by their register number).  Here, they would be 
pseudo registers
	- read, using regcache_cooked_read, those registers and contatenate 
them into a large buffer
	- ``modify'' the bytes implied by [value->address, value->length)
	- write, using regcache_cooked_write, each of the registers back into 
the regcache

The procedural interface means that something like $dr5 is always 
constructed / updated on the fly (using the relevant raw registers), and 
potential problems with out-dated register values are avoided.

In the end, GDB doesn't store multiple values of the same register, but 
it can end up caching the various values.  That doesn't matter, GDB 
flushes all such cached values after any target modify.

enjoy,
Andrew