Re: out of bounds access in insn-automata.c

[Aldy Hernandez <aldyh at redhat dot com>]

On 03/23/2016 10:25 AM, Bernd Schmidt wrote:
> On 03/23/2016 07:32 AM, Aldy Hernandez wrote:
> > int
> > maximal_insn_latency (rtx insn)
> > {
> >    int insn_code;
> >
> >    if (insn == 0)
> >      insn_code = DFA__ADVANCE_CYCLE;
> >
> >
> >    else
> >      {
> >        insn_code = dfa_insn_code (as_a <rtx_insn *> (insn));
> >        if (insn_code > DFA__ADVANCE_CYCLE)
> >          return 0;
> >      }
> >    return internal_maximal_insn_latency (insn_code, insn);
> > }
> >
> > In the case where insn==0, insn_code is set to the size of
> > default_latencies[] which will get accessed in the return.
> >
> > Does insn==0 never happen?
>
> I suspect it never happens in this function. I'd add a gcc_assert to
> that effect and try a bootstrap/test. Hmm, it seems to be a sel-sched
> only thing so a normal bootstrap would be meaningless, but from the
> context it looks fairly clearly like insn is always nonnull.

Vlad.  Bernd.  Thanks for your input.

I've added the assert on the caller (maximal_insn_latency), because as 
you mentioned, the helper function is used for other things in which 
insn==0 can happen.

Now we generate:


int
maximal_insn_latency (rtx insn)
{
  int insn_code;
  gcc_assert (insn != 0);	// <<<<--- Added code.

  if (insn == 0)
    insn_code = DFA__ADVANCE_CYCLE;


  else
    {
      insn_code = dfa_insn_code (as_a <rtx_insn *> (insn));
      if (insn_code > DFA__ADVANCE_CYCLE)
        return 0;
    }
  return internal_maximal_insn_latency (insn_code, insn);
}

> It looks like this block of code is written by a helper function that is
> really intended for other purposes than for maximal_insn_latency. Might
> be worth changing to
>   int insn_code = dfa_insn_code (as_a <rtx_insn *> (insn));
>   gcc_assert (insn_code <= DFA__ADVANCE_CYCLE);

dfa_insn_code_* and friends can return > DFA__ADVANCE_CYCLE so I can't 
put that assert on the helper function.

While I was at it, I changed the helper function comment to reflect what 
it has been generating.  It was wrong.

First round of tests was ok, but test machine died.  OK pending tests?

Aldy

Attachment: curr
Description: Text document