On November 18, 2016 10:41:06 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>Hi!
>
>The following testcase ICEs because of buffer overflow in the
>expand_target_clones function. The main bug is that get_attr_str
>doesn't count the commas in the strings, so while it handles
>__attribute__((target_clones ("avx", "foo", "avx2", "avx512f",
>"default")))
>it doesn't handle
>__attribute__((target_clones ("avx,foo,avx2,avx512f,default")))
>which is also meant to be valid.
>This is fixed by the get_attr_str hunk.
>The rest of the changes are to avoid leaking memory, avoid double
>diagnostics (if targetm.target_option.valid_attribute_p fails,
>it has reported errors already, so there is no point to report
>further error or warning), fixing location_t of the diagnostics
>(targetm.target_option.valid_attribute_p uses input_location),
>and various formatting fixes and cleanups.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
>2016-11-18 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/78419
> * multiple_target.c (get_attr_len): Start with argnum and increment
> argnum on every arg. Use strchr in a loop instead of counting commas
> manually.
> (get_attr_str): Increment argnum for every comma in the string.
> (separate_attrs): Use for instead of while loop, simplify.
> (expand_target_clones): Rename defenition argument to definition.
> Free attrs and attr_str even when diagnosing errors. Temporarily
> change input_location around targetm.target_option.valid_attribute_p
> calls. Don't emit warning or errors if that function fails.
>
> * gcc.target/i386/pr78419.c: New test.
>
>--- gcc/multiple_target.c.jj 2016-08-29 12:17:37.000000000 +0200
>+++ gcc/multiple_target.c 2016-11-18 14:14:11.684489030 +0100
>@@ -101,22 +101,18 @@ get_attr_len (tree arglist)
> {
> tree arg;
> int str_len_sum = 0;
>- int argnum = 1;
>+ int argnum = 0;
>
> for (arg = arglist; arg; arg = TREE_CHAIN (arg))
> {
>- unsigned int i;
> const char *str = TREE_STRING_POINTER (TREE_VALUE (arg));
>- int len = strlen (str);
>-
>+ size_t len = strlen (str);
> str_len_sum += len + 1;
>- if (arg != arglist)
>+ for (const char *p = strchr (str, ','); p; p = strchr (p + 1,
>','))
> argnum++;
>- for (i = 0; i < strlen (str); i++)
>- if (str[i] == ',')
>- argnum++;
>+ argnum++;
> }
>- if (argnum == 1)
>+ if (argnum <= 1)
> return -1;
> return str_len_sum;
> }
>@@ -134,8 +130,9 @@ get_attr_str (tree arglist, char *attr_s
> for (arg = arglist; arg; arg = TREE_CHAIN (arg))
> {
> const char *str = TREE_STRING_POINTER (TREE_VALUE (arg));
>-
> size_t len = strlen (str);
>+ for (const char *p = strchr (str, ','); p; p = strchr (p + 1,
>','))
>+ argnum++;
> memcpy (attr_str + str_len_sum, str, len);
> attr_str[str_len_sum + len] = TREE_CHAIN (arg) ? ',' : '\0';
> str_len_sum += len + 1;
>@@ -152,19 +149,16 @@ separate_attrs (char *attr_str, char **a
> {
> int i = 0;
> bool has_default = false;
>- char *attr = strtok (attr_str, ",");
>
>- while (attr != NULL)
>+ for (char *attr = strtok (attr_str, ",");
>+ attr != NULL; attr = strtok (NULL, ","))
> {
> if (strcmp (attr, "default") == 0)
> {
> has_default = true;
>- attr = strtok (NULL, ",");
> continue;
> }
>- attrs[i] = attr;
>- attr = strtok (NULL, ",");
>- i++;
>+ attrs[i++] = attr;
> }
> if (!has_default)
> return -1;
>@@ -235,7 +229,7 @@ create_target_clone (cgraph_node *node,
> create the appropriate clone for each valid target attribute. */
>
> static bool
>-expand_target_clones (struct cgraph_node *node, bool defenition)
>+expand_target_clones (struct cgraph_node *node, bool definition)
> {
> int i;
> /* Parsing target attributes separated by comma. */
>@@ -266,6 +260,8 @@ expand_target_clones (struct cgraph_node
> {
> error_at (DECL_SOURCE_LOCATION (node->decl),
> "default target was not set");
>+ XDELETEVEC (attrs);
>+ XDELETEVEC (attr_str);
> return false;
> }
>
>@@ -286,22 +282,24 @@ expand_target_clones (struct cgraph_node
>
> create_new_asm_name (attr, suffix);
> /* Create new target clone. */
>- cgraph_node *new_node = create_target_clone (node, defenition,
>suffix);
>+ cgraph_node *new_node = create_target_clone (node, definition,
>suffix);
> XDELETEVEC (suffix);
>
> /* Set new attribute for the clone. */
> tree attributes = make_attribute ("target", attr,
> DECL_ATTRIBUTES (new_node->decl));
> DECL_ATTRIBUTES (new_node->decl) = attributes;
>+ location_t saved_loc = input_location;
>+ input_location = DECL_SOURCE_LOCATION (node->decl);
> if (!targetm.target_option.valid_attribute_p (new_node->decl, NULL,
>- TREE_VALUE (attributes), 0))
>+ TREE_VALUE (attributes),
>+ 0))
> {
>- warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
>- "attribute(target_clones(\"%s\")) is not "
>- "valid for current target", attr);
>+ input_location = saved_loc;
> continue;
> }
>
>+ input_location = saved_loc;
> decl2_v = new_node->function_version ();
> if (decl2_v != NULL)
> continue;
>@@ -320,22 +318,20 @@ expand_target_clones (struct cgraph_node
> DECL_FUNCTION_VERSIONED (new_node->decl) = 1;
> }
>
>- /* Setting new attribute to initial function. */
>- tree attributes = make_attribute ("target", "default",
>- DECL_ATTRIBUTES (node->decl));
>- DECL_ATTRIBUTES (node->decl) = attributes;
>- if (!targetm.target_option.valid_attribute_p (node->decl, NULL,
>- TREE_VALUE (attributes), 0))
>- {
>- error_at (DECL_SOURCE_LOCATION (node->decl),
>- "attribute(target_clones(\"default\")) is not "
>- "valid for current target");
>- return false;
>- }
>-
> XDELETEVEC (attrs);
> XDELETEVEC (attr_str);
>- return true;
>+
>+ /* Setting new attribute to initial function. */
>+ tree attributes = make_attribute ("target", "default",
>+ DECL_ATTRIBUTES (node->decl));
>+ DECL_ATTRIBUTES (node->decl) = attributes;
>+ location_t saved_loc = input_location;
>+ input_location = DECL_SOURCE_LOCATION (node->decl);
>+ bool ret
>+ = targetm.target_option.valid_attribute_p (node->decl, NULL,
>+ TREE_VALUE (attributes), 0);
>+ input_location = saved_loc;
>+ return ret;
> }
>
> static bool target_clone_pass;
>--- gcc/testsuite/gcc.target/i386/pr78419.c.jj 2016-11-18
>14:17:32.500967895 +0100
>+++ gcc/testsuite/gcc.target/i386/pr78419.c 2016-11-18
>14:17:09.000000000 +0100
>@@ -0,0 +1,23 @@
>+/* PR middle-end/78419 */
>+/* { dg-do compile } */
>+
>+static double bar (double *__restrict, double *__restrict, int)
>+__attribute__ ((target_clones("avx,foo,avx2,avx512f,default")));
>+
>+double
>+foo (double *__restrict a, double *__restrict b, int n)
>+{
>+ return bar (a,b,n);
>+}
>+
>+double
>+bar (double *__restrict a, double *__restrict b, int n) /* { dg-error
>"attribute\[^\n\r]*foo\[^\n\r]* is unknown" } */
>+{
>+ double s;
>+ int i;
>+ s = 0.0;
>+ for (i=0; i<n; i++)
>+ s += a[i] + b[i];
>+
>+ return s;
>+}
>
> Jakub
