diff mbox series

correctly handle attribute access (PR 92721)

Message ID d55a011f-f784-6841-fe7c-bf7aebf0f636@gmail.com
State New
Headers show
Series correctly handle attribute access (PR 92721) | expand

Commit Message

Martin Sebor Feb. 25, 2020, 10:57 p.m. UTC
Release checking has exposed a bug in the transformation of
attribute access to the condensed internal format: the handler
modifies the declaration's type in place, which subsequently
triggers an ICE.  A fix also exposed another bug where
the condensed format sets the attribute argument's value
directly to a STRING_CST rather than to
TREE_LIST (TREE_VALUE (STRING_CST)) as is apparently expected.

The patch that corrects both of these problems and that's been
tested on x86_64-linux is attached.  I plan to commit it tomorrow.

Martin
diff mbox series

Patch

PR middle-end/92721 - checking ICE on attribute access redeclaration

gcc/c-family/ChangeLog:

	PR c++/92721
	* c-attribs.c (append_access_attrs): Correctly handle attribute.
	(handle_access_attribute): Same.

gcc/ChangeLog:

	PR c++/92721
	* calls.c (init_attr_rdwr_indices): Correctly handle attribute.

gcc/testsuite/ChangeLog:

	PR c++/92721
	g++.dg/ext/attr-access.C: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 2ea5fd5ff46..9abf81d0248 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -3845,6 +3845,10 @@  append_access_attrs (tree t, tree attrs, const char *attrstr,
 
   if (tree acs = lookup_attribute ("access", attrs))
     {
+      /* The TREE_VALUE of an attribute is a TREE_LIST whose TREE_VALUE
+	 is the attribute argument's value.  */
+      acs = TREE_VALUE (acs);
+      gcc_assert (TREE_CODE (acs) == TREE_LIST);
       acs = TREE_VALUE (acs);
       gcc_assert (TREE_CODE (acs) == STRING_CST);
 
@@ -3971,11 +3975,20 @@  handle_access_attribute (tree *node, tree name, tree args,
       return NULL_TREE;
     }
 
+  tree access_mode = TREE_VALUE (args);
+  if (TREE_CODE (access_mode) == STRING_CST)
+    {
+      /* This must be a recursive call to handle the condensed internal
+	 form of the attribute (see below).  Since all validation has
+	 been done simply return here, accepting the attribute as is.  */
+      *no_add_attrs = false;
+      return NULL_TREE;
+    }
+
   /* Set to true when the access mode has the form of a function call
      as in 'attribute (read_only (1, 2))'.  That's an easy mistake to
      make and so worth a special diagnostic.  */
   bool funcall = false;
-  tree access_mode = TREE_VALUE (args);
   if (TREE_CODE (access_mode) == CALL_EXPR)
     {
       access_mode = CALL_EXPR_FN (access_mode);
@@ -4170,6 +4183,7 @@  handle_access_attribute (tree *node, tree name, tree args,
   /* Replace any existing access attribute specification with
      the concatenation above.  */
   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);
 
   if (node[1])
@@ -4182,11 +4196,14 @@  handle_access_attribute (tree *node, tree name, tree args,
 	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;
     }
 
-  TYPE_ATTRIBUTES (*node) = new_attrs;
+  /* Recursively call self to "replace" the documented/external form
+     of the attribute with the condensend internal form.  */
+  decl_attributes (node, new_attrs, flags);
   return NULL_TREE;
 }
 
diff --git a/gcc/calls.c b/gcc/calls.c
index d1c53171176..4c3a8f3c215 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1891,8 +1891,13 @@  init_attr_rdwr_indices (rdwr_map *rwm, tree fntype)
   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; )
     {
diff --git a/gcc/testsuite/g++.dg/ext/attr-access.C b/gcc/testsuite/g++.dg/ext/attr-access.C
new file mode 100644
index 00000000000..fcb54cd8861
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-access.C
@@ -0,0 +1,109 @@ 
+/* PR middle-end/92721 - checking ICE on attribute access redeclaration
+   Test to verify the handling of attribute access combining multiple
+   declarations of the same function.
+   { dg-do compile }
+   { dg-options "-O0 -Wall -ftrack-macro-expansion=0" } */
+
+#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;
+
+void RO (1) RO (1) rop1_ror2 (const int32_t*, const int32_t&);
+void RO (1) RO (2) rop1_ror2 (const int32_t*, const int32_t&);
+void RO (2) RO (1) rop1_ror2 (const int32_t*, const int32_t&);
+void RO (2) RO (2) rop1_ror2 (const int32_t*, const int32_t&);
+
+void RW (1) RW (1) rdwrp1_rdwrr2 (int32_t*, int32_t&);
+void RW (1) RW (2) rdwrp1_rdwrr2 (int32_t*, int32_t&);
+void RW (2) RW (1) rdwrp1_rdwrr2 (int32_t*, int32_t&);
+void RW (2) RW (2) rdwrp1_rdwrr2 (int32_t*, int32_t&);
+
+void WO (1) WO (1) wop1_wor2 (int32_t*, int32_t&);
+void WO (1) WO (2) wop1_wor2 (int32_t*, int32_t&);
+void WO (2) WO (1) wop1_wor2 (int32_t*, int32_t&);
+void WO (2) WO (2) wop1_wor2 (int32_t*, int32_t&);
+
+
+// Verify that everything works even with no optimization.
+
+void call_rop1_ror2_O0 (void)
+{
+  const int32_t x[1] = { };
+
+  rop1_ror2 (x, x[0]);
+  rop1_ror2 (x, x[1]);            // { dg-warning "reading 4 bytes from a region of size 0" }
+  rop1_ror2 (x + 1, x[0]);        // { dg-warning "reading 4 bytes from a region of size 0" }
+}
+
+void call_rdwrp1_rdwrr2_O0 (void)
+{
+  int32_t x[1];
+
+  rdwrp1_rdwrr2 (x, x[0]);
+  rdwrp1_rdwrr2 (x, x[1]);        // { dg-warning "writing 4 bytes into a region of size 0" }
+  rdwrp1_rdwrr2 (x + 1, x[0]);    // { dg-warning "writing 4 bytes into a region of size 0" }
+}
+
+void call_wop1_wor2_O0 (void)
+{
+  int32_t x[1];
+
+  wop1_wor2 (x, x[0]);
+  wop1_wor2 (x, x[1]);            // { dg-warning "writing 4 bytes into a region of size 0" }
+  wop1_wor2 (x + 1, x[0]);        // { dg-warning "writing 4 bytes into a region of size 0" }
+}
+
+
+// Verify that everything still works with -O1.
+
+#pragma GCC optimize "1"
+
+void call_rop1_ror2_O1 (void)
+{
+  const int32_t x[1] = { 1 };
+  const int32_t *p0 = x, &r0 = x[0];
+  const int32_t *p1 = (const int32_t*)((const char*)p0 + 1);
+  const int32_t &r2 = *(const int32_t*)((const char*)p1 + 1);
+
+  rop1_ror2 (x, x[0]);
+  rop1_ror2 (x, x[1]);            // { dg-warning "reading 4 bytes from a region of size 0" }
+  rop1_ror2 (x + 1, x[0]);        // { dg-warning "reading 4 bytes from a region of size 0" }
+
+  rop1_ror2 (p0, r0);
+  rop1_ror2 (p0, r2);             // { dg-warning "reading 4 bytes from a region of size 2" }
+  rop1_ror2 (p1, r0);             // { dg-warning "reading 4 bytes from a region of size 3" }
+}
+
+void call_rdwrp1_rdwrr2_O1 (void)
+{
+  int32_t x[1];
+  int32_t *p0 = x, &r0 = x[0];
+  int32_t *p1 = (int32_t*)((char*)p0 + 1);
+  int32_t &r2 = *(int32_t*)((char*)p1 + 1);
+
+  rdwrp1_rdwrr2 (x, x[0]);
+  rdwrp1_rdwrr2 (x, x[1]);        // { dg-warning "writing 4 bytes into a region of size 0" }
+  rdwrp1_rdwrr2 (x + 1, x[0]);    // { dg-warning "writing 4 bytes into a region of size 0" }
+
+  rdwrp1_rdwrr2 (p0, r0);
+  rdwrp1_rdwrr2 (p0, r2);         // { dg-warning "writing 4 bytes into a region of size 2" }
+  rdwrp1_rdwrr2 (p1, r0);         // { dg-warning "writing 4 bytes into a region of size 3" }
+}
+
+void call_wop1_wor2_O1 (void)
+{
+  int32_t x[1];
+  int32_t *p0 = x, &r0 = x[0];
+  int32_t *p1 = (int32_t*)((char*)p0 + 1);
+  int32_t &r2 = *(int32_t*)((char*)p1 + 1);
+
+  wop1_wor2 (x, x[0]);
+  wop1_wor2 (x, x[1]);            // { dg-warning "writing 4 bytes into a region of size 0" }
+  wop1_wor2 (x + 1, x[0]);        // { dg-warning "writing 4 bytes into a region of size 0" }
+
+  wop1_wor2 (p0, r0);
+  wop1_wor2 (p0, r2);             // { dg-warning "writing 4 bytes into a region of size 2" }
+  wop1_wor2 (p1, r0);             // { dg-warning "writing 4 bytes into a region of size 3" }
+}