Hi!

On the following testcase we emit various (correct) -Wnonnull warnings
more than once, sometimes many times.  The problem on the reported memcpy
testcase is that glibc uses __attribute__((nonnull (1, 2))) and gcc
uses __attribute__((nonnull)) on the memset builtin and we end up with both of
the attributes (as they have different parameters and thus aren't merged).
The check_function_nonnull then for each nonnull attribute went through all
the arguments and warned if the argument matched the current nonnull
attribute (so, for the combination of glibc and gcc provided nonnull
argument 1 and argument 2 each matched twice, once the list variant, once
the non-argument variant).  The following patch fixes it by first looking
if there is any nonnull attribute without argument, then it warns about all
pointer arguments, or otherwise for each argument walks the list of
nonnull attributes and if any of them matches, warns.

tree-vrp.c apparently handled just the first nonnull attribute and not more
than one of them.  That seems to be all users of that attribute in GCC.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-07-19  Jakub Jelinek  <ja...@redhat.com>

        PR c++/28656
        * tree-vrp.c (nonnull_arg_p): Handle all nonnull attributes instead
        of just the first one.

        * c-common.c (check_function_nonnull): Handle multiple nonnull
        attributes properly.

        * c-c++-common/pr28656.c: New test.

--- gcc/tree-vrp.c.jj   2012-07-16 14:38:13.000000000 +0200
+++ gcc/tree-vrp.c      2012-07-19 14:24:27.277354132 +0200
@@ -353,32 +353,35 @@ nonnull_arg_p (const_tree arg)
     return true;
 
   fntype = TREE_TYPE (current_function_decl);
-  attrs = lookup_attribute ("nonnull", TYPE_ATTRIBUTES (fntype));
-
-  /* If "nonnull" wasn't specified, we know nothing about the argument.  */
-  if (attrs == NULL_TREE)
-    return false;
-
-  /* If "nonnull" applies to all the arguments, then ARG is non-null.  */
-  if (TREE_VALUE (attrs) == NULL_TREE)
-    return true;
-
-  /* Get the position number for ARG in the function signature.  */
-  for (arg_num = 1, t = DECL_ARGUMENTS (current_function_decl);
-       t;
-       t = DECL_CHAIN (t), arg_num++)
+  for (attrs = TYPE_ATTRIBUTES (fntype); attrs; attrs = TREE_CHAIN (attrs))
     {
-      if (t == arg)
-       break;
-    }
+      attrs = lookup_attribute ("nonnull", attrs);
 
-  gcc_assert (t == arg);
+      /* If "nonnull" wasn't specified, we know nothing about the argument.  */
+      if (attrs == NULL_TREE)
+       return false;
 
-  /* Now see if ARG_NUM is mentioned in the nonnull list.  */
-  for (t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t))
-    {
-      if (compare_tree_int (TREE_VALUE (t), arg_num) == 0)
+      /* If "nonnull" applies to all the arguments, then ARG is non-null.  */
+      if (TREE_VALUE (attrs) == NULL_TREE)
        return true;
+
+      /* Get the position number for ARG in the function signature.  */
+      for (arg_num = 1, t = DECL_ARGUMENTS (current_function_decl);
+          t;
+          t = DECL_CHAIN (t), arg_num++)
+       {
+         if (t == arg)
+           break;
+       }
+
+      gcc_assert (t == arg);
+
+      /* Now see if ARG_NUM is mentioned in the nonnull list.  */
+      for (t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t))
+       {
+         if (compare_tree_int (TREE_VALUE (t), arg_num) == 0)
+           return true;
+       }
     }
 
   return false;
