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?
2016-11-18 Jakub Jelinek <[email protected]>
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
