This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] mips-tdep.c: Sign extend values placed into registers for inferior function calls
On Tue, 7 Dec 2010 22:45:26 +0100 (CET)
Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > 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?
I suspect that most real hardware would not care. Here's what a comment
in the mips simulator has to say about the matter:
This function implements what the MIPS32 and MIPS64 ISAs define as
"UNPREDICTABLE" behaviour.
About UNPREDICTABLE behaviour they say: "UNPREDICTABLE results
may vary from processor implementation to processor implementation,
instruction to instruction, or as a function of time on the same
implementation or instruction. Software can never depend on results
that are UNPREDICTABLE. ..." (MIPS64 Architecture for Programmers
Volume II, The MIPS64 Instruction Set. MIPS Document MD00087 revision
0.95, page 2.)
For UNPREDICTABLE behaviour, we print a message, if possible print
the offending instructions mips.igen instruction name (provided by
the caller), and stop the simulator.
> Did you test this on real hardware?
I have, though not recently. I don't recall running into this issue
on the hardware that I used for testing.
> > @@ -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.
As I recall, back when I wrote this patch, my conclusion at that time
was that it didn't matter whether extract_integer_unsigned() or
extract_integer_signed() was used. But I do agree that it looks odd.
I've amended my patch to use extract_signed_integer instead. See
below for the amended patch. (I don't see any change in test results
using the revised patch.)
Thanks for looking it over...
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 22:24:13 -0000
@@ -2826,23 +2834,23 @@ mips_eabi_push_dummy_call (struct gdbarc
{
int low_offset = gdbarch_byte_order (gdbarch)
== BFD_ENDIAN_BIG ? 4 : 0;
- unsigned long regval;
+ long regval;
/* Write the low word of the double to the even register(s). */
- regval = extract_unsigned_integer (val + low_offset,
- 4, byte_order);
+ regval = extract_signed_integer (val + low_offset,
+ 4, byte_order);
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,
- 4, byte_order);
+ regval = extract_signed_integer (val + 4 - low_offset,
+ 4, byte_order);
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);
}
else
{
@@ -2850,11 +2858,11 @@ mips_eabi_push_dummy_call (struct gdbarc
in a single register. */
/* On 32 bit ABI's the float_argreg is further adjusted
above to ensure that it is even register aligned. */
- LONGEST regval = extract_unsigned_integer (val, len, byte_order);
+ LONGEST regval = extract_signed_integer (val, len, byte_order);
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);
}
}
else
@@ -2937,13 +2945,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++;
}