Re: [RFA] regcache.c (register_fetched) + related changes

Andrew Cagney
2001-03-13T08:52:00Z

To follow this up, David went through the problem in detail by phone. I
belive the below describes the underlying problem.

Consider an architecture with three registers:

$s0 $s1
$d0

where $d0 is made up of both $s0 and $s1. Using the current code, the
developer would lay this out as:

REGNUM name size offset
0 s0 1 0
1 s1 1 1
then the pseudo as either
2 d0 2 0
or
2 d0 2 2

The choice is pretty arbitrary. Under my proposed changes to make
everything bound to a frame, the user would instead lay it out as:

RAWNUM rawname size
0 rs0 1
1 rs1 1

and then the frame/cooked registers would be arranged as:

REGNUM name size
0 s0 1
1 s1 1
2 d0 2

and d0 would be implemented as:

read/write rs0
read/write rs1

Anyway....

The user enters:

(gdb) set $d0 = value

GDB creates an expression tree that contains an LVAL with (at a guess)
the following:

register number
register size
register address (in regcache or in memory)

this infomation is then used (valops.c) to write the register value
using write_register_bytes().

[Side note one: Previous discussion has agreed that
write_register_bytes() is bad. The above should be changed so that it
stores the location of the register in some less implementation specific
way. Fixing this little nastyness is part of binding everything to a
frame.]

write_register_bytes() iterates through all the registers (real and
pseudo) looking to see where those bytes need to go. In michaels case,
that code would generate one or more calls (depending on how the
registers are layed out). One of the calls would be:

write_register_gen (d1regnum, buffer);

Write register gen, then writes bytes into the pseudo register, set
register_valid[] for the pseudo register and finally calls:

store_register (d1regnum);

The immediate problem is that the pseudo registers register_valid[] bit
has been set. That is a pseudo register has been marked as valid in the
register cache.

This becomes a problem when a user goes to again display that or one of
the other registers affected by that pseudo. For instance consider:

(gdb) set $d0 = 22
(gdb) set $s1 = 1
(gdb) set $s0 = 0
(gdb) print $d0

If you follow the register read side through, you'll find that $d0 can
potentially become inconsistent with s0/s1 because $d0 is considered to
already be valid.

The obvious fix is to modify store_register() so that it clears
register_valid[] and thus ensures that D1 is never valid in the cache.
(an invalid register is always fetched/computed).

Discussion with David suggests a better fix - one that is more in line
with with the direction that the register interface is going - catch
writes to pseudos before they hit the cache so that the inferior-arch
can redirect them as needed.

The first bit of the change is to modify write_register_gen() to ask the
architecture to perform the write vis:

write_register_gen (regnum, buf)
gdbarch_write_register_gen (regnum, buf)

In Davids case, write register gen would look like:

if (regnum < NUM_REGS)
regcache_write (regnum, buff)
else if (regnum == d0regnum)
regcache_write (s0regnum, buff)
regcache_write (s1regnum, buff)

regcache_write() would only allow writes to 0 .. NUM_REG. Legacy code
would use a compatiblity function that would also allow writes to
NUM_REG .. NUM_REG+PSEUDO_NUM_REG. Sane code would just set
gdbarch_write_register_gen() to regcache_write

Of course, life isn't that simple. One additional change is to
write_register() which should be calling write_register_bytes(). This
also avoids the write_register_bytes() problem and completly avoids
read_register_bytes() :-(

However, on the bright side. This does mean that Davids immediate
problem is addressed _and_ the need to blat register_valid[] is
avoided. As an additional bonus, this pushes the code in the general
direction of separate cooked and raw registers. In the future, the
above gdbarch_write_register_gen would become part of a write
frame-register function.

Andrew