Re: [PATCH] Correct irix5-nat.c's fetch_core_registers

[Roger Sayle <roger at eyesopen dot com>]

Hi Daniel,

Many thanks for the review.

On Mon, 24 Jul 2006, Daniel Jacobowitz wrote:
> On Fri, Jul 21, 2006 at 10:12:10AM -0600, Roger Sayle wrote:
> > !   else if (mips_isa_regsize (current_gdbarch) == 4 &&
> > ! 	   core_reg_size == (2 * mips_isa_regsize (current_gdbarch)) * NUM_REGS)
>
> > !   else /* mips_isa_regsize (current_gdbarch) == 4 */
> >       {
> >         /* This is a core file from a N32 executable, 64 bits are saved
> >            for all registers.  */
>
> This change doesn't make sense to me.  If I'm missing something, could
> you try to explain it again?

The current implementation in fetch_core_registers has two main
clauses.  The first which currently reads "core_reg_size ==
deprecated_register_bytes ()" really expects deprecated_register_bytes
to return 568, or 8*NUM_REGS are explained in the original post.
The second clause checks whether "mips_isa_regsize (current_gdbarch) == 4"
and then whether
  core_reg_size == (2 * mips_isa_regsize (current_gdbarch)) * NUM_REGS

A quick bit of CSE, constant-folding and mental arithmetic, reveals
that this is equivalent to 8*NUM_REGS, or 568 bytes.  Hence, it looks
like GDB has only ever supported core files from ABIs that use 568
bytes to store all 71 MIPS registers.

Once this is grasped, the code can be cleaned up.  We check at the
top of the function whether the core_reg_size has the correct value,
and then use one of two implementations to extract the actual
register values based upon whether mips_isa_regsize is 4 or 8.
When mips_isa_regsize is 8 everything is simple, otherwise the
existing code selects whether the register is stored in the high-end
or low-end of the 8 bytes stored on disk (depending upon whether
the register is integer or floating point).


> As far as I remember, Irix supports O32 executables.  So at a minimum
> the comment is wrong.  I don't know if it dumps 32-bit or 64-bit
> registers for O32 core files; I wouldn't be too surprised if it dumped
> 64-bit core files and just the comment needed fixing.

Ok, I've just done some experimentation, and it looks like GDB has
never supported O32 core files.  Not only do I get the error message
"warning: wrong size gregset struct in core file" with all releases
of GDB since v6.0 (as I do with N32 core files), but I also get the
same failure with gdb v5.3.  So N32 core files have been broken
since 2002, but O32 core files have been broken for much longer or
never supported.

Turns out the code in fetch_core_registers only supports N32/N64,
and this comment is in a section of code that can only ever be
reached for N32.  Infact, its an N32 core file that I'm trying to
debug, but because I'm running gdb on a 64-bit mips4 host, GDB's
mips_isa_regsize returns 8.  Hence, its the first clause that needs
to be fixed for me.

The good news is I think I may be able to get O32 to work.  Turns
out in O32 core files, core_reg_size=284 (which is conveniently
4*NUM_REGS).  It also looks like when working on an O32 executable,
mips_isa_regsize is 4 (even on my MIPS4 box).  So this aspect of
the logic seems simple enough, even though I'm not sure how much
other support would need to be added to GDB to finish an O32 port.


Of course, it's academic at the moment as GDB on IRIX is still very
broken; I still get "You can't do that without a process to debug"
error messages long before the code ever gets to fetch_core_registers
when attempting to read an O32 executable and core file, and I need
to apply my other "cast" patch to just get the tree to build!


Many thanks again for looking at this patch.  Much appreciated.

Roger
--