Re: assertion failure in regcache.c

Elena Zannoni
2003-05-23T20:33:24Z

Kris Warkentin writes:
> Interesting. If I go through sh-tdep.c and comment out all the
> 'set_gdbarch_register_byte(blah)' calls, it works. Are there any potential
> negative implications of this or can we just trust regcache to do it's job?
>

Oh my. I have fainted. It works? :-) You mean all that pseudo
register stuff written 2+ years back still works with all the register
changes? I really think that working on gdb is just like the Boston
Big Dig.

What exactly do you mean by it works? Testsuite failures approach
acceptable levels? This is only sh4, right? There are so many variants
to test. The really scary one would be sh64.

elena

> cheers,
>
> Kris
>
> ----- Original Message -----
> From: "Andrew Cagney" <ac131313@redhat.com>
> To: "Elena Zannoni" <ezannoni@redhat.com>; "Kris Warkentin"
> <kewarken@qnx.com>
> Cc: "Gdb@Sources.Redhat.Com" <gdb@sources.redhat.com>
> Sent: Friday, May 23, 2003 3:46 PM
> Subject: Re: assertion failure in regcache.c
>
>
> > > 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
> >
> >
> >