The recent fix to avoid modifying in place the type argument in
handle_access_attribute (PR 92721) was incomplete and didn't fully
resolve the problem (an ICE in the C++ front-end). The attached
patch removes the remaining modification that causes the ICE. In
addition, the change adjusts checking calls to functions declared
with the attribute to scan all its instances.
The attached patch was tested on x86_64-linux.
I'm puzzled that the ICE only triggers in the C++ front-end and not
also in C. The code that issues the internal error is in comptypes()
in cp/typeck.c and has this comment:
/* The two types are structurally equivalent, but their
canonical types were different. This is a failure of the
canonical type propagation code.*/
internal_error
("canonical types differ for identical types %qT and %qT",
t1, t2);
What is "the canonical type propagation code" it refers to? Is it
valid to modify the type in an attribute handler and does it in C++
just require updating some other code somewhere else?
If not, and if modifying a type in place is not valid I'd expect
decl_attributes to enforce it. I looked for other attribute handlers
to see if any also modify the type argument in place (by adding or
removing attributes) but couldn't really find any. So if it is
invalid I'd like to add such an assertion (probably for GCC 11) but
before I do I want to make sure I'm not missing something.
Martin
PR c++/94098 - ICE on attribute access redeclaration
gcc/c-family/ChangeLog:
PR c++/94098
* c-attribs.c (handle_access_attribute): Avoid setting TYPE_ATTRIBUTES
here.
gcc/ChangeLog:
PR c++/94098
* calls.c (init_attr_rdwr_indices): Iterate over all access attributes.
gcc/testsuite/ChangeLog:
PR c++/94098
* g++.dg/ext/attr-access-2.C: New test.
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..9fa2a8ecefc 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4190,15 +4190,13 @@ handle_access_attribute (tree *node, tree name, tree args,
{
/* Repeat for the previously declared type. */
attrs = TYPE_ATTRIBUTES (TREE_TYPE (node[1]));
- tree new_attrs
- = append_access_attrs (node[1], attrs, attrstr, code, idxs);
- if (!new_attrs)
+ tree attrs1 = append_access_attrs (node[1], attrs, attrstr, code, idxs);
+ if (!attrs1)
return NULL_TREE;
attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs);
- new_attrs = tree_cons (NULL_TREE, new_attrs, NULL_TREE);
- new_attrs = tree_cons (name, new_attrs, attrs);
- TYPE_ATTRIBUTES (TREE_TYPE (node[1])) = new_attrs;
+ attrs1 = tree_cons (NULL_TREE, attrs1, NULL_TREE);
+ new_attrs = tree_cons (name, attrs1, attrs);
}
/* Recursively call self to "replace" the documented/external form
diff --git a/gcc/calls.c b/gcc/calls.c
index 4c3a8f3c215..5bd922779af 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1873,7 +1873,7 @@ struct rdwr_access_hash: int_hash<int, -1> { };
typedef hash_map<rdwr_access_hash, attr_access> rdwr_map;
/* Initialize a mapping for a call to function FNDECL declared with
- attribute access. Each attribute poisitional operand inserts one
+ attribute access. Each attribute positional operand inserts one
entry into the mapping with the operand number as the key. */
static void
@@ -1882,54 +1882,50 @@ init_attr_rdwr_indices (rdwr_map *rwm, tree fntype)
if (!fntype)
return;
- tree access = TYPE_ATTRIBUTES (fntype);
- /* If the function's type has no attributes there's nothing to do. */
- if (!access)
- return;
-
- access = lookup_attribute ("access", access);
- if (!access)
- return;
-
- /* The TREE_VALUE of an attribute is a TREE_LIST whose TREE_VALUE
- is the attribute argument's value. */
- tree mode = TREE_VALUE (access);
- gcc_assert (TREE_CODE (mode) == TREE_LIST);
- mode = TREE_VALUE (mode);
- gcc_assert (TREE_CODE (mode) == STRING_CST);
-
- const char *modestr = TREE_STRING_POINTER (mode);
- for (const char *m = modestr; *m; )
+ for (tree access = TYPE_ATTRIBUTES (fntype);
+ (access = lookup_attribute ("access", access));
+ access = TREE_CHAIN (access))
{
- attr_access acc = { };
-
- switch (*m)
+ /* The TREE_VALUE of an attribute is a TREE_LIST whose TREE_VALUE
+ is the attribute argument's value. */
+ tree mode = TREE_VALUE (access);
+ gcc_assert (TREE_CODE (mode) == TREE_LIST);
+ mode = TREE_VALUE (mode);
+ gcc_assert (TREE_CODE (mode) == STRING_CST);
+
+ const char *modestr = TREE_STRING_POINTER (mode);
+ for (const char *m = modestr; *m; )
{
- case 'r': acc.mode = acc.read_only; break;
- case 'w': acc.mode = acc.write_only; break;
- default: acc.mode = acc.read_write; break;
- }
+ attr_access acc = { };
- char *end;
- acc.ptrarg = strtoul (++m, &end, 10);
- m = end;
- if (*m == ',')
- {
- acc.sizarg = strtoul (++m, &end, 10);
+ switch (*m)
+ {
+ case 'r': acc.mode = acc.read_only; break;
+ case 'w': acc.mode = acc.write_only; break;
+ default: acc.mode = acc.read_write; break;
+ }
+
+ char *end;
+ acc.ptrarg = strtoul (++m, &end, 10);
m = end;
- }
- else
- acc.sizarg = UINT_MAX;
+ if (*m == ',')
+ {
+ acc.sizarg = strtoul (++m, &end, 10);
+ m = end;
+ }
+ else
+ acc.sizarg = UINT_MAX;
- acc.ptr = NULL_TREE;
- acc.size = NULL_TREE;
+ acc.ptr = NULL_TREE;
+ acc.size = NULL_TREE;
- /* Unconditionally add an entry for the required pointer
- operand of the attribute, and one for the optional size
- operand when it's specified. */
- rwm->put (acc.ptrarg, acc);
- if (acc.sizarg != UINT_MAX)
- rwm->put (acc.sizarg, acc);
+ /* Unconditionally add an entry for the required pointer
+ operand of the attribute, and one for the optional size
+ operand when it's specified. */
+ rwm->put (acc.ptrarg, acc);
+ if (acc.sizarg != UINT_MAX)
+ rwm->put (acc.sizarg, acc);
+ }
}
}
diff --git a/gcc/testsuite/g++.dg/ext/attr-access-2.C b/gcc/testsuite/g++.dg/ext/attr-access-2.C
new file mode 100644
index 00000000000..46f9075a6c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-access-2.C
@@ -0,0 +1,88 @@
+/* PR c++/94098 - checking ICE on attribute access redeclaration
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+#define RO(...) __attribute__ ((access (read_only, __VA_ARGS__)))
+#define RW(...) __attribute__ ((access (read_write, __VA_ARGS__)))
+#define WO(...) __attribute__ ((access (write_only, __VA_ARGS__)))
+
+typedef __INT32_TYPE__ int32_t;
+
+int rdwr1_2_3_4 (void*, void*, void*, void*);
+int RW (1) rdwr1_2_3_4 (void*, void*, void*, void*);
+int RW (2) rdwr1_2_3_4 (void*, void*, void*, void*);
+int RW (3) rdwr1_2_3_4 (void*, void*, void*, void*);
+int RW (4) rdwr1_2_3_4 (void*, void*, void*, void*);
+
+extern int32_t x[1];
+
+void call_rdwrp1_2_3_4 (void)
+{
+ rdwr1_2_3_4 (x, x, x, x);
+ rdwr1_2_3_4 (x, x, x, x + 1); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwr1_2_3_4 (x, x, x + 1, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwr1_2_3_4 (x, x + 1, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwr1_2_3_4 (x + 1, x, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+int rdwr4_3_2_1 (void*, void*, void*, void*);
+int RW (4) rdwr4_3_2_1 (void*, void*, void*, void*);
+int RW (3) rdwr4_3_2_1 (void*, void*, void*, void*);
+int RW (2) rdwr4_3_2_1 (void*, void*, void*, void*);
+int RW (1) rdwr4_3_2_1 (void*, void*, void*, void*);
+
+void call_rdwr4_3_2_1 (void)
+{
+ rdwr4_3_2_1 (x, x, x, x);
+ rdwr4_3_2_1 (x, x, x, x + 1); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwr4_3_2_1 (x, x, x + 1, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwr4_3_2_1 (x, x + 1, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwr4_3_2_1 (x + 1, x, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+int rdwrall (void*, void*, void*, void*);
+int RW (1) rdwrall (void*, void*, void*, void*);
+int RW (1) RW (2) rdwrall (void*, void*, void*, void*);
+int RW (1) RW (2) RW (3) rdwrall (void*, void*, void*, void*);
+int RW (1) RW (2) RW (3) RW (4) rdwrall (void*, void*, void*, void*);
+
+void call_rdwrall (void)
+{
+ rdwrall (x, x, x, x);
+ rdwrall (x, x, x, x + 1); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwrall (x, x, x + 1, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwrall (x, x + 1, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ rdwrall (x + 1, x, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify the attribute is a part of the function's type.
+typedef __typeof__ (rdwrall) F;
+
+void call_fnptr_typeof (F *f)
+{
+ f (x, x, x, x);
+ f (x, x, x, x + 1); // { dg-warning "\\\[-Wstringop-overflow" }
+ f (x, x, x + 1, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ f (x, x + 1, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+ f (x + 1, x, x, x); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+// Verify the attribute is effective on a typedef.
+typedef void FWRall (void*, void*, void*, void*);
+typedef RW (1) void FWRall (void*, void*, void*, void*);
+typedef RW (2) void FWRall (void*, void*, void*, void*);
+typedef RW (3) void FWRall (void*, void*, void*, void*);
+typedef RW (4) void FWRall (void*, void*, void*, void*);
+
+void call_fnptr (FWRall *f)
+{
+ f (x, x, x, x);
+ f (x, x, x, x + 1); // { dg-warning "\\\[-Wstringop-overflow" "pr94171" { xfail *-*-* } }
+ f (x, x, x + 1, x); // { dg-warning "\\\[-Wstringop-overflow" "pr94171" { xfail *-*-* } }
+ f (x, x + 1, x, x); // { dg-warning "\\\[-Wstringop-overflow" "pr94171" { xfail *-*-* } }
+ f (x + 1, x, x, x); // { dg-warning "\\\[-Wstringop-overflow" "pr94171" { xfail *-*-* } }
+}
