Re: [PATCH 6/8] Return -1 on memory error in print_insn_m68k

[Yao Qi <qiyaoltc at gmail dot com>]

On 17-01-12 08:45:14, Alan Modra wrote:
> On Tue, Jan 10, 2017 at 12:26:16PM +0000, Yao Qi wrote:
> > 	* m68k-dis.c (match_insn_m68k): Extend comments.  Return -1
> > 	if FETCH_DATA returns 0.
> > 	(m68k_scan_mask): Likewise.
> > 	(print_insn_m68k): Update code to handle -1 return value.
> 
> This misses one FETCH_DATA call, in m68k_scan_mask.
> 

I can't remember the reason I didn't change it.  Sorry.

What about the patch below?  Regression tested on x86_64-linux
with all targets enabled.

-- 
Yao (齐尧)

>From 926e74e41c2f0248d18821c2ac83adbd6c13d544 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 8 Dec 2016 17:16:39 +0000
Subject: [PATCH] Return -1 on  memory error in print_insn_m68k

m68k-dis.c:print_insn_m68k doesn't return -1 on memory error, but GDB
expects it returning -1 on memory error.

FETCH_DATA is used in m68k_scan_mask to fetch the next two bytes and
match instruction in the buffer if opcode is four-byte long.  That
makes difficult to check the return value of FETCH_DATA, so this
patch moves FETCH_DATA before matching instruction, so FETCH_DATA
is called if we know the opcode is four-byte long by
((0xffff & match) != 0).

opcodes:

2017-01-12  Yao Qi  <yao.qi@linaro.org>

	* m68k-dis.c (match_insn_m68k): Extend comments.  Return -1
	if FETCH_DATA returns 0.
	(m68k_scan_mask): Likewise.  Call FETCH_DATA before matching
	instructions.
	(print_insn_m68k): Update code to handle -1 return value.

diff --git a/opcodes/m68k-dis.c b/opcodes/m68k-dis.c
index a14db9a..2871ad8 100644
--- a/opcodes/m68k-dis.c
+++ b/opcodes/m68k-dis.c
@@ -1331,7 +1331,8 @@ print_insn_arg (const char *d,
 }
 
 /* Try to match the current instruction to best and if so, return the
-   number of bytes consumed from the instruction stream, else zero.  */
+   number of bytes consumed from the instruction stream, else zero.
+   Return -1 on memory error.  */
 
 static int
 match_insn_m68k (bfd_vma memaddr,
@@ -1415,12 +1416,14 @@ match_insn_m68k (bfd_vma memaddr,
 	 this because we know exactly what the second word is, and we
 	 aren't going to print anything based on it.  */
       p = buffer + 6;
-      FETCH_DATA (info, p);
+      if (!FETCH_DATA (info, p))
+	return -1;
       buffer[2] = buffer[4];
       buffer[3] = buffer[5];
     }
 
-  FETCH_DATA (info, p);
+  if (!FETCH_DATA (info, p))
+    return -1;
 
   save_p = p;
   info->print_address_func = dummy_print_address;
@@ -1439,7 +1442,7 @@ match_insn_m68k (bfd_vma memaddr,
 	{
 	  info->fprintf_func = save_printer;
 	  info->print_address_func = save_print_address;
-	  return 0;
+	  return eaten == PRINT_INSN_ARG_MEMORY_ERROR ? -1 : 0;
 	}
       else
 	{
@@ -1481,7 +1484,8 @@ match_insn_m68k (bfd_vma memaddr,
 /* Try to interpret the instruction at address MEMADDR as one that
    can execute on a processor with the features given by ARCH_MASK.
    If successful, print the instruction to INFO->STREAM and return
-   its length in bytes.  Return 0 otherwise.  */
+   its length in bytes.  Return 0 otherwise.  Return -1 on memory
+   error.  */
 
 static int
 m68k_scan_mask (bfd_vma memaddr, disassemble_info *info,
@@ -1523,7 +1527,8 @@ m68k_scan_mask (bfd_vma memaddr, disassemble_info *info,
 	*opc_pointer[(m68k_opcodes[i].opcode >> 28) & 15]++ = &m68k_opcodes[i];
     }
 
-  FETCH_DATA (info, buffer + 2);
+  if (!FETCH_DATA (info, buffer + 2))
+    return -1;
   major_opcode = (buffer[0] >> 4) & 15;
 
   for (i = 0; i < numopcodes[major_opcode]; i++)
@@ -1536,14 +1541,19 @@ m68k_scan_mask (bfd_vma memaddr, disassemble_info *info,
       if (*args == '.')
 	args++;
 
+      /* Fetch the next two bytes if opcode is four-bytes long.  */
+      if ((0xffff & match) != 0)
+	{
+	  if (!FETCH_DATA (info, buffer + 4))
+	    return -1;
+	}
+
       if (((0xff & buffer[0] & (match >> 24)) == (0xff & (opcode >> 24)))
 	  && ((0xff & buffer[1] & (match >> 16)) == (0xff & (opcode >> 16)))
-	  /* Only fetch the next two bytes if we need to.  */
 	  && (((0xffff & match) == 0)
-	      ||
-	      (FETCH_DATA (info, buffer + 4)
-	       && ((0xff & buffer[2] & (match >> 8)) == (0xff & (opcode >> 8)))
-	       && ((0xff & buffer[3] & match) == (0xff & opcode)))
+	      /* Match the next two bytes if opcode is four-bytes long.  */
+	      || (((0xff & buffer[2] & (match >> 8)) == (0xff & (opcode >> 8)))
+		  && ((0xff & buffer[3] & match) == (0xff & opcode)))
 	      )
 	  && (opc->arch & arch_mask) != 0)
 	{
@@ -1628,7 +1638,7 @@ print_insn_m68k (bfd_vma memaddr, disassemble_info *info)
       /* First try printing an m680x0 instruction.  Try printing a Coldfire
 	 one if that fails.  */
       val = m68k_scan_mask (memaddr, info, m68k_mask);
-      if (val == 0)
+      if (val <= 0)
 	val = m68k_scan_mask (memaddr, info, mcf_mask);
     }
   else