--- gcc/c-family/c-common.c.jj  2012-07-18 12:02:11.000000000 +0200
+++ gcc/c-family/c-common.c     2012-07-19 14:32:05.915905501 +0200
@@ -8194,26 +8194,42 @@ handle_nonnull_attribute (tree *node, tr
 static void
 check_function_nonnull (tree attrs, int nargs, tree *argarray)
 {
-  tree a, args;
+  tree a;
   int i;
 
-  for (a = attrs; a; a = TREE_CHAIN (a))
+  attrs = lookup_attribute ("nonnull", attrs);
+  if (attrs == NULL_TREE)
+    return;
+
+  a = attrs;
+  /* See if any of the nonnull attributes has no arguments.  If so,
+     then every pointer argument is checked (in which case the check
+     for pointer type is done in check_nonnull_arg).  */
+  if (TREE_VALUE (a) != NULL_TREE)
+    do
+      a = lookup_attribute ("nonnull", TREE_CHAIN (a));
+    while (a != NULL_TREE && TREE_VALUE (a) != NULL_TREE);
+
+  if (a != NULL_TREE)
+    for (i = 0; i < nargs; i++)
+      check_function_arguments_recurse (check_nonnull_arg, NULL, argarray[i],
+                                       i + 1);
+  else
     {
-      if (is_attribute_p ("nonnull", TREE_PURPOSE (a)))
+      /* Walk the argument list.  If we encounter an argument number we
+        should check for non-null, do it.  */
+      for (i = 0; i < nargs; i++)
        {
-         args = TREE_VALUE (a);
-
-         /* Walk the argument list.  If we encounter an argument number we
-            should check for non-null, do it.  If the attribute has no args,
-            then every pointer argument is checked (in which case the check
-            for pointer type is done in check_nonnull_arg).  */
-         for (i = 0; i < nargs; i++)
+         for (a = attrs; ; a = TREE_CHAIN (a))
            {
-             if (!args || nonnull_check_p (args, i + 1))
-               check_function_arguments_recurse (check_nonnull_arg, NULL,
-                                                 argarray[i],
-                                                 i + 1);
+             a = lookup_attribute ("nonnull", a);
+             if (a == NULL_TREE || nonnull_check_p (TREE_VALUE (a), i + 1))
+               break;
            }
+
+         if (a != NULL_TREE)
+           check_function_arguments_recurse (check_nonnull_arg, NULL,
+                                             argarray[i], i + 1);
        }
     }
 }
--- gcc/testsuite/c-c++-common/pr28656.c.jj     2012-07-19 15:05:56.790975802 
+0200
+++ gcc/testsuite/c-c++-common/pr28656.c        2012-07-19 15:19:55.098486448 
+0200
@@ -0,0 +1,29 @@
+/* PR c++/28656 */
+/* { dg-do compile } */
+/* { dg-options "-Wnonnull" } */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+extern void *memcpy (void *__restrict, const void *__restrict, __SIZE_TYPE__)
+  __attribute__((nonnull (1), nonnull (2), nonnull (1, 2), nonnull));
+#ifdef __cplusplus
+}
+#endif
+
+extern void bar (void *p1, void *p2, void *p3, void *p4, void *p5)
+  __attribute__((nonnull (1), nonnull (1, 3), nonnull (3, 5), nonnull (4)));
+
+void
+foo (void)
+{
+  memcpy (0, 0, 0);
+  bar (0, 0, 0, 0, 0);
+}
+
+/* { dg-warning "null argument where non-null required\[^\n\r\]*argument 1" "" 
{ target *-*-* } 20 } */
+/* { dg-warning "null argument where non-null required\[^\n\r\]*argument 2" "" 
{ target *-*-* } 20 } */
+/* { dg-warning "null argument where non-null required\[^\n\r\]*argument 1" "" 
{ target *-*-* } 21 } */
+/* { dg-warning "null argument where non-null required\[^\n\r\]*argument 3" "" 
{ target *-*-* } 21 } */
+/* { dg-warning "null argument where non-null required\[^\n\r\]*argument 4" "" 
{ target *-*-* } 21 } */
+/* { dg-warning "null argument where non-null required\[^\n\r\]*argument 5" "" 
{ target *-*-* } 21 } */

        Jakub

Reply via email to