On 11/02/2016 01:20 PM, Bernd Schmidt wrote:
On 10/29/2016 06:21 PM, Jeff Law wrote:

On a small number of ports, we only have 2 defined register classes.
NO_REGS and ALL_REGS.  Examples would include nvptx and vax.

So let's look at check_and_process_move from lra-constraints.c:

  sclass = dclass = NO_REGS;
  if (REG_P (dreg))
    dclass = get_reg_class (REGNO (dreg));
  if (dclass == ALL_REGS)
    /* ALL_REGS is used for new pseudos created by transformations
       like reload of SUBREG_REG (see function
       simplify_operand_subreg).  We don't know their class yet.  We
       should figure out the class from processing the insn
       constraints not in this fast path function.  Even if ALL_REGS
       were a right class for the pseudo, secondary_... hooks usually
       are not define for ALL_REGS.  */
    return false;
  [ ... ]
  /* Set up hard register for a reload pseudo for hook
     secondary_reload because some targets just ignore unassigned
     pseudos in the hook.  */
  if (dclass != NO_REGS && lra_get_regno_hard_regno (REGNO (dreg)) < 0)
    {
      dregno = REGNO (dreg);
      reg_renumber[dregno] = ira_class_hard_regs[dclass][0];
    }


The reference to ira_class_hard_regs is flagged by VRP as having an
out-of-bounds array reference.  WTF!?  Of course it's pretty obvious
once you look at the dumps...

On most targets dclass in this code will have a VR_VARYING range and as
a result no warning will be issued.  But on the ptx and vax ports VRP is
able to give us the range ~[NO_REGS,ALL_REGS]  aka ~[0,1]     The
out-of-range array index is now obvious.

So I tried to look up the rules for enum values and it seems like we
can't prove that the code in the if statement is dead. However, can't we
at least prove that it is "dead enough" for warning purposes?
For both C & C++ you can shove in a value outside the range of the enum. There is new strongly typed enum in C++11, but I didn't figure converting to those would be well received :-) So for the given code the range really is ~[0,1] and we can overflow the array bounds.


Ok for the trunk?

Hmm, seems like a deficiency in the warning code TBH, but I guess this
patch can't hurt. Maybe open a PR for improving the warning.
I guess we could enhance the warning on the assumption that getting a value that is not one of the enumeration constants. That's assuming we've got the right type in the right place. I'll take a look and see before going forward.

jeff

Reply via email to