[6/8] Return -1 on memory error in print_insn_m68k
Commit Message
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.
Comments
On Thu, Jan 12, 2017 at 11:50:21AM +0000, Yao Qi wrote:
> 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.
Actually there is a good reason not to change that call, I just didn't
look at the code well enough and thought you'd accidentally missed
it. The previous patch is OK. The newest one would fail if you had
a 2-byte insn at the end of a segment and happened to try a 4-byte
insn match first.
On 17-01-13 01:08:14, Alan Modra wrote:
> On Thu, Jan 12, 2017 at 11:50:21AM +0000, Yao Qi wrote:
> > 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.
>
> Actually there is a good reason not to change that call, I just didn't
> look at the code well enough and thought you'd accidentally missed
> it. The previous patch is OK. The newest one would fail if you had
> a 2-byte insn at the end of a segment and happened to try a 4-byte
> insn match first.
>
Hi Alan,
This won't happen in the 2nd patch, because if the instruction is 2-byte,
it won't read and match the next 2 bytes. There is a guard
((0xffff & match) != 0) for read. Am I missing something?
On Thu, Jan 12, 2017 at 02:52:30PM +0000, Yao Qi wrote:
> On 17-01-13 01:08:14, Alan Modra wrote:
> > On Thu, Jan 12, 2017 at 11:50:21AM +0000, Yao Qi wrote:
> > > 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.
> >
> > Actually there is a good reason not to change that call, I just didn't
> > look at the code well enough and thought you'd accidentally missed
> > it. The previous patch is OK. The newest one would fail if you had
> > a 2-byte insn at the end of a segment and happened to try a 4-byte
> > insn match first.
> >
>
> Hi Alan,
> This won't happen in the 2nd patch, because if the instruction is 2-byte,
> it won't read and match the next 2 bytes. There is a guard
> ((0xffff & match) != 0) for read. Am I missing something?
The code is looping over an opcode table. You might try to match a
4-byte instruction from the opcode table before matching the 2-byte
instruction you have. If you exit the loop due to failing to read 4
bytes then you won't disassemble the instruction.
On 17-01-13 12:24:12, Alan Modra wrote:
> > > Actually there is a good reason not to change that call, I just didn't
> > > look at the code well enough and thought you'd accidentally missed
> > > it. The previous patch is OK. The newest one would fail if you had
> > > a 2-byte insn at the end of a segment and happened to try a 4-byte
> > > insn match first.
> > >
> >
> > Hi Alan,
> > This won't happen in the 2nd patch, because if the instruction is 2-byte,
> > it won't read and match the next 2 bytes. There is a guard
> > ((0xffff & match) != 0) for read. Am I missing something?
>
> The code is looping over an opcode table. You might try to match a
> 4-byte instruction from the opcode table before matching the 2-byte
> instruction you have. If you exit the loop due to failing to read 4
> bytes then you won't disassemble the instruction.
>
OK, the v1 is pushed in.
@@ -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