[RFC/mips] Problem with fetching/setting 32 bit registers

[Joel Brobecker <brobecker at adacore dot com>]

Hello,

I think the perfect person to ask this would have been Andrew, but
he's been away for a while now (good for him, though). Anyway, I think
others have a good knowledge of this port, so hopefully I can get some
help in solving my issue. I know what happens, and I have a patch that
gives me very results, but I think I did not fix it the proper way.

This is on mips-irix.

A customer of ours was debugging his program that was misfunctioning.
He was suspecting a float overflow. So he tried changing a flag in
the FSR register so that divisions leading to an infinity would trigger
a signal rather than generate an Inf value. That's when he discovered
that the FSR value read by GDB was incorrect, and that he was also
unable to change it.

To reproduce the problem, we have a small testcase. Sources are pasted
at the end of this message. For those who don't understand Ada, all it
does is basically do a float division by a very small number, leading
to an Inf result. What we're trying to do is check the FSR register
mask, modify it before the division is made, and verify that the
division now causes a SIGFPE signal to be raised.

This is what GDB does, using a recent version from CVS head:

        (gdb) start
        (gdb) p /x $fsr
 !!! -> $1 = 0x0
        (gdb) set $fsr := 0x01000e00
        (gdb) p /x $fsr
 !!! -> $2 = 0x0
        (gdb) c
        Continuing.
 !!! -> overflow did not crash..result:  +Inf****************
        
        Program exited normally.

For $1, the actual value is: 0x1000000. And for $2, we expect to
see the value we set (0x01000e00), not 0x0. And as a confirmation
that the value was incorrectly set we see the program running past
the division without any SIGFPE being raised.

I think this regression has been introduced when the mips port
was converted to the new register cache mechanism, where the
distinction between raw and cooked registers is made. It used
to work with 5.1.1 for instance (although not perfectly, GDB was
printing this register as a 64 bits value, which is not true).

What I found was that the problem was related to the fact that
the FSR register is 32 bits. So the cooked register type was set
to a 32 bits type, while underneath, the value provided by the
kernel is located in a 64 bits buffer. And looking close, I found
that we're reading and writing th wrong end of the buffer :-(.

Here is the relevant code using when reading a pseudo register
whose size is smaller than the size of the cooked register:

  else if (register_size (gdbarch, rawnum) >
           register_size (gdbarch, cookednum))
    {
      if (gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p
          || TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
        regcache_raw_read_part (regcache, rawnum, 0, 4, buf);
      else
        regcache_raw_read_part (regcache, rawnum, 4, 4, buf);

In our case (mips-irix), we have a big-endian byte order, and
therefore read the register value from the last 4 bytes, while
I found out that they are actually in the first 4.

The gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p check
gave me the idea of trying to set remote-mips64-transfers-32bit-regs,
and this gave me the expected result:

        (gdb) set remote-mips64-transfers-32bit-regs
        (gdb) start
        (gdb) info reg fsr
        fsr: 0x1000000
        (gdb) set $fsr := 0x01000e00
        (gdb) info reg fsr
        fsr: 0x1000e00
        (gdb) cont
        Continuing.

        Program received signal SIGFPE, Arithmetic exception.
        0x10004188 in hello () at hello.adb:7
        7         f := 1.0e200/value;

So this confirmed that we were reading the register value from
the wrong end.

But this is not the right fix, since this flag actually tells
GDB that all registers are 32 bit, which is wrong for mips-irix.
All general registers are 64 bits.

I think I understand the logic behind having deciding to use
a zero offset in the little-endian case while using the second
half for big-endian targets. And that suggests that mips-irix
is an exception to the usual rule.

I have tried the following change, just as an experiment:

diff -u -p -r1.5 mips-tdep.c
--- mips-tdep.c 17 Mar 2005 04:43:28 -0000      1.5
+++ mips-tdep.c 23 Mar 2005 00:30:38 -0000
@@ -606,7 +606,8 @@ mips_pseudo_register_read (struct gdbarc
           register_size (gdbarch, cookednum))
     {
       if (gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p
-         || TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
+         || TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE
+          || gdbarch_osabi (current_gdbarch) == GDB_OSABI_IRIX)
        regcache_raw_read_part (regcache, rawnum, 0, 4, buf);
       else
        regcache_raw_read_part (regcache, rawnum, 4, 4, buf);
@@ -628,7 +629,8 @@ mips_pseudo_register_write (struct gdbar
           register_size (gdbarch, cookednum))
     {
       if (gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p
-         || TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
+         || TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE
+          || gdbarch_osabi (current_gdbarch) == GDB_OSABI_IRIX)
        regcache_raw_write_part (regcache, rawnum, 0, 4, buf);
       else
        regcache_raw_write_part (regcache, rawnum, 4, 4, buf);

And it works very well. In fact, not only does it fix the problem
exposed here, but it does help a lot in the testsuite as well
(with function calls, and signal testcases).

But I am hesitating a bit. It looks a bit like a hack to be specifying
specifically IRIX here. I am afraid that there might be another way of
fixing this more cleanly. But I don't see anything. One idea I had was
to add yet another flag in the tdep part to say force the side where
to look. But that wouldn't very practical either. So I'm left with
the change above.

Any comments?

Thanks,
-- 
Joel

-- hello.adb
with text_io; use text_io;
with data; use data;
procedure hello is

begin
  f := 1.0e200/value;
  put_line ("overflow did not crash..result: " & long_float'image(f));
end;

--- data.ads
package data is
 f : long_float;
 value : long_float := 1.0e-300;
end;
---