Re: [RFC] mips-tdep.c: Sign extend values placed into registers for inferior function calls

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

> Date: Tue, 7 Dec 2010 13:28:06 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> Here is an example, taken from the log file, of one failure fixed by
> this patch:
> 
>     p t_short_values(10,-23)
>     UNPREDICTABLE: PC = 0x800209d4
>     Quit
>     (gdb) FAIL: gdb.base/callfuncs.exp: p t_short_values(10,-23)
> 
> In creating the inferior function call, gdb is placing a negative
> value, -23, sign extended only to 32-bits, into one of the 64-bit
> argument registers.  When the simulator executes a move instruction
> in the course of executing t_short_values(), it first checks to make
> sure that the register value is correctly sign extended.  I.e. it looks
> at bits 32-63 and makes sure that they all match the value in bit 31. 
> If it's not correctly sign extended, it outputs the "UNPREDICTABLE"
> message.  It then aborts back to GDB, often causing further problems
> for later tests.

So do I understand correctly that this is a problem with the
simulator, and that real hardware doesn't really care what is in the
unused upper half of the register?

Did you test this on real hardware?

> Comments?
> 
> Kevin
> 
> 	* mips-tdep.c (mips_eabi_push_dummy_call): Place signed, rather
> 	than unsigned, values in 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 19:52:14 -0000
> @@ -2755,7 +2755,7 @@ mips_eabi_push_dummy_call (struct gdbarc
>  	fprintf_unfiltered (gdb_stdlog,
>  			    "mips_eabi_push_dummy_call: struct_return reg=%d %s\n",
>  			    argreg, paddress (gdbarch, struct_addr));
> -      regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
> +      regcache_cooked_write_signed (regcache, argreg++, struct_addr);

This one makes sense, func_addr and bp_addr are already written signed.

> @@ -2834,7 +2834,7 @@ mips_eabi_push_dummy_call (struct gdbarc
>  	      if (mips_debug)
>  		fprintf_unfiltered (gdb_stdlog, " - fpreg=%d val=%s",
>  				    float_argreg, phex (regval, 4));
> -	      regcache_cooked_write_unsigned (regcache, float_argreg++, regval);
> +	      regcache_cooked_write_signed (regcache, float_argreg++, regval);
>  
>  	      /* Write the high word of the double to the odd register(s).  */
>  	      regval = extract_unsigned_integer (val + 4 - low_offset,
> @@ -2842,7 +2842,7 @@ mips_eabi_push_dummy_call (struct gdbarc
>  	      if (mips_debug)
>  		fprintf_unfiltered (gdb_stdlog, " - fpreg=%d val=%s",
>  				    float_argreg, phex (regval, 4));
> -	      regcache_cooked_write_unsigned (regcache, float_argreg++, regval);
> +	      regcache_cooked_write_signed (regcache, float_argreg++, regval);

These two are odd though, since the values are still read using
extract_integer_unsigned.  That's a bit odd.

>  	    }
>  	  else
>  	    {
> @@ -2854,7 +2854,7 @@ mips_eabi_push_dummy_call (struct gdbarc
>  	      if (mips_debug)
>  		fprintf_unfiltered (gdb_stdlog, " - fpreg=%d val=%s",
>  				    float_argreg, phex (regval, len));
> -	      regcache_cooked_write_unsigned (regcache, float_argreg++, regval);
> +	      regcache_cooked_write_signed (regcache, float_argreg++, regval);

same here

> @@ -2937,13 +2937,13 @@ mips_eabi_push_dummy_call (struct gdbarc
>  		  && !fp_register_arg_p (gdbarch, typecode, arg_type))
>  		{
>  		  LONGEST regval =
> -		    extract_unsigned_integer (val, partial_len, byte_order);
> +		    extract_signed_integer (val, partial_len, byte_order);
>  
>  		  if (mips_debug)
>  		    fprintf_filtered (gdb_stdlog, " - reg=%d val=%s",
>  				      argreg,
>  				      phex (regval, regsize));
> -		  regcache_cooked_write_unsigned (regcache, argreg, regval);
> +		  regcache_cooked_write_signed (regcache, argreg, regval);
>  		  argreg++;
>  		}
>  
> 
>