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 <ja...@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
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
