[binutils-gdb] MIPS: Fix microMIPS instruction size determination

[Maciej W.Rozycki <macro at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3f7f365076a112313870e2a321452bfda6d40914

commit 3f7f365076a112313870e2a321452bfda6d40914
Author: Maciej W. Rozycki <macro@imgtec.com>
Date:   Mon Jan 18 20:24:34 2016 +0000

    MIPS: Fix microMIPS instruction size determination
    
    Fix a bug in `micromips_insn_at_pc_has_delay_slot' in instruction size
    determination via `mips_insn_size'.  In the microMIPS case the latter
    function expects a lone 16-bit instruction word containing the major
    opcode regardless of whether the opcode requires another 16-bit word to
    follow, to form a complete 32-bit instruction.  Code however passes the
    16-bit word previously retrieved shifted left by 16 bits.  Consequently
    `mips_insn_size', which examines the low 16-bit only, always sees 0.
    
    By pure coincidence a major opcode of 0 denotes a 32-bit instruction in
    the microMIPS instruction set, so the size of 4 is always returned here,
    and the following 16-bit word is then merged in the low 16 bits of the
    instruction previously shifted by 16 bits.  The resulting 32-bit value
    is then passed to `micromips_instruction_has_delay_slot' for delay slot
    presence determination.  This function in turn first examines the high
    16 bits of the instruction word received and ignores the low 16 bits for
    16-bit instructions.
    
    Consequently the only effect of this bug is an extraneous memory read
    issued to retrieve a subsequent 16-bit word where a 16-bit instruction
    is being examined.  Which in turn may fail if the instruction is located
    right at the end of a readable memory area, in which case the lack of a
    delay slot will be reported to the caller, which may be incorrect.
    
    This code is used in breakpoint maintenance, for delay slot avoidance,
    so the bug would only trigger for the unlikely case of someone placing
    a breakpoint in a delay slot of an instruction which is at the end of
    readable memory.  Which explains why the bug remained unnoticed so long.
    
    	gdb/
    	* mips-tdep.c (micromips_insn_at_pc_has_delay_slot): Pass
    	unshifted 16-bit microMIPS instruction word to `mips_insn_size'.

Diff:
---
 gdb/ChangeLog   | 5 +++++
 gdb/mips-tdep.c | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8fe1381..cfe889d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-18  Maciej W. Rozycki  <macro@imgtec.com>
+
+	* mips-tdep.c (micromips_insn_at_pc_has_delay_slot): Pass
+	unshifted 16-bit microMIPS instruction word to `mips_insn_size'.
+
 2016-01-18  Pedro Alves  <palves@redhat.com>
 
 	* NEWS: Mention that GDB now displays the ID and name of the
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index ca17864..f787a6d 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -7376,12 +7376,14 @@ micromips_insn_at_pc_has_delay_slot (struct gdbarch *gdbarch,
 {
   ULONGEST insn;
   int status;
+  int size;
 
   insn = mips_fetch_instruction (gdbarch, ISA_MICROMIPS, addr, &status);
   if (status)
     return 0;
+  size = mips_insn_size (ISA_MICROMIPS, insn);
   insn <<= 16;
-  if (mips_insn_size (ISA_MICROMIPS, insn) == 2 * MIPS_INSN16_SIZE)
+  if (size == 2 * MIPS_INSN16_SIZE)
     {
       insn |= mips_fetch_instruction (gdbarch, ISA_MICROMIPS, addr, &status);
       if (status)