cgen: opcodes: Fix memory corruption in in lookup

  The buf variable is used after it is free'd.  This causes the lookups to
fail and also causes memory corruption.

Re-arrange the code a bit to make sure we always free memory before
returning. This was caught in openrisc testing, one of the only user of
this method.

opcodes/ChangeLog:

2017-02-09  Stafford Horne  <shorne@gmail.com>
	cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.
---
 opcodes/cgen-opc.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
  

Hi Stafford,
opcodes/ changes should be reviewed in binutils@sourceware.org.

On Wed, Feb 8, 2017 at 4:40 PM, Stafford Horne <shorne@gmail.com> wrote:
> The buf variable is used after it is free'd.  This causes the lookups to
> fail and also causes memory corruption.
>
> Re-arrange the code a bit to make sure we always free memory before
> returning. This was caught in openrisc testing, one of the only user of
> this method.
>
> opcodes/ChangeLog:
>
> 2017-02-09  Stafford Horne  <shorne@gmail.com>
>         cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.

ChangeLog format issue,

017-02-09  Stafford Horne  <shorne@gmail.com>

* cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.

> ---
>  opcodes/cgen-opc.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/opcodes/cgen-opc.c b/opcodes/cgen-opc.c
> index 72b4f05..40a6320 100644
> --- a/opcodes/cgen-opc.c
> +++ b/opcodes/cgen-opc.c
> @@ -463,7 +463,6 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>        buf = (unsigned char *) xmalloc (cd->max_insn_bitsize / 8);
>        cgen_put_insn_value (cd, buf, length, insn_int_value);
>        base_insn = insn_int_value;
> -      free (buf);
>      }
>    else
>      {
> @@ -475,7 +474,7 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>        base_insn = cgen_get_insn_value (cd, buf, length);
>      }
>
> -  if (!insn)
> +  if (insn == NULL)
>      {
>        const CGEN_INSN_LIST *insn_list;
>
> @@ -505,7 +504,8 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>                       /* sanity check */
>                       if (length != 0 && length != elength)
>                         abort ();
> -                     return insn;
> +                     /* found, done */
> +                     break;
>                     }
>                 }
>             }
> @@ -530,10 +530,12 @@ cgen_lookup_insn (CGEN_CPU_DESC cd,
>          Could relax this later if it ever proves useful.  */
>        if (length == 0)
>         abort ();
> -      return insn;
>      }
>
> -  return NULL;
> +  if (cd->int_insn_p)
> +    free (buf);
> +
> +  return insn;
>  }
>
>  /* Fill in the operand instances used by INSN whose operands are FIELDS.
> --
> 2.9.3
>
  

Hi Yao,

On Wed, Feb 08, 2017 at 05:01:05PM +0000, Yao Qi wrote:
> Hi Stafford,
> opcodes/ changes should be reviewed in binutils@sourceware.org.

Right, it was late when I sent this and I was working on a gdb bug.  So
just sent it here.  Resent now to binutils list.

> On Wed, Feb 8, 2017 at 4:40 PM, Stafford Horne <shorne@gmail.com> wrote:
> > The buf variable is used after it is free'd.  This causes the lookups to
> > fail and also causes memory corruption.
> >
> > Re-arrange the code a bit to make sure we always free memory before
> > returning. This was caught in openrisc testing, one of the only user of
> > this method.
> >
> > opcodes/ChangeLog:
> >
> > 2017-02-09  Stafford Horne  <shorne@gmail.com>
> >         cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.
> 
> ChangeLog format issue,
> 
> 017-02-09  Stafford Horne  <shorne@gmail.com>
> 
> * cgen-opc.c (cgen_lookup_insn): Fix memory corruption issue.

Thanks,

-Stafford