Fix another CONSTRAINT_LEN related bug (PR middle-end/84831)

Hi!

I thought the vregs pass is the first one to analyze constraints,
but I was wrong, already before that parse_{output,input}_constraint
are called, already during GIMPLE passes.  They don't really fail miserably
if , appears in the middle of multi-letter constraint (unlike e.g. the RA),
which is what my previous patch was fixing, but care if '\0' appears
in the middle of multi-letter constraint, because then we continue walking
random characters after the constraint string.  parse_input_constraint
is actually fine, because it has c_len = strlen (constraint); and uses
j < c_len, but parse_output_constraint doesn't do that.  While doing it
is possible, I think it is needlessly slow (needs to walk the constraint
twice), so this patch instead makes sure there are no '\0's in the middle
of constraints, if there are, doesn't jump over it.

The patch is larger because of reindentation, here is diff -upbd for
easier understanding what has changed.


	Jakub

On Mon, 12 Mar 2018, Jakub Jelinek wrote:

> Hi!
> 
> I thought the vregs pass is the first one to analyze constraints,
> but I was wrong, already before that parse_{output,input}_constraint
> are called, already during GIMPLE passes.  They don't really fail miserably
> if , appears in the middle of multi-letter constraint (unlike e.g. the RA),
> which is what my previous patch was fixing, but care if '\0' appears
> in the middle of multi-letter constraint, because then we continue walking
> random characters after the constraint string.  parse_input_constraint
> is actually fine, because it has c_len = strlen (constraint); and uses
> j < c_len, but parse_output_constraint doesn't do that.  While doing it
> is possible, I think it is needlessly slow (needs to walk the constraint
> twice), so this patch instead makes sure there are no '\0's in the middle
> of constraints, if there are, doesn't jump over it.
> 
> The patch is larger because of reindentation, here is diff -upbd for
> easier understanding what has changed.
> 
> --- gcc/stmt.c.jj	2018-01-03 10:19:55.150533956 +0100
> +++ gcc/stmt.c	2018-03-12 13:25:03.790733765 +0100
> @@ -247,7 +247,8 @@ parse_output_constraint (const char **co
>      }
>  
>    /* Loop through the constraint string.  */
> -  for (p = constraint + 1; *p; p += CONSTRAINT_LEN (*p, p))
> +  for (p = constraint + 1; *p; )
> +    {
>      switch (*p)
>        {
>        case '+':
> @@ -304,6 +305,11 @@ parse_output_constraint (const char **co
>  	break;
>        }
>  
> +      for (size_t len = CONSTRAINT_LEN (*p, p); len; len--, p++)
> +	if (*p == '\0')
> +	  break;
> +    }
> +
>    return true;
>  }
>  
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> 2018-03-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/84831
> 	* stmt.c (parse_output_constraint): If the CONSTRAINT_LEN (*p, p)
> 	characters starting at p contain '\0' character, don't look beyond
> 	that.
> 
> --- gcc/stmt.c.jj	2018-01-03 10:19:55.150533956 +0100
> +++ gcc/stmt.c	2018-03-12 13:25:03.790733765 +0100
> @@ -247,62 +247,68 @@ parse_output_constraint (const char **co
>      }
>  
>    /* Loop through the constraint string.  */
> -  for (p = constraint + 1; *p; p += CONSTRAINT_LEN (*p, p))
> -    switch (*p)
> -      {
> -      case '+':
> -      case '=':
> -	error ("operand constraint contains incorrectly positioned "
> -	       "%<+%> or %<=%>");
> -	return false;
> -
> -      case '%':
> -	if (operand_num + 1 == ninputs + noutputs)
> -	  {
> -	    error ("%<%%%> constraint used with last operand");
> -	    return false;
> -	  }
> -	break;
> -
> -      case '?':  case '!':  case '*':  case '&':  case '#':
> -      case '$':  case '^':
> -      case 'E':  case 'F':  case 'G':  case 'H':
> -      case 's':  case 'i':  case 'n':
> -      case 'I':  case 'J':  case 'K':  case 'L':  case 'M':
> -      case 'N':  case 'O':  case 'P':  case ',':
> -	break;
> -
> -      case '0':  case '1':  case '2':  case '3':  case '4':
> -      case '5':  case '6':  case '7':  case '8':  case '9':
> -      case '[':
> -	error ("matching constraint not valid in output operand");
> -	return false;
> -
> -      case '<':  case '>':
> -	/* ??? Before flow, auto inc/dec insns are not supposed to exist,
> -	   excepting those that expand_call created.  So match memory
> -	   and hope.  */
> -	*allows_mem = true;
> -	break;
> -
> -      case 'g':  case 'X':
> -	*allows_reg = true;
> -	*allows_mem = true;
> -	break;
> -
> -      default:
> -	if (!ISALPHA (*p))
> -	  break;
> -	enum constraint_num cn = lookup_constraint (p);
> -	if (reg_class_for_constraint (cn) != NO_REGS
> -	    || insn_extra_address_constraint (cn))
> +  for (p = constraint + 1; *p; )
> +    {
> +      switch (*p)
> +	{
> +	case '+':
> +	case '=':
> +	  error ("operand constraint contains incorrectly positioned "
> +		 "%<+%> or %<=%>");
> +	  return false;
> +
> +	case '%':
> +	  if (operand_num + 1 == ninputs + noutputs)
> +	    {
> +	      error ("%<%%%> constraint used with last operand");
> +	      return false;
> +	    }
> +	  break;
> +
> +	case '?':  case '!':  case '*':  case '&':  case '#':
> +	case '$':  case '^':
> +	case 'E':  case 'F':  case 'G':  case 'H':
> +	case 's':  case 'i':  case 'n':
> +	case 'I':  case 'J':  case 'K':  case 'L':  case 'M':
> +	case 'N':  case 'O':  case 'P':  case ',':
> +	  break;
> +
> +	case '0':  case '1':  case '2':  case '3':  case '4':
> +	case '5':  case '6':  case '7':  case '8':  case '9':
> +	case '[':
> +	  error ("matching constraint not valid in output operand");
> +	  return false;
> +
> +	case '<':  case '>':
> +	  /* ??? Before flow, auto inc/dec insns are not supposed to exist,
> +	     excepting those that expand_call created.  So match memory
> +	     and hope.  */
> +	  *allows_mem = true;
> +	  break;
> +
> +	case 'g':  case 'X':
>  	  *allows_reg = true;
> -	else if (insn_extra_memory_constraint (cn))
>  	  *allows_mem = true;
> -	else
> -	  insn_extra_constraint_allows_reg_mem (cn, allows_reg, allows_mem);
> -	break;
> -      }
> +	  break;
> +
> +	default:
> +	  if (!ISALPHA (*p))
> +	    break;
> +	  enum constraint_num cn = lookup_constraint (p);
> +	  if (reg_class_for_constraint (cn) != NO_REGS
> +	      || insn_extra_address_constraint (cn))
> +	    *allows_reg = true;
> +	  else if (insn_extra_memory_constraint (cn))
> +	    *allows_mem = true;
> +	  else
> +	    insn_extra_constraint_allows_reg_mem (cn, allows_reg, allows_mem);
> +	  break;
> +	}
> +
> +      for (size_t len = CONSTRAINT_LEN (*p, p); len; len--, p++)
> +	if (*p == '\0')
> +	  break;
> +    }
>  
>    return true;
>  }
> 
> 	Jakub
> 
>