* [patch] m68k-dis.c: Don't get stuck in a strange state.
@ 2007-04-05 0:21 Kazu Hirata
2007-04-09 15:40 ` Daniel Jacobowitz
0 siblings, 1 reply; 2+ messages in thread
From: Kazu Hirata @ 2007-04-05 0:21 UTC (permalink / raw)
To: binutils
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)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [patch] m68k-dis.c: Don't get stuck in a strange state.
2007-04-05 0:21 [patch] m68k-dis.c: Don't get stuck in a strange state Kazu Hirata
@ 2007-04-09 15:40 ` Daniel Jacobowitz
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Jacobowitz @ 2007-04-09 15:40 UTC (permalink / raw)
To: Kazu Hirata; +Cc: binutils
On Wed, Apr 04, 2007 at 05:21:39PM -0700, Kazu Hirata wrote:
> 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.
I agree with your reasoning; this is OK to apply.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2007-04-09 15:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-05 0:21 [patch] m68k-dis.c: Don't get stuck in a strange state Kazu Hirata
2007-04-09 15:40 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).