Re: [RFC] mips-tdep.c: pseudo-register -> raw register sign extension

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Tue, 7 Dec 2010 16:47:37 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> Below is another patch that fixes another problem arising from the
> simulator catching UNPREDICTABLE behavior.  Again, this affects mips64
> targets that are being run in 32-bit mode.
>
> ...
>
> What is happening here is that `l' resides in register s0 and
> the following instruction is the first of line 86:
> 
>     0x800205bc <wack_int+40>:    move    a0,s0
> 
> Note from the log above that `l' (register s0) starts out as -1, but
> is changed by the testing machinery to 4.  In the course of changing
> this value, mips_pseudo_register_write() is called in order to
> transfer the 32-bit pseudo register for s0 to the corresponding 64-bit
> raw register.  Without the patch below, only 32 bits are being
> transferred, thus leaving in place the (high 32-bit) sign extension of
> -1.  When the sim attempts to execute the instruction noted above, it
> first checks to make sure that the sign extension for the register
> being transferred is sane.  It is not, and therefore quits printing
> the UNPREDICTABLE message.  This ends up causing the first failure as
> well as the resulting cascade since GDB prints the following message
> for many of the later commands that it tries to execute:  "Cannot
> execute this command while the selected thread is running. 
> 
> Comments?

Sortof destroys the symmetry between pesudo_register_read and
pseudo_register_write.

> 	* mips-tdep.c (mips_pseudo_register_write): Sign extend 32-bit
> 	cooked values that are being transferred to 64-bit raw registers.
> 
> Index: mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.508
> diff -u -p -r1.508 mips-tdep.c
> --- mips-tdep.c	28 Nov 2010 04:31:24 -0000	1.508
> +++ mips-tdep.c	7 Dec 2010 21:58:58 -0000
> @@ -582,11 +582,19 @@ mips_pseudo_register_write (struct gdbar
>    else if (register_size (gdbarch, rawnum) >
>  	   register_size (gdbarch, cookednum))
>      {
> -      if (gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p
> -	  || gdbarch_byte_order (gdbarch) == BFD_ENDIAN_LITTLE)
> +      if (gdbarch_tdep (gdbarch)->mips64_transfers_32bit_regs_p)
>  	regcache_raw_write_part (regcache, rawnum, 0, 4, buf);
>        else
> -	regcache_raw_write_part (regcache, rawnum, 4, 4, buf);
> +	{
> +	  /* Sign extend the shortened version of the register prior
> +	     to placing it in the raw register.  This is required for
> +	     some mips64 parts in order to avoid unpredictable behavior.  */
> +	  gdb_byte buf8[8];
> +	  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +	  LONGEST regval = extract_signed_integer (buf, 4, byte_order);
> +	  store_signed_integer (buf8, 8, byte_order, regval);
> +	  regcache_raw_write (regcache, rawnum, buf8);

How about using regcache_raw_write_signed() here?  I think that
simplifies the code enough to change mips_pseudo_register_read() to
use regcache_raw_read_signed() and get rid of the explicit endian-ness
check as well.