[patch] m68k-dis.c: Don't get stuck in a strange state.

[Kazu Hirata <kazu at codesourcery dot com>]

Hi,

Attached is a patch to prevent m68k-dis from entering a strange state.

Consider the following output from objdump -D.

03000000 <_vector_table>:
 3000000:       021f fffc       andib #-4,%sp@+
 3000004:       0300            btst %d1,%d0
    :
 (a bunch of lines here)
    :
 30003f6:       3398 0300       movew %a0@+,%a1@(00000000,%d0:w:2)
 30003fa:       3398 0300       movew %a0@+,%a1@(00000000,%d0:w:2)
 30003fe:       Disassembly of section .text:

03000400 <_start>:
 3000400:       207c 021f fffc  moveal #35651580,%a0
 3000406:       b1fc 0000 0000  cmpal #0,%a0
 300040c:       6702            beqs 
    :
    :

Notice that the last instruction above doesn't show any operand even
though it is a conditional branch instruction.  This happens for many
instructions involving addresses.

Analysis: Notice that we have two bytes to go at 0x30003fe before the
end of the vector section.  We have 0x3398 at 0x30003fe.  With the -D
option, the disassembler tries to fetch two bytes of data after 0x3398
just like the previous two instructions.  However, that attempt fails
this time becuase we run out of bytes in the secion.  At this point we
have a call tree like so:

  print_insn_m68k           uses setjmp
    match_insn_m68k
      ...
        ...
          FETCH_DATA        uses longjmp

match_insn_m68k temporally replaces insn->fprintf_func and
insn->print_address_func with dummy functions while calculating the
total number of bytes for the current instruction.  In the m68k
backend, FETCH_DATA is the first function to notice that we run of
bytes to eat.  Then FETCH_DATA bails out and uses longjmp to go all
the way back to print_insn_m68k, skipping match_insn_m68k.  This skip
leaves insn->fprintf_func and insn->print_address_func pointing to the
dummy functions.  Subsequently, all the address printing would go to
the dummy function, resulting in lines like:

 300040c:       6702            beqs 
 300053c:       4879 0300 33c4  pea 
 3000542:       61ff 0000 00c4  bsrl 

with no addresses printed.

The patch fixes the problem by restoring insn->fprintf_func and
insn->print_address_func in print_insn_m68k when longjmp is used.

Now, I admit that this solution is a bit dirty.  Specifically, this
patch exposes the fact that match_insn_m68k temporarily replaces
insn->fprintf_func and insn->print_address_func.

Perhaps, a real fix is to report a FETCH_DATA failure with a return
value of some sort, without using setjmp/longjmp.  A better fix may be
to teach the m68k disassembler do its job without temporarily
replacing insn->fprintf_func and insn->print_address_func, but that
sounds like too big a surgery.

OK to apply?

Kazu Hirata

opcodes/
2007-04-04  Kazu Hirata  <kazu@codesourcery.com>

	* m68k-dis.c (print_insn_m68k): Restore info->fprintf_func and
	info->print_address_func if longjmp is called.

Index: opcodes/m68k-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/m68k-dis.c,v
retrieving revision 1.24
diff -u -d -p -r1.24 m68k-dis.c
--- opcodes/m68k-dis.c	27 Dec 2006 07:15:02 -0000	1.24
+++ opcodes/m68k-dis.c	5 Apr 2007 00:19:05 -0000
@@ -1475,6 +1475,12 @@ print_insn_m68k (bfd_vma memaddr, disass
 
   bfd_byte *buffer = priv.the_buffer;
 
+  /* Save these printing functions in case we need to restore them
+     later.  */
+  fprintf_ftype save_printer = info->fprintf_func;
+  void (* save_print_address) (bfd_vma, struct disassemble_info *)
+    = info->print_address_func;
+
   info->private_data = (PTR) &priv;
   /* Tell objdump to use two bytes per chunk
      and six bytes per line for displaying raw data.  */
@@ -1485,8 +1491,26 @@ print_insn_m68k (bfd_vma memaddr, disass
   priv.insn_start = memaddr;
 
   if (setjmp (priv.bailout) != 0)
-    /* Error return.  */
-    return -1;
+    {
+      /* longjmp may be called while these printing functions are
+	 temporarily replaced with dummy functions.  Restore them
+	 before we leave.
+
+	 Admittedly, this save-and-restore operation is somewhat ugly
+	 in that we are exposing the fact that match_insn_m68k
+	 temporarily replaces insn->fprintf_func and
+	 insn->print_address_func.  Perhaps, a real fix is to report a
+	 FETCH_DATA failure with a return value of some sort, without
+	 using setjmp/longjmp.  A better fix may be to teach the m68k
+	 disassembler do its job without temporarily replacing
+	 insn->fprintf_func and insn->print_address_func, but that's a
+	 task for another day.  */
+      info->fprintf_func = save_printer;
+      info->print_address_func = save_print_address;
+
+      /* Error return.  */
+      return -1;
+    }
 
   arch_mask = bfd_m68k_mach_to_features (info->mach);
   if (!arch_mask)