Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Steve Watt]

On Jul 27,  9:50, Andrew Cagney wrote:
} Subject: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] 
} 
} Hello,
} 
} The attached hopefully fixes the panic that SteveW was noticing with the 
} MIPS GNU/Linux target.   It chops the legacy register cache (which the 
} MIPS uses) back so that it can no longer contain pseudo registers.
} 
} Working through the build list, it has the potential of also affecting:
} 
} h8300hms:gdbarch_dump: NUM_PSEUDO_REGS = 1
} i386-elf:gdbarch_dump: NUM_PSEUDO_REGS = 8
} m68hc11-elf:gdbarch_dump: NUM_PSEUDO_REGS = 37
} mips-elf:gdbarch_dump: NUM_PSEUDO_REGS = 90
} x86_64-linux-gnu:gdbarch_dump: NUM_PSEUDO_REGS = 8
} 
} (and sh64)
} 
} but since everything is now using pseudo register read/write, instead of 
} fetch/store pseudo register (I deleted that), it should ho longer have 
} any affect.
} 
} Hmm, if fetch/store pseudo is deleted all sorts of code can go ....
} 
} Steve, does this fix your problem?  Assuming it does, I'll also look to 
} commit this to the 6.0 branch.

How about halfway?  It fixes the abort, but leaves a weirdo in the
output:

(steve@mustang) 48> ./gdb /users/steve/tmp/t/hello.mips
GNU gdb 2003-07-27-cvs
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "--host=i686-pc-linux-gnu --target=mips-elf"...
Setting up the environment for debugging gdb.
.gdbinit:5: Error in sourced command file:
Function "internal_error" not defined.
(gdb) target sim
Connected to the simulator.
(gdb) load
Loading section .text, size 0x150c vma 0xa0020000
Loading section .init, size 0x38 vma 0xa002150c
Loading section .fini, size 0x28 vma 0xa0021544
Loading section .sdata, size 0x14 vma 0xa0021d20
Loading section .ctors, size 0x8 vma 0xa002156c
Loading section .dtors, size 0x8 vma 0xa0021574
Loading section .rodata, size 0x2 vma 0xa002157c
Loading section .eh_frame, size 0x4 vma 0xa0021580
Loading section .data, size 0x790 vma 0xa0021588
Loading section .jcr, size 0x4 vma 0xa0021d18
Start address 0xa0020004
Transfer rate: 59728 bits in <1 sec.
(gdb) break _start
Breakpoint 1 at 0xa0020004: file ../../../../combined/libgloss/mips/crt0.S, line 74.
(gdb) run
Starting program: /users/steve/tmp/t/hello.mips

Breakpoint 1, _start () at ../../../../combined/libgloss/mips/crt0.S:74
74              li      v0, STATUS_MASK
Current language:  auto; currently asm
(gdb) info regs
Undefined info command: "regs".  Try "help info".
(gdb) info regi
          zero       at       v0       v1       a0       a1       a2       a3
 R90  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
            t0       t1       t2       t3       t4       t5       t6       t7
 R98  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
            s0       s1       s2       s3       s4       s5       s6       s7
 R106 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
            t8       t9       k0       k1       gp       sp       s8       ra
 R114 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
            sr       lo       hi      bad    cause       pc
      00400004 00000000 00000000 00000000 00000000 a0020004
           fsr      fir
      00000000 00000000
(gdb)

It's displaying stuff (and further runs appear correct), but the register
number in the left column needs 90 subtracted.  (Well, I'm sure there's
a real fix, but...)

So far, so good.

-- 
Steve Watt       KD6GGD  PP-ASEL-IA       Email at  home: steve@watt.com
Chelsio Communications   http://www.chelsio.com/    work: steve@chelsio.com
510 N. Pastoria Ave                                Voice: +1 408 962 3627
Sunnyvale, CA, USA, 94085                            Fax: +1 408 730 2580

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]


