This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH] MIPS: MIPS16 offset calculation and branch decoding fixes
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: <gdb-patches at sourceware dot org>
- Date: Sat, 3 Dec 2011 00:54:48 +0000
- Subject: [PATCH] MIPS: MIPS16 offset calculation and branch decoding fixes
Hi,
Here's a fix to MIPS16 offset calculation I promised along the clean-up
patch committed last week. Included are changes to extended_offset to
correct swapped immediate instruction fields as well as numerous
corrections throughout unpack_mips16 and extended_mips16_next_pc to
address errors in the calculation of immediates of instruction-specific
width and signedness.
This change includes fixes to branch decoding as well, in particular
adding the missing mapping of the MIPS16 3-bit register encoding to the
full 5-bit register file index and adding the decoding of MIPS16e compact
jumps missing entirely.
All of this code is only used for software single-stepping -- in
calculation of the address of the second next instruction to execute to
place a single-stepping breakpoint at. As it stands this code therefore
is only used on Linux targets.
However our MIPS16/Linux support is not up to running the test suite
reliably -- which is certainly the reason this code has survived in such a
poor shape for so long. Therefore I ran regression-testing with the bare
iron mips-sde-elf target instead (using a MIPS16 multilib configuration
and the MIPS32r2 ISA selected for code generation, i.e. with the MIPS16e
compact jumps enabled and verified actually present across test cases),
using a simulator.
So in preparation to get reference results I have first forced the bare
iron configuration to use software single-stepping (with a trivial patch).
This, as expected, caused numerous regressions as tests went astray
because of breakpoints placed at the wrong places. Or 119 to be exact:
=== gdb Summary ===
# of expected passes 13006
# of unexpected failures 475
# of unexpected successes 2
# of expected failures 48
# of known failures 61
# of unresolved testcases 18
# of untested testcases 34
# of unsupported tests 140
vs original:
=== gdb Summary ===
# of expected passes 13124
# of unexpected failures 357
# of unexpected successes 2
# of expected failures 48
# of known failures 61
# of unresolved testcases 17
# of untested testcases 34
# of unsupported tests 140
I have applied the change below then which made the figures go back to the
originals, making the results the same as with hardware single-stepping.
Which makes me believe I have fixed at least all the significant bugs in
this code.
OK to apply?
2011-12-03 Maciej W. Rozycki <macro@codesourcery.com>
gdb/
* mips-tdep.c (extended_offset): Correct calculation.
(unpack_mips16): Correct bitfield positions used for extraction
of the immediate argument; fix sign-extension of same.
(extended_mips16_next_pc): Correct B instruction's offset
calculation. Correct register decoding of the BEQZ and BNEZ
as well as jump-register instructions. Handle compact jumps.
Maciej
gdb-mips16-offset-fix.diff
Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c 2011-11-24 16:52:16.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c 2011-11-24 16:54:18.285614621 +0000
@@ -1392,9 +1392,9 @@ extended_offset (unsigned int extension)
{
CORE_ADDR value;
- value = (extension >> 21) & 0x3f; /* Extract 15:11. */
+ value = (extension >> 16) & 0x1f; /* Extract 15:11. */
value = value << 6;
- value |= (extension >> 16) & 0x1f; /* Extract 10:5. */
+ value |= (extension >> 21) & 0x3f; /* Extract 10:5. */
value = value << 5;
value |= extension & 0x1f; /* Extract 4:0. */
@@ -1434,14 +1434,13 @@ unpack_mips16 (struct gdbarch *gdbarch,
CORE_ADDR value;
if (extension)
{
- value = extended_offset (extension);
- value = value << 11; /* rom for the original value */
- value |= inst & 0x7ff; /* eleven bits from instruction */
+ value = extended_offset ((extension << 16) | inst);
+ value = (value ^ 0x8000) - 0x8000; /* Sign-extend. */
}
else
{
value = inst & 0x7ff;
- /* FIXME : Consider sign extension. */
+ value = (value ^ 0x400) - 0x400; /* Sign-extend. */
}
offset = value;
regx = -1;
@@ -1456,28 +1455,16 @@ unpack_mips16 (struct gdbarch *gdbarch,
CORE_ADDR value;
if (extension)
{
- value = extended_offset (extension);
- value = value << 8; /* from the original instruction */
- value |= inst & 0xff; /* eleven bits from instruction */
- regx = (extension >> 8) & 0x07; /* or i8 funct */
- if (value & 0x4000) /* Test the sign bit, bit 26. */
- {
- value &= ~0x3fff; /* Remove the sign bit. */
- value = -value;
- }
+ value = extended_offset ((extension << 16) | inst);
+ value = (value ^ 0x8000) - 0x8000; /* Sign-extend. */
}
else
{
- value = inst & 0xff; /* 8 bits */
- regx = (inst >> 8) & 0x07; /* or i8 funct */
- /* FIXME: Do sign extension, this format needs it. */
- if (value & 0x80) /* THIS CONFUSES ME. */
- {
- value &= 0xef; /* Remove the sign bit. */
- value = -value;
- }
+ value = inst & 0xff; /* 8 bits */
+ value = (value ^ 0x80) - 0x80; /* Sign-extend. */
}
offset = value;
+ regx = (inst >> 8) & 0x07; /* i8 funct */
regy = -1;
break;
}
@@ -1523,13 +1510,7 @@ extended_mips16_next_pc (struct frame_in
CORE_ADDR offset;
struct upk_mips16 upk;
unpack_mips16 (gdbarch, pc, extension, insn, itype, &upk);
- offset = upk.offset;
- if (offset & 0x800)
- {
- offset &= 0xeff;
- offset = -offset;
- }
- pc += (offset << 1) + 2;
+ pc += (upk.offset << 1) + 2;
break;
}
case 3: /* JAL , JALX - Watch out, these are 32 bit
@@ -1549,7 +1530,7 @@ extended_mips16_next_pc (struct frame_in
struct upk_mips16 upk;
int reg;
unpack_mips16 (gdbarch, pc, extension, insn, ritype, &upk);
- reg = get_frame_register_signed (frame, upk.regx);
+ reg = get_frame_register_signed (frame, mips16_to_32_reg[upk.regx]);
if (reg == 0)
pc += (upk.offset << 1) + 2;
else
@@ -1561,7 +1542,7 @@ extended_mips16_next_pc (struct frame_in
struct upk_mips16 upk;
int reg;
unpack_mips16 (gdbarch, pc, extension, insn, ritype, &upk);
- reg = get_frame_register_signed (frame, upk.regx);
+ reg = get_frame_register_signed (frame, mips16_to_32_reg[upk.regx]);
if (reg != 0)
pc += (upk.offset << 1) + 2;
else
@@ -1593,21 +1574,10 @@ extended_mips16_next_pc (struct frame_in
int reg;
upk.regx = (insn >> 8) & 0x07;
upk.regy = (insn >> 5) & 0x07;
- switch (upk.regy)
- {
- case 0:
- reg = upk.regx;
- break;
- case 1:
- reg = 31;
- break; /* Function return instruction. */
- case 2:
- reg = upk.regx;
- break;
- default:
- reg = 31;
- break; /* BOGUS Guess */
- }
+ if ((upk.regy & 1) == 0)
+ reg = mips16_to_32_reg[upk.regx];
+ else
+ reg = 31; /* Function return instruction. */
pc = get_frame_register_signed (frame, reg);
}
else