diff mbox series

[PING,2,C] Synthesize nonnull attribute for parameters declared with static

Message ID 564177ba373bb73cfc2c2b106b7e57b4bcbc512e.camel@tugraz.at
State New
Headers show
Series [PING,2,C] Synthesize nonnull attribute for parameters declared with static | expand

Commit Message

Martin Uecker Oct. 21, 2023, 11:09 a.m. UTC 
> 
> C programmers increasingly use static to indicate that
> pointer parameters are non-null.  Clang can exploit this
> for warnings and optimizations.  GCC has some warnings
> but not all warnings it has for nonnull.  Below is a
> patch to add a nonnull attribute automatically for such 
> arguments and to remove the special and more limited
> nonnull warnings for static. This patch found some 
> misplaced annotations in one of my projects via
> -Wnonnull-compare which clang does not seem to have, 
> so I think this could be useful.

    c: Synthesize nonnull attribute for parameters declared with static [PR110815]
    
    Parameters declared with `static` are nonnull. We synthesize
    an artifical nonnull attribute for such parameters to get the
    same warnings and optimizations.
    
    Bootstrapped and regression tested on x86.
    
	    PR c/102558
	    PR 102556
            PR c/110815
    
    gcc/c-family:
            * c-attribs.cc (build_attr_access_from_parms): Synthesize
            nonnull attribute for parameters declared with `static`.
    
    gcc:
            * gimple-ssa-warn-access.cc (pass_waccess::maybe_check_access_sizes):
            remove warning for parameters declared with `static`.
    
    gcc/testsuite:
            * gcc.dg/Wnonnull-8.c: Adapt test.
            * gcc.dg/Wnonnull-9.c: New test.

Comments

Jeff Law Nov. 10, 2023, 8:43 p.m. UTC | #1 
On 10/21/23 05:09, Martin Uecker wrote:
>>
>> C programmers increasingly use static to indicate that
>> pointer parameters are non-null.  Clang can exploit this
>> for warnings and optimizations.  GCC has some warnings
>> but not all warnings it has for nonnull.  Below is a
>> patch to add a nonnull attribute automatically for such
>> arguments and to remove the special and more limited
>> nonnull warnings for static. This patch found some
>> misplaced annotations in one of my projects via
>> -Wnonnull-compare which clang does not seem to have,
>> so I think this could be useful.
> 
>      c: Synthesize nonnull attribute for parameters declared with static [PR110815]
>      
>      Parameters declared with `static` are nonnull. We synthesize
>      an artifical nonnull attribute for such parameters to get the
>      same warnings and optimizations.
>      
>      Bootstrapped and regression tested on x86.
>      
> 	    PR c/102558
> 	    PR 102556
>              PR c/110815
>      
>      gcc/c-family:
>              * c-attribs.cc (build_attr_access_from_parms): Synthesize
>              nonnull attribute for parameters declared with `static`.
>      
>      gcc:
>              * gimple-ssa-warn-access.cc (pass_waccess::maybe_check_access_sizes):
>              remove warning for parameters declared with `static`.
>      
>      gcc/testsuite:
>              * gcc.dg/Wnonnull-8.c: Adapt test.
>              * gcc.dg/Wnonnull-9.c: New test.
This is OK -- assuming you did the usual bootstrap & regression test 
cycle.

Jeff
diff mbox series

Patch

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index e2792ca6898..ae7ffeb1f65 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -5280,6 +5280,7 @@  build_attr_access_from_parms (tree parms, bool skip_voidptr)
 	arg2pos.put (arg, argpos);
     }
 
+  tree nnlist = NULL_TREE;
   argpos = 0;
   for (tree arg = parms; arg; arg = TREE_CHAIN (arg), ++argpos)
     {
@@ -5313,6 +5314,11 @@  build_attr_access_from_parms (tree parms, bool skip_voidptr)
       tree str = TREE_VALUE (argspec);
       const char *s = TREE_STRING_POINTER (str);
 
+      /* Collect the list of nonnull arguments which use "[static ..]".  */
+      if (s != NULL && s[0] == '[' && s[1] == 's')
+	nnlist = tree_cons (NULL_TREE, build_int_cst (integer_type_node,
+						      argpos + 1), nnlist);
+
       /* Create the attribute access string from the arg spec string,
 	 optionally followed by position of the VLA bound argument if
 	 it is one.  */
@@ -5380,6 +5386,10 @@  build_attr_access_from_parms (tree parms, bool skip_voidptr)
   if (!spec.length ())
     return NULL_TREE;
 
+  /* If we have nonnull arguments, synthesize an attribute.  */
+  if (nnlist != NULL_TREE)
+    nnlist = build_tree_list (get_identifier ("nonnull"), nnlist);
+
   /* Attribute access takes a two or three arguments.  Wrap VBLIST in
      another list in case it has more nodes than would otherwise fit.  */
   vblist = build_tree_list (NULL_TREE, vblist);
@@ -5390,7 +5400,7 @@  build_attr_access_from_parms (tree parms, bool skip_voidptr)
   tree str = build_string (spec.length (), spec.c_str ());
   tree attrargs = tree_cons (NULL_TREE, str, vblist);
   tree name = get_identifier ("access");
-  return build_tree_list (name, attrargs);
+  return tree_cons (name, attrargs, nnlist);
 }
 
 /* Handle a "nothrow" attribute; arguments as in
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index b70d67dc47b..b8e89d01088 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -3491,16 +3491,6 @@  pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
 				   ptridx + 1, sizidx + 1, sizstr))
 		arg_warned = OPT_Wnonnull;
 	    }
-	  else if (access_size && access.second.static_p)
-	    {
-	      /* Warn about null pointers for [static N] array arguments
-		 but do not warn for ordinary (i.e., nonstatic) arrays.  */
-	      if (warning_at (loc, OPT_Wnonnull,
-			      "argument %i to %<%T[static %E]%> "
-			      "is null where non-null expected",
-			      ptridx + 1, argtype, access_nelts))
-		arg_warned = OPT_Wnonnull;
-	    }
 
 	  if (arg_warned != no_warning)
 	    {
diff --git a/gcc/testsuite/gcc.dg/Wnonnull-8.c b/gcc/testsuite/gcc.dg/Wnonnull-8.c
index 02871a76689..b24fd67cebc 100644
--- a/gcc/testsuite/gcc.dg/Wnonnull-8.c
+++ b/gcc/testsuite/gcc.dg/Wnonnull-8.c
@@ -10,5 +10,5 @@  foo (int a[static 7])
 int
 main ()
 {
-  foo ((int *) 0);	/* { dg-warning "argument 1 to 'int\\\[static 7\\\]' is null where non-null expected" } */
+  foo ((int *) 0);	/* { dg-warning "argument 1 null where non-null expected" } */
 }
diff --git a/gcc/testsuite/gcc.dg/Wnonnull-9.c b/gcc/testsuite/gcc.dg/Wnonnull-9.c
new file mode 100644
index 00000000000..1c57eefd2ae
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wnonnull-9.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+
+void
+foo (int a[static 1])
+{
+  if ((void*)0 == a)	/* { dg-warning "argument" "compared to NULL" } */
+    return;
+}
+
+int
+main ()
+{
+  foo ((void*)0);	/* { dg-warning "argument 1 null where non-null expected" } */
+}
+