> Undefined info command: "regs".  Try "help info".
> (gdb) info regi
>           zero       at       v0       v1       a0       a1       a2       a3
>  R90  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>             t0       t1       t2       t3       t4       t5       t6       t7
>  R98  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>             s0       s1       s2       s3       s4       s5       s6       s7
>  R106 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>             t8       t9       k0       k1       gp       sp       s8       ra
>  R114 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>             sr       lo       hi      bad    cause       pc
>       00400004 00000000 00000000 00000000 00000000 a0020004
>            fsr      fir
>       00000000 00000000
> (gdb)
> 
> It's displaying stuff (and further runs appear correct), but the register
> number in the left column needs 90 subtracted.  (Well, I'm sure there's
> a real fix, but...)

Ah, oops.  The real fix IS `regnum - 90' (well regnum % NUM_REGS).  See 
attached.

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 877 bytes --]

2003-07-27  Andrew Cagney  <cagney@redhat.com>

	* mips-tdep.c (print_gp_register_row): Print the GPR's register
	MOD NUM_REGS.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.223
diff -u -r1.223 mips-tdep.c
--- mips-tdep.c	7 Jul 2003 17:36:26 -0000	1.223
+++ mips-tdep.c	27 Jul 2003 21:15:40 -0000
@@ -4286,10 +4286,10 @@
       col++;
     }
   /* print the R0 to R31 names */
-  fprintf_filtered (file,
-		    (start_regnum % NUM_REGS) < MIPS_NUMREGS
-		    ? "\n R%-4d" : "\n      ",
-		    start_regnum);
+  if ((start_regnum % NUM_REGS) < MIPS_NUMREGS)
+    fprintf_filtered (file, "\n R%-4d", start_regnum % NUM_REGS);
+  else
+    fprintf_filtered (file, "\n      ");
 
   /* now print the values in hex, 4 or 8 to the row */
   for (col = 0, regnum = start_regnum;

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Andrew Cagney]

I've checked this tweak into 6.0 and mainline:

> 2003-07-27  Andrew Cagney  <cagney@redhat.com>
> 
> 	* mips-tdep.c (print_gp_register_row): Print the GPR's register
> 	MOD NUM_REGS.
> 

and this change into just the mainline (6.0 will follow in a few days if 
no one notices a problem):

> 2003-07-27  Andrew Cagney  <cagney@redhat.com>
> 
> 	* regcache.c (struct regcache_descr): Update comments on
> 	nr_raw_registers.
> 	(init_legacy_regcache_descr): Don't set nr_raw_registers or
> 	sizeof_raw_register_valid_p.
> 	(init_regcache_descr): Set nr_raw_registers and
> 	sizeof_raw_register_valid_p before calling
> 	init_legacy_regcache_descr.
> 

Andrew

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Daniel Jacobowitz]

On Mon, Jul 28, 2003 at 11:37:12AM -0400, Andrew Cagney wrote:
> I've checked this tweak into 6.0 and mainline:
> 
> >2003-07-27  Andrew Cagney  <cagney@redhat.com>
> >
> >	* mips-tdep.c (print_gp_register_row): Print the GPR's register
> >	MOD NUM_REGS.
> >
> 
> and this change into just the mainline (6.0 will follow in a few days if 
> no one notices a problem):
> 
> >2003-07-27  Andrew Cagney  <cagney@redhat.com>
> >
> >	* regcache.c (struct regcache_descr): Update comments on
> >	nr_raw_registers.
> >	(init_legacy_regcache_descr): Don't set nr_raw_registers or
> >	sizeof_raw_register_valid_p.
> >	(init_regcache_descr): Set nr_raw_registers and
> >	sizeof_raw_register_valid_p before calling
> >	init_legacy_regcache_descr.
> >

On a related note, before this change (2003-07-07 actually)
mipsel-linux was completely broken.  Info registers, in addition to
printing R90, also claimed that all registers were zero.  Backtraces
broke for the same reason.

I haven't tried newer mainline yet; I will today or tomorrow.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Andrew Cagney]

> and this change into just the mainline (6.0 will follow in a few days if no one notices a problem):
> 
> 2003-07-27  Andrew Cagney  <cagney@redhat.com>
> 
>     * regcache.c (struct regcache_descr): Update comments on
>     nr_raw_registers.
>     (init_legacy_regcache_descr): Don't set nr_raw_registers or
>     sizeof_raw_register_valid_p.
>     (init_regcache_descr): Set nr_raw_registers and
>     sizeof_raw_register_valid_p before calling
>     init_legacy_regcache_descr.
> 
I've committed this to the 6.0 branch.

Andrew

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Daniel Jacobowitz]

On Mon, Jul 28, 2003 at 12:38:57PM -0400, Daniel Jacobowitz wrote:
> On Mon, Jul 28, 2003 at 11:37:12AM -0400, Andrew Cagney wrote:
> > I've checked this tweak into 6.0 and mainline:
> > 
> > >2003-07-27  Andrew Cagney  <cagney@redhat.com>
> > >
> > >	* mips-tdep.c (print_gp_register_row): Print the GPR's register
> > >	MOD NUM_REGS.
> > >
> > 
> > and this change into just the mainline (6.0 will follow in a few days if 
> > no one notices a problem):
> > 
> > >2003-07-27  Andrew Cagney  <cagney@redhat.com>
> > >
> > >	* regcache.c (struct regcache_descr): Update comments on
> > >	nr_raw_registers.
> > >	(init_legacy_regcache_descr): Don't set nr_raw_registers or
> > >	sizeof_raw_register_valid_p.
> > >	(init_regcache_descr): Set nr_raw_registers and
> > >	sizeof_raw_register_valid_p before calling
> > >	init_legacy_regcache_descr.
> > >
> 
> On a related note, before this change (2003-07-07 actually)
> mipsel-linux was completely broken.  Info registers, in addition to
> printing R90, also claimed that all registers were zero.  Backtraces
> broke for the same reason.
> 
> I haven't tried newer mainline yet; I will today or tomorrow.

Sorry, Andrew - we're almost there but MIPS register handling is still
messed up.  Take a look at "info all-registers":

(gdb) i all-registers
          zero       at       v0       v1       a0       a1       a2       a3
 R0   00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
            t0       t1       t2       t3       t4       t5       t6       t7
 R8   00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
            s0       s1       s2       s3       s4       s5       s6       s7
 R16  7fff7eaf 7fff7dcc 7fff7dd4 00000003 004059cc 10012808 00000000 00000000
            t8       t9       k0       k1       gp       sp       s8       ra
 R24  00000000 00000000 00000000 00000000 00000000 7fff7df0 00000000 00000000
          zero       at       v0       v1       a0       a1
      00000000 0067194b 00000004 80279720 00000020 2aac0ac0
 a2:  0xffffffff flt: nan               dbl: nan
 a3:  0xffffffff flt: nan
 t0:  0xffffffff flt: nan               dbl: nan
 t1:  0xffffffff flt: nan
 t2:  0xffffffff flt: nan               dbl: nan
 t3:  0xffffffff flt: nan


Obviously the registers aren't where it expects them to be.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Andrew Cagney]

> On Mon, Jul 28, 2003 at 12:38:57PM -0400, Daniel Jacobowitz wrote:
> 
>> On Mon, Jul 28, 2003 at 11:37:12AM -0400, Andrew Cagney wrote:
> 
>> > I've checked this tweak into 6.0 and mainline:
>> > 
> 
>> > >2003-07-27  Andrew Cagney  <cagney@redhat.com>
>> > >
>> > >	* mips-tdep.c (print_gp_register_row): Print the GPR's register
>> > >	MOD NUM_REGS.
>> > >
> 
>> > 
>> > and this change into just the mainline (6.0 will follow in a few days if 
>> > no one notices a problem):
>> > 
> 
>> > >2003-07-27  Andrew Cagney  <cagney@redhat.com>
>> > >
>> > >	* regcache.c (struct regcache_descr): Update comments on
>> > >	nr_raw_registers.
>> > >	(init_legacy_regcache_descr): Don't set nr_raw_registers or
>> > >	sizeof_raw_register_valid_p.
>> > >	(init_regcache_descr): Set nr_raw_registers and
>> > >	sizeof_raw_register_valid_p before calling
>> > >	init_legacy_regcache_descr.
>> > >
> 
>> 
>> On a related note, before this change (2003-07-07 actually)
>> mipsel-linux was completely broken.  Info registers, in addition to
>> printing R90, also claimed that all registers were zero.  Backtraces
>> broke for the same reason.
>> 
>> I haven't tried newer mainline yet; I will today or tomorrow.
> 
> 
> Sorry, Andrew - we're almost there but MIPS register handling is still
> messed up.  Take a look at "info all-registers":

What does "maint print [raw-]registers" look like?

> (gdb) i all-registers
>           zero       at       v0       v1       a0       a1       a2       a3
>  R0   00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>             t0       t1       t2       t3       t4       t5       t6       t7
>  R8   00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>             s0       s1       s2       s3       s4       s5       s6       s7
>  R16  7fff7eaf 7fff7dcc 7fff7dd4 00000003 004059cc 10012808 00000000 00000000
>             t8       t9       k0       k1       gp       sp       s8       ra
>  R24  00000000 00000000 00000000 00000000 00000000 7fff7df0 00000000 00000000
>           zero       at       v0       v1       a0       a1
>       00000000 0067194b 00000004 80279720 00000020 2aac0ac0
>  a2:  0xffffffff flt: nan               dbl: nan
>  a3:  0xffffffff flt: nan
>  t0:  0xffffffff flt: nan               dbl: nan
>  t1:  0xffffffff flt: nan
>  t2:  0xffffffff flt: nan               dbl: nan
>  t3:  0xffffffff flt: nan
> 
> 
> Obviously the registers aren't where it expects them to be.
> 
> -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 

Re: [patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Daniel Jacobowitz]

On Mon, Aug 18, 2003 at 01:42:39PM -0400, Andrew Cagney wrote:
> What does "maint print [raw-]registers" look like?
> 
> >(gdb) i all-registers
> >          zero       at       v0       v1       a0       a1       a2       
> >          a3
> > R0   00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> > 00000000
> >            t0       t1       t2       t3       t4       t5       t6       
> >            t7
> > R8   00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> > 00000000
> >            s0       s1       s2       s3       s4       s5       s6       
> >            s7
> > R16  7fff7eaf 7fff7dcc 7fff7dd4 00000003 004059cc 10012808 00000000 
> > 00000000
> >            t8       t9       k0       k1       gp       sp       s8       
> >            ra
> > R24  00000000 00000000 00000000 00000000 00000000 7fff7df0 00000000 
> > 00000000
> >          zero       at       v0       v1       a0       a1
> >      00000000 0067194b 00000004 80279720 00000020 2aac0ac0
> > a2:  0xffffffff flt: nan               dbl: nan
> > a3:  0xffffffff flt: nan
> > t0:  0xffffffff flt: nan               dbl: nan
> > t1:  0xffffffff flt: nan
> > t2:  0xffffffff flt: nan               dbl: nan
> > t3:  0xffffffff flt: nan
> >
> >
> >Obviously the registers aren't where it expects them to be.

Thanks for the pointer, and sorry for the noise.  The problem was a
local patch which added MIPS_REGISTER_NAMES for GNU/Linux; it recently
changed to start at 32 and I still had the first 32 registers listed.

After 6.0 I'll try to flush my MIPS queue.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

[patch rfc, 6.0?] only allow raw raw registers; Was: [patch rfc] Add NUM_REGS pseudo regs to MIPS

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

Hello,

The attached hopefully fixes the panic that SteveW was noticing with the 
MIPS GNU/Linux target.   It chops the legacy register cache (which the 
MIPS uses) back so that it can no longer contain pseudo registers.

Working through the build list, it has the potential of also affecting:

h8300hms:gdbarch_dump: NUM_PSEUDO_REGS = 1
i386-elf:gdbarch_dump: NUM_PSEUDO_REGS = 8
m68hc11-elf:gdbarch_dump: NUM_PSEUDO_REGS = 37
mips-elf:gdbarch_dump: NUM_PSEUDO_REGS = 90
x86_64-linux-gnu:gdbarch_dump: NUM_PSEUDO_REGS = 8

(and sh64)

but since everything is now using pseudo register read/write, instead of 
fetch/store pseudo register (I deleted that), it should ho longer have 
any affect.

Hmm, if fetch/store pseudo is deleted all sorts of code can go ....

Steve, does this fix your problem?  Assuming it does, I'll also look to 
commit this to the 6.0 branch.

Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3283 bytes --]

2003-07-27  Andrew Cagney  <cagney@redhat.com>

	* regcache.c (struct regcache_descr): Update comments on
	nr_raw_registers.
	(init_legacy_regcache_descr): Don't set nr_raw_registers or
	sizeof_raw_register_valid_p.
	(init_regcache_descr): Set nr_raw_registers and
	sizeof_raw_register_valid_p before calling
	init_legacy_regcache_descr.

Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.90
diff -u -r1.90 regcache.c
--- regcache.c	9 Jul 2003 14:35:26 -0000	1.90
+++ regcache.c	27 Jul 2003 13:16:00 -0000
@@ -51,10 +51,11 @@
      for raw and pseudo registers and allow access to both.  */
   int legacy_p;
 
-  /* The raw register cache.  This should contain just [0
-     .. NUM_RAW_REGISTERS).  However, for older targets, it contains
-     space for the full [0 .. NUM_RAW_REGISTERS +
-     NUM_PSEUDO_REGISTERS).  */
+  /* The raw register cache.  Each raw (or hard) register is supplied
+     by the target interface.  The raw cache should not contain
+     redundant information - if the PC is constructed from two
+     registers then those regigisters and not the PC lives in the raw
+     cache.  */
   int nr_raw_registers;
   long sizeof_raw_registers;
   long sizeof_raw_register_valid_p;
@@ -91,12 +92,6 @@
      ``gdbarch'' as a parameter.  */
   gdb_assert (gdbarch != NULL);
 
-  /* FIXME: cagney/2002-05-11: Shouldn't be including pseudo-registers
-     in the register cache.  Unfortunatly some architectures still
-     rely on this and the pseudo_register_write() method.  */
-  descr->nr_raw_registers = descr->nr_cooked_registers;
-  descr->sizeof_raw_register_valid_p = descr->sizeof_cooked_register_valid_p;
-
   /* Compute the offset of each register.  Legacy architectures define
      REGISTER_BYTE() so use that.  */
   /* FIXME: cagney/2002-11-07: Instead of using REGISTER_BYTE() this
@@ -176,6 +171,16 @@
 	descr->register_type[i] = REGISTER_VIRTUAL_TYPE (i); /* OK */
     }
 
+  /* Construct a strictly RAW register cache.  Don't allow pseudo's
+     into the register cache.  */
+  descr->nr_raw_registers = NUM_REGS;
+
+  /* FIXME: cagney/2002-08-13: Overallocate the register_valid_p
+     array.  This pretects GDB from erant code that accesses elements
+     of the global register_valid_p[] array in the range [NUM_REGS
+     .. NUM_REGS + NUM_PSEUDO_REGS).  */
+  descr->sizeof_raw_register_valid_p = descr->sizeof_cooked_register_valid_p;
+
   /* If an old style architecture, fill in the remainder of the
      register cache descriptor using the register macros.  */
   /* NOTE: cagney/2003-06-29: If either of REGISTER_BYTE or
@@ -193,16 +198,6 @@
       init_legacy_regcache_descr (gdbarch, descr);
       return descr;
     }
-
-  /* Construct a strictly RAW register cache.  Don't allow pseudo's
-     into the register cache.  */
-  descr->nr_raw_registers = NUM_REGS;
-
-  /* FIXME: cagney/2002-08-13: Overallocate the register_valid_p
-     array.  This pretects GDB from erant code that accesses elements
-     of the global register_valid_p[] array in the range [NUM_REGS
-     .. NUM_REGS + NUM_PSEUDO_REGS).  */
-  descr->sizeof_raw_register_valid_p = descr->sizeof_cooked_register_valid_p;
 
   /* Lay out the register cache.