[PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Jakub Jelinek]

Hi!

As has been reported today on gcc@, the new -Wnonnull warning addition
warns even about nonnull parameters that were changed (or could be changed)
in the function.  IMHO the nonnull attribute only talks about the value of
the parameter upon entry to the function, if you assign something else
to the argument afterwards and test that for NULL or non-NULL, it is like
any other variable.
But, we don't have the infrastructure to detect this in the FE, so this
patch moves the warning soon after we go into SSA form.
As -Wnonnull is a C/C++/ObjC/ObjC++ only warning, I've added a new
-Wnonnull-compare switch for it too (and enable it for C/C++/ObjC/ObjC++
from -Wall).

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

2016-02-16  Jakub Jelinek  <jakub@redhat.com>

	PR c/69835
	* common.opt (Wnonnull-compare): New warning.
	* doc/invoke.texi (-Wnonnull): Remove text about comparison
	of arguments against NULL.
	(-Wnonnull-compare): Document.
	* tree-ssa-uninit.c (warn_uninitialized_vars): Add warn_nonnull_cmp
	argument, if true, warn about SSA_NAME (D) of nonnull_arg_p
	comparisons against NULL pointers.
	(pass_late_warn_uninitialized::execute): Adjust caller.
	(execute_early_warn_uninitialized): Likewise.
	(gate_early_warn_uninitialized): New function.
	(pass_early_warn_uninitialized::gate): Call it instead of
	gate_warn_uninitialized.
c-family/
	* c.opt (Wnonnull-compare): Enable for -Wall.
c/
	* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
cp/
	* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
testsuite/
	* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
	-Wnonnull in dg-options.
	* c-c++-common/nonnull-2.c: New test.

--- gcc/common.opt.jj	2016-01-27 19:47:35.000000000 +0100
+++ gcc/common.opt	2016-02-16 11:52:42.641623690 +0100
@@ -616,6 +616,10 @@ Wlarger-than=
 Common RejectNegative Joined UInteger Warning
 -Wlarger-than=<number>	Warn if an object is larger than <number> bytes.
 
+Wnonnull-compare
+Var(warn_nonnull_compare) Warning
+Warn if comparing pointer parameter with nonnull attribute with NULL.
+
 Wnull-dereference
 Common Var(warn_null_dereference) Warning
 Warn if dereferencing a NULL pointer may lead to erroneous or undefined behavior.
--- gcc/doc/invoke.texi.jj	2016-02-08 18:39:16.000000000 +0100
+++ gcc/doc/invoke.texi	2016-02-16 12:31:42.037232875 +0100
@@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
 -Wmisleading-indentation -Wmissing-braces @gol
 -Wmissing-field-initializers -Wmissing-include-dirs @gol
--Wno-multichar  -Wnonnull  -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
+-Wno-multichar -Wnonnull -Wnonnull-compare @gol
+-Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
 -Wnull-dereference -Wodr  -Wno-overflow  -Wopenmp-simd  @gol
 -Woverride-init-side-effects -Woverlength-strings @gol
 -Wpacked  -Wpacked-bitfield-compat  -Wpadded @gol
@@ -3537,6 +3538,7 @@ Options} and @ref{Objective-C and Object
 -Wmissing-braces @r{(only for C/ObjC)} @gol
 -Wnarrowing @r{(only for C++)}  @gol
 -Wnonnull  @gol
+-Wnonnull-compare  @gol
 -Wopenmp-simd @gol
 -Wparentheses  @gol
 -Wpointer-sign  @gol
@@ -3795,12 +3797,18 @@ formats that may yield only a two-digit
 Warn about passing a null pointer for arguments marked as
 requiring a non-null value by the @code{nonnull} function attribute.
 
-Also warns when comparing an argument marked with the @code{nonnull}
-function attribute against null inside the function.
-
 @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
 can be disabled with the @option{-Wno-nonnull} option.
 
+@item -Wnonnull-compare
+@opindex Wnonnull-compare
+@opindex Wno-nonnull-compare
+Warn when comparing an argument marked with the @code{nonnull}
+function attribute against null inside the function.
+
+@option{-Wnonnull-compare} is included in @option{-Wall}.  It
+can be disabled with the @option{-Wno-nonnull-compare} option.
+
 @item -Wnull-dereference
 @opindex Wnull-dereference
 @opindex Wno-null-dereference
--- gcc/tree-ssa-uninit.c.jj	2016-01-13 13:28:41.000000000 +0100
+++ gcc/tree-ssa-uninit.c	2016-02-16 12:50:30.390619823 +0100
@@ -171,7 +171,8 @@ warn_uninit (enum opt_code wc, tree t, t
 }
 
 static unsigned int
-warn_uninitialized_vars (bool warn_possibly_uninitialized)
+warn_uninitialized_vars (bool warn_possibly_uninitialized,
+			 bool warn_nonnull_cmp)
 {
   gimple_stmt_iterator gsi;
   basic_block bb;
@@ -190,6 +191,60 @@ warn_uninitialized_vars (bool warn_possi
 	  if (is_gimple_debug (stmt))
 	    continue;
 
+	  if (warn_nonnull_cmp)
+	    {
+	      tree op0 = NULL_TREE, op1 = NULL_TREE;
+	      location_t loc = gimple_location (stmt);
+	      if (gimple_code (stmt) == GIMPLE_COND)
+		switch (gimple_cond_code (stmt))
+		  {
+		  case EQ_EXPR:
+		  case NE_EXPR:
+		    op0 = gimple_cond_lhs (stmt);
+		    op1 = gimple_cond_rhs (stmt);
+		    break;
+		  default:
+		    break;
+		  }
+	      else if (is_gimple_assign (stmt))
+		switch (gimple_assign_rhs_code (stmt))
+		  {
+		  case EQ_EXPR:
+		  case NE_EXPR:
+		    op0 = gimple_assign_rhs1 (stmt);
+		    op1 = gimple_assign_rhs2 (stmt);
+		    break;
+		  case COND_EXPR:
+		    switch (TREE_CODE (gimple_assign_rhs1 (stmt)))
+		      {
+		      case EQ_EXPR:
+		      case NE_EXPR:
+			op1 = gimple_assign_rhs1 (stmt);
+			loc = EXPR_LOC_OR_LOC (op1, loc);
+			op0 = TREE_OPERAND (op1, 0);
+			op1 = TREE_OPERAND (op1, 1);
+			break;
+		      default:
+			break;
+		      }
+		    break;
+		  default:
+		    break;
+		  }
+	      if (op0
+		  && TREE_CODE (op0) == SSA_NAME
+		  && SSA_NAME_IS_DEFAULT_DEF (op0)
+		  && TREE_CODE (SSA_NAME_VAR (op0)) == PARM_DECL
+		  && (POINTER_TYPE_P (TREE_TYPE (op0))
+		      || TREE_CODE (TREE_TYPE (op0)) == OFFSET_TYPE)
+		  && nonnull_arg_p (SSA_NAME_VAR (op0))
+		  && (POINTER_TYPE_P (TREE_TYPE (op0))
+		      ? integer_zerop (op1) : integer_minus_onep (op1)))
+		warning_at (gimple_location (stmt), OPT_Wnonnull_compare,
+			    "nonnull argument %qD compared to NULL",
+			    SSA_NAME_VAR (op0));
+	    }
+
 	  /* We only do data flow with SSA_NAMEs, so that's all we
 	     can warn about.  */
 	  FOR_EACH_SSA_USE_OPERAND (use_p, stmt, op_iter, SSA_OP_USE)
@@ -2416,7 +2471,7 @@ pass_late_warn_uninitialized::execute (f
   /* Re-do the plain uninitialized variable check, as optimization may have
      straightened control flow.  Do this first so that we don't accidentally
      get a "may be" warning when we'd have seen an "is" warning later.  */
-  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/1);
+  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/true, false);
 
   timevar_push (TV_TREE_UNINIT);
 
@@ -2478,6 +2533,14 @@ make_pass_late_warn_uninitialized (gcc::
 }
 
 
+static bool
+gate_early_warn_uninitialized (void)
+{
+  return (warn_uninitialized
+	  || warn_maybe_uninitialized
+	  || warn_nonnull_compare);
+}
+
 static unsigned int
 execute_early_warn_uninitialized (void)
 {
@@ -2488,7 +2551,8 @@ execute_early_warn_uninitialized (void)
      optimization we need to warn here about "may be uninitialized".  */
   calculate_dominance_info (CDI_POST_DOMINATORS);
 
-  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/!optimize);
+  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/!optimize,
+			   warn_nonnull_compare);
 
   /* Post-dominator information can not be reliably updated. Free it
      after the use.  */
@@ -2521,7 +2585,7 @@ public:
   {}
 
   /* opt_pass methods: */
-  virtual bool gate (function *) { return gate_warn_uninitialized (); }
+  virtual bool gate (function *) { return gate_early_warn_uninitialized (); }
   virtual unsigned int execute (function *)
     {
       return execute_early_warn_uninitialized ();
--- gcc/c-family/c.opt.jj	2016-02-08 18:39:17.000000000 +0100
+++ gcc/c-family/c.opt	2016-02-16 12:30:00.299641823 +0100
@@ -677,6 +677,10 @@ Wnonnull
 C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
 ;
 
+Wnonnull-compare
+C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
+;
+
 Wnormalized
 C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
 ;
--- gcc/c/c-typeck.c.jj	2016-02-12 10:23:50.000000000 +0100
+++ gcc/c/c-typeck.c	2016-02-16 11:40:08.474048701 +0100
@@ -11086,11 +11086,6 @@ build_binary_op (location_t location, en
 	short_compare = 1;
       else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op0) == PARM_DECL && nonnull_arg_p (op0))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op0);
-
 	  if (TREE_CODE (op0) == ADDR_EXPR
 	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
 	    {
@@ -11111,11 +11106,6 @@ build_binary_op (location_t location, en
 	}
       else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op1) == PARM_DECL && nonnull_arg_p (op1))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op1);
-
 	  if (TREE_CODE (op1) == ADDR_EXPR
 	      && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
 	    {
--- gcc/cp/typeck.c.jj	2016-02-12 10:23:50.000000000 +0100
+++ gcc/cp/typeck.c	2016-02-16 11:40:37.783643278 +0100
@@ -4514,11 +4514,6 @@ cp_build_binary_op (location_t location,
 	       || (code0 == POINTER_TYPE
 		   && TYPE_PTR_P (type1) && integer_zerop (op1)))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op0) == PARM_DECL && nonnull_arg_p (op0))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op0);
-
 	  if (TYPE_PTR_P (type1))
 	    result_type = composite_pointer_type (type0, type1, op0, op1,
 						  CPO_COMPARISON, complain);
@@ -4558,11 +4553,6 @@ cp_build_binary_op (location_t location,
 	       || (code1 == POINTER_TYPE
 		   && TYPE_PTR_P (type0) && integer_zerop (op0)))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op1) == PARM_DECL && nonnull_arg_p (op1))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op1);
-
 	  if (TYPE_PTR_P (type0))
 	    result_type = composite_pointer_type (type0, type1, op0, op1,
 						  CPO_COMPARISON, complain);
--- gcc/testsuite/c-c++-common/nonnull-1.c.jj	2015-10-11 19:11:06.000000000 +0200
+++ gcc/testsuite/c-c++-common/nonnull-1.c	2016-02-16 12:38:53.917256278 +0100
@@ -1,7 +1,7 @@
 /* Test for the bad usage of "nonnull" function attribute parms.  */
 /*  */
 /* { dg-do compile } */
-/* { dg-options "-Wnonnull" } */
+/* { dg-options "-Wnonnull-compare" } */
 
 #include <stddef.h>
 #include <stdlib.h>
--- gcc/testsuite/c-c++-common/nonnull-2.c.jj	2016-02-16 12:52:17.149142596 +0100
+++ gcc/testsuite/c-c++-common/nonnull-2.c	2016-02-16 12:56:29.904645198 +0100
@@ -0,0 +1,26 @@
+/* Test for the bad usage of "nonnull" function attribute parms.  */
+/* { dg-do compile } */
+/* { dg-options "-Wnonnull-compare" } */
+
+void bar (char **);
+
+__attribute__((nonnull (1, 3))) int
+foo (char *cp1, char *cp2, char *cp3, char *cp4)
+{
+  if (cp1 == (char *) 0) /* { dg-warning "nonnull argument" "cp1 compared to NULL" } */
+    return 1;
+
+  cp1 = cp2;
+  if (cp1 == (char *) 0) /* { dg-bogus "nonnull argument" } */
+    return 2;
+
+  if (!cp4)	   /* { dg-bogus "nonnull argument" } */
+    return 3;
+
+  char **p = &cp3;
+  bar (p);
+  if (cp3 == (char *) 0) /* { dg-bogus "nonnull argument" } */
+    return 4;
+
+  return 5;
+}

	Jakub

Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Richard Biener]

On Tue, 16 Feb 2016, Jakub Jelinek wrote:

> Hi!
> 
> As has been reported today on gcc@, the new -Wnonnull warning addition
> warns even about nonnull parameters that were changed (or could be changed)
> in the function.  IMHO the nonnull attribute only talks about the value of
> the parameter upon entry to the function, if you assign something else
> to the argument afterwards and test that for NULL or non-NULL, it is like
> any other variable.
> But, we don't have the infrastructure to detect this in the FE, so this
> patch moves the warning soon after we go into SSA form.
> As -Wnonnull is a C/C++/ObjC/ObjC++ only warning, I've added a new
> -Wnonnull-compare switch for it too (and enable it for C/C++/ObjC/ObjC++
> from -Wall).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Not sure if it matters but wouldn't walking over function parameters
and their default def SSA names immediate uses be cheaper than
matching all conditions?  Esp. as nonnull_arg_p walks over all
DECL_ARGUMENTS (up to the searched index) over and over again?
[we should simply set a flag on the PARM_DECL!]

Richard.

> 2016-02-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/69835
> 	* common.opt (Wnonnull-compare): New warning.
> 	* doc/invoke.texi (-Wnonnull): Remove text about comparison
> 	of arguments against NULL.
> 	(-Wnonnull-compare): Document.
> 	* tree-ssa-uninit.c (warn_uninitialized_vars): Add warn_nonnull_cmp
> 	argument, if true, warn about SSA_NAME (D) of nonnull_arg_p
> 	comparisons against NULL pointers.
> 	(pass_late_warn_uninitialized::execute): Adjust caller.
> 	(execute_early_warn_uninitialized): Likewise.
> 	(gate_early_warn_uninitialized): New function.
> 	(pass_early_warn_uninitialized::gate): Call it instead of
> 	gate_warn_uninitialized.
> c-family/
> 	* c.opt (Wnonnull-compare): Enable for -Wall.
> c/
> 	* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
> cp/
> 	* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
> testsuite/
> 	* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
> 	-Wnonnull in dg-options.
> 	* c-c++-common/nonnull-2.c: New test.
> 
> --- gcc/common.opt.jj	2016-01-27 19:47:35.000000000 +0100
> +++ gcc/common.opt	2016-02-16 11:52:42.641623690 +0100
> @@ -616,6 +616,10 @@ Wlarger-than=
>  Common RejectNegative Joined UInteger Warning
>  -Wlarger-than=<number>	Warn if an object is larger than <number> bytes.
>  
> +Wnonnull-compare
> +Var(warn_nonnull_compare) Warning
> +Warn if comparing pointer parameter with nonnull attribute with NULL.
> +
>  Wnull-dereference
>  Common Var(warn_null_dereference) Warning
>  Warn if dereferencing a NULL pointer may lead to erroneous or undefined behavior.
> --- gcc/doc/invoke.texi.jj	2016-02-08 18:39:16.000000000 +0100
> +++ gcc/doc/invoke.texi	2016-02-16 12:31:42.037232875 +0100
> @@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
>  -Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
>  -Wmisleading-indentation -Wmissing-braces @gol
>  -Wmissing-field-initializers -Wmissing-include-dirs @gol
> --Wno-multichar  -Wnonnull  -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
> +-Wno-multichar -Wnonnull -Wnonnull-compare @gol
> +-Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
>  -Wnull-dereference -Wodr  -Wno-overflow  -Wopenmp-simd  @gol
>  -Woverride-init-side-effects -Woverlength-strings @gol
>  -Wpacked  -Wpacked-bitfield-compat  -Wpadded @gol
> @@ -3537,6 +3538,7 @@ Options} and @ref{Objective-C and Object
>  -Wmissing-braces @r{(only for C/ObjC)} @gol
>  -Wnarrowing @r{(only for C++)}  @gol
>  -Wnonnull  @gol
> +-Wnonnull-compare  @gol
>  -Wopenmp-simd @gol
>  -Wparentheses  @gol
>  -Wpointer-sign  @gol
> @@ -3795,12 +3797,18 @@ formats that may yield only a two-digit
>  Warn about passing a null pointer for arguments marked as
>  requiring a non-null value by the @code{nonnull} function attribute.
>  
> -Also warns when comparing an argument marked with the @code{nonnull}
> -function attribute against null inside the function.
> -
>  @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
>  can be disabled with the @option{-Wno-nonnull} option.
>  
> +@item -Wnonnull-compare
> +@opindex Wnonnull-compare
> +@opindex Wno-nonnull-compare
> +Warn when comparing an argument marked with the @code{nonnull}
> +function attribute against null inside the function.
> +
> +@option{-Wnonnull-compare} is included in @option{-Wall}.  It
> +can be disabled with the @option{-Wno-nonnull-compare} option.
> +
>  @item -Wnull-dereference
>  @opindex Wnull-dereference
>  @opindex Wno-null-dereference
> --- gcc/tree-ssa-uninit.c.jj	2016-01-13 13:28:41.000000000 +0100
> +++ gcc/tree-ssa-uninit.c	2016-02-16 12:50:30.390619823 +0100
> @@ -171,7 +171,8 @@ warn_uninit (enum opt_code wc, tree t, t
>  }
>  
>  static unsigned int
> -warn_uninitialized_vars (bool warn_possibly_uninitialized)
> +warn_uninitialized_vars (bool warn_possibly_uninitialized,
> +			 bool warn_nonnull_cmp)
>  {
>    gimple_stmt_iterator gsi;
>    basic_block bb;
> @@ -190,6 +191,60 @@ warn_uninitialized_vars (bool warn_possi
>  	  if (is_gimple_debug (stmt))
>  	    continue;
>  
> +	  if (warn_nonnull_cmp)
> +	    {
> +	      tree op0 = NULL_TREE, op1 = NULL_TREE;
> +	      location_t loc = gimple_location (stmt);
> +	      if (gimple_code (stmt) == GIMPLE_COND)

For new if (gimple_code () ...) code I'd prefer dyn_cast and gcond *
and gassign *, that should be marginally faster, esp. with checking.

> +		switch (gimple_cond_code (stmt))
> +		  {
> +		  case EQ_EXPR:
> +		  case NE_EXPR:
> +		    op0 = gimple_cond_lhs (stmt);
> +		    op1 = gimple_cond_rhs (stmt);
> +		    break;
> +		  default:
> +		    break;
> +		  }
> +	      else if (is_gimple_assign (stmt))
> +		switch (gimple_assign_rhs_code (stmt))
> +		  {
> +		  case EQ_EXPR:
> +		  case NE_EXPR:
> +		    op0 = gimple_assign_rhs1 (stmt);
> +		    op1 = gimple_assign_rhs2 (stmt);
> +		    break;
> +		  case COND_EXPR:
> +		    switch (TREE_CODE (gimple_assign_rhs1 (stmt)))
> +		      {
> +		      case EQ_EXPR:
> +		      case NE_EXPR:
> +			op1 = gimple_assign_rhs1 (stmt);
> +			loc = EXPR_LOC_OR_LOC (op1, loc);
> +			op0 = TREE_OPERAND (op1, 0);
> +			op1 = TREE_OPERAND (op1, 1);
> +			break;
> +		      default:
> +			break;
> +		      }
> +		    break;
> +		  default:
> +		    break;
> +		  }
> +	      if (op0
> +		  && TREE_CODE (op0) == SSA_NAME
> +		  && SSA_NAME_IS_DEFAULT_DEF (op0)
> +		  && TREE_CODE (SSA_NAME_VAR (op0)) == PARM_DECL
> +		  && (POINTER_TYPE_P (TREE_TYPE (op0))
> +		      || TREE_CODE (TREE_TYPE (op0)) == OFFSET_TYPE)
> +		  && nonnull_arg_p (SSA_NAME_VAR (op0))
> +		  && (POINTER_TYPE_P (TREE_TYPE (op0))
> +		      ? integer_zerop (op1) : integer_minus_onep (op1)))

Why do we have OFFSET_TYPE and why do we say "nonnull" for -1 offset?

Other than the walking issue the patch looks reasonable.

Thanks,
Richard.

> +		warning_at (gimple_location (stmt), OPT_Wnonnull_compare,
> +			    "nonnull argument %qD compared to NULL",
> +			    SSA_NAME_VAR (op0));
> +	    }
> +
>  	  /* We only do data flow with SSA_NAMEs, so that's all we
>  	     can warn about.  */
>  	  FOR_EACH_SSA_USE_OPERAND (use_p, stmt, op_iter, SSA_OP_USE)
> @@ -2416,7 +2471,7 @@ pass_late_warn_uninitialized::execute (f
>    /* Re-do the plain uninitialized variable check, as optimization may have
>       straightened control flow.  Do this first so that we don't accidentally
>       get a "may be" warning when we'd have seen an "is" warning later.  */
> -  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/1);
> +  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/true, false);
>  
>    timevar_push (TV_TREE_UNINIT);
>  
> @@ -2478,6 +2533,14 @@ make_pass_late_warn_uninitialized (gcc::
>  }
>  
>  
> +static bool
> +gate_early_warn_uninitialized (void)
> +{
> +  return (warn_uninitialized
> +	  || warn_maybe_uninitialized
> +	  || warn_nonnull_compare);
> +}
> +
>  static unsigned int
>  execute_early_warn_uninitialized (void)
>  {
> @@ -2488,7 +2551,8 @@ execute_early_warn_uninitialized (void)
>       optimization we need to warn here about "may be uninitialized".  */
>    calculate_dominance_info (CDI_POST_DOMINATORS);
>  
> -  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/!optimize);
> +  warn_uninitialized_vars (/*warn_possibly_uninitialized=*/!optimize,
> +			   warn_nonnull_compare);
>  
>    /* Post-dominator information can not be reliably updated. Free it
>       after the use.  */
> @@ -2521,7 +2585,7 @@ public:
>    {}
>  
>    /* opt_pass methods: */
> -  virtual bool gate (function *) { return gate_warn_uninitialized (); }
> +  virtual bool gate (function *) { return gate_early_warn_uninitialized (); }
>    virtual unsigned int execute (function *)
>      {
>        return execute_early_warn_uninitialized ();
> --- gcc/c-family/c.opt.jj	2016-02-08 18:39:17.000000000 +0100
> +++ gcc/c-family/c.opt	2016-02-16 12:30:00.299641823 +0100
> @@ -677,6 +677,10 @@ Wnonnull
>  C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
>  ;
>  
> +Wnonnull-compare
> +C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
> +;
> +
>  Wnormalized
>  C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
>  ;
> --- gcc/c/c-typeck.c.jj	2016-02-12 10:23:50.000000000 +0100
> +++ gcc/c/c-typeck.c	2016-02-16 11:40:08.474048701 +0100
> @@ -11086,11 +11086,6 @@ build_binary_op (location_t location, en
>  	short_compare = 1;
>        else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
>  	{
> -	  if (warn_nonnull
> -	      && TREE_CODE (op0) == PARM_DECL && nonnull_arg_p (op0))
> -	    warning_at (location, OPT_Wnonnull,
> -			"nonnull argument %qD compared to NULL", op0);
> -
>  	  if (TREE_CODE (op0) == ADDR_EXPR
>  	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
>  	    {
> @@ -11111,11 +11106,6 @@ build_binary_op (location_t location, en
>  	}
>        else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
>  	{
> -	  if (warn_nonnull
> -	      && TREE_CODE (op1) == PARM_DECL && nonnull_arg_p (op1))
> -	    warning_at (location, OPT_Wnonnull,
> -			"nonnull argument %qD compared to NULL", op1);
> -
>  	  if (TREE_CODE (op1) == ADDR_EXPR
>  	      && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
>  	    {
> --- gcc/cp/typeck.c.jj	2016-02-12 10:23:50.000000000 +0100
> +++ gcc/cp/typeck.c	2016-02-16 11:40:37.783643278 +0100
> @@ -4514,11 +4514,6 @@ cp_build_binary_op (location_t location,
>  	       || (code0 == POINTER_TYPE
>  		   && TYPE_PTR_P (type1) && integer_zerop (op1)))
>  	{
> -	  if (warn_nonnull
> -	      && TREE_CODE (op0) == PARM_DECL && nonnull_arg_p (op0))
> -	    warning_at (location, OPT_Wnonnull,
> -			"nonnull argument %qD compared to NULL", op0);
> -
>  	  if (TYPE_PTR_P (type1))
>  	    result_type = composite_pointer_type (type0, type1, op0, op1,
>  						  CPO_COMPARISON, complain);
> @@ -4558,11 +4553,6 @@ cp_build_binary_op (location_t location,
>  	       || (code1 == POINTER_TYPE
>  		   && TYPE_PTR_P (type0) && integer_zerop (op0)))
>  	{
> -	  if (warn_nonnull
> -	      && TREE_CODE (op1) == PARM_DECL && nonnull_arg_p (op1))
> -	    warning_at (location, OPT_Wnonnull,
> -			"nonnull argument %qD compared to NULL", op1);
> -
>  	  if (TYPE_PTR_P (type0))
>  	    result_type = composite_pointer_type (type0, type1, op0, op1,
>  						  CPO_COMPARISON, complain);
> --- gcc/testsuite/c-c++-common/nonnull-1.c.jj	2015-10-11 19:11:06.000000000 +0200
> +++ gcc/testsuite/c-c++-common/nonnull-1.c	2016-02-16 12:38:53.917256278 +0100
> @@ -1,7 +1,7 @@
>  /* Test for the bad usage of "nonnull" function attribute parms.  */
>  /*  */
>  /* { dg-do compile } */
> -/* { dg-options "-Wnonnull" } */
> +/* { dg-options "-Wnonnull-compare" } */
>  
>  #include <stddef.h>
>  #include <stdlib.h>
> --- gcc/testsuite/c-c++-common/nonnull-2.c.jj	2016-02-16 12:52:17.149142596 +0100
> +++ gcc/testsuite/c-c++-common/nonnull-2.c	2016-02-16 12:56:29.904645198 +0100
> @@ -0,0 +1,26 @@
> +/* Test for the bad usage of "nonnull" function attribute parms.  */
> +/* { dg-do compile } */
> +/* { dg-options "-Wnonnull-compare" } */
> +
> +void bar (char **);
> +
> +__attribute__((nonnull (1, 3))) int
> +foo (char *cp1, char *cp2, char *cp3, char *cp4)
> +{
> +  if (cp1 == (char *) 0) /* { dg-warning "nonnull argument" "cp1 compared to NULL" } */
> +    return 1;
> +
> +  cp1 = cp2;
> +  if (cp1 == (char *) 0) /* { dg-bogus "nonnull argument" } */
> +    return 2;
> +
> +  if (!cp4)	   /* { dg-bogus "nonnull argument" } */
> +    return 3;
> +
> +  char **p = &cp3;
> +  bar (p);
> +  if (cp3 == (char *) 0) /* { dg-bogus "nonnull argument" } */
> +    return 4;
> +
> +  return 5;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Jeff Law]

On 02/16/2016 08:36 AM, Richard Biener wrote:
> On Tue, 16 Feb 2016, Jakub Jelinek wrote:
>
>> Hi!
>>
>> As has been reported today on gcc@, the new -Wnonnull warning addition
>> warns even about nonnull parameters that were changed (or could be changed)
>> in the function.  IMHO the nonnull attribute only talks about the value of
>> the parameter upon entry to the function, if you assign something else
>> to the argument afterwards and test that for NULL or non-NULL, it is like
>> any other variable.
>> But, we don't have the infrastructure to detect this in the FE, so this
>> patch moves the warning soon after we go into SSA form.
>> As -Wnonnull is a C/C++/ObjC/ObjC++ only warning, I've added a new
>> -Wnonnull-compare switch for it too (and enable it for C/C++/ObjC/ObjC++
>> from -Wall).
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Not sure if it matters but wouldn't walking over function parameters
> and their default def SSA names immediate uses be cheaper than
> matching all conditions?  Esp. as nonnull_arg_p walks over all
> DECL_ARGUMENTS (up to the searched index) over and over again?
> [we should simply set a flag on the PARM_DECL!]
Seems like a waste to burn a flag bit for something like this.

We're already walking all the statements in this code so it's really 
just the cost of testing if the existing statement is "interesting" for 
this warning.  I guess you're suggesting jakub walk the arguments and if 
one is a marked as non-null, then walk the immediate uses.  Yea, that's 
probably faster than doing the test for every statement.

Swapping the nonnull_arg_p and the test for whether or not op1 is 0 
would be a slight help as well, particularly if there was machine 
generated code with oodles of arguments.

I'm not sure why we're testing OFFSET_TYPE against -1.

warn_uninitialized_vars seems like a poor name if we're warning for 
non-null too.  So I'd suggest tweaking its name appropriately as well.


Jeff

Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Jakub Jelinek]

On Tue, Feb 16, 2016 at 11:12:27AM -0700, Jeff Law wrote:
> >Not sure if it matters but wouldn't walking over function parameters
> >and their default def SSA names immediate uses be cheaper than
> >matching all conditions?  Esp. as nonnull_arg_p walks over all
> >DECL_ARGUMENTS (up to the searched index) over and over again?
> >[we should simply set a flag on the PARM_DECL!]
> Seems like a waste to burn a flag bit for something like this.
> 
> We're already walking all the statements in this code so it's really just
> the cost of testing if the existing statement is "interesting" for this
> warning.  I guess you're suggesting jakub walk the arguments and if one is a
> marked as non-null, then walk the immediate uses.  Yea, that's probably
> faster than doing the test for every statement.

I'm already bootstrapping/regtesting following variant.

> Swapping the nonnull_arg_p and the test for whether or not op1 is 0 would be
> a slight help as well, particularly if there was machine generated code with
> oodles of arguments.
> 
> I'm not sure why we're testing OFFSET_TYPE against -1.

That is because NULL pointers to member are represented as -1 in the middle
end.

2016-02-16  Jakub Jelinek  <jakub@redhat.com>

	PR c/69835
	* common.opt (Wnonnull-compare): New warning.
	* doc/invoke.texi (-Wnonnull): Remove text about comparison
	of arguments against NULL.
	(-Wnonnull-compare): Document.
	* Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o.
	* tree-pass.h (make_pass_warn_nonnull_compare): Declare.
	* passes.def (pass_warn_nonnull_compare): Add.
	* gimple-ssa-nonnull-compare.c: New file.
c-family/
	* c.opt (Wnonnull-compare): Enable for -Wall.
c/
	* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
cp/
	* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
testsuite/
	* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
	-Wnonnull in dg-options.
	* c-c++-common/nonnull-2.c: New test.

--- gcc/common.opt.jj	2016-02-16 16:14:40.787071893 +0100
+++ gcc/common.opt	2016-02-16 17:34:12.850648209 +0100
@@ -616,6 +616,10 @@ Wlarger-than=
 Common RejectNegative Joined UInteger Warning
 -Wlarger-than=<number>	Warn if an object is larger than <number> bytes.
 
+Wnonnull-compare
+Var(warn_nonnull_compare) Warning
+Warn if comparing pointer parameter with nonnull attribute with NULL.
+
 Wnull-dereference
 Common Var(warn_null_dereference) Warning
 Warn if dereferencing a NULL pointer may lead to erroneous or undefined behavior.
--- gcc/doc/invoke.texi.jj	2016-02-16 16:14:41.692059415 +0100
+++ gcc/doc/invoke.texi	2016-02-16 17:26:55.346587296 +0100
@@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
 -Wmisleading-indentation -Wmissing-braces @gol
 -Wmissing-field-initializers -Wmissing-include-dirs @gol
--Wno-multichar  -Wnonnull  -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
+-Wno-multichar -Wnonnull -Wnonnull-compare @gol
+-Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
 -Wnull-dereference -Wodr  -Wno-overflow  -Wopenmp-simd  @gol
 -Woverride-init-side-effects -Woverlength-strings @gol
 -Wpacked  -Wpacked-bitfield-compat  -Wpadded @gol
@@ -3537,6 +3538,7 @@ Options} and @ref{Objective-C and Object
 -Wmissing-braces @r{(only for C/ObjC)} @gol
 -Wnarrowing @r{(only for C++)}  @gol
 -Wnonnull  @gol
+-Wnonnull-compare  @gol
 -Wopenmp-simd @gol
 -Wparentheses  @gol
 -Wpointer-sign  @gol
@@ -3795,12 +3797,18 @@ formats that may yield only a two-digit
 Warn about passing a null pointer for arguments marked as
 requiring a non-null value by the @code{nonnull} function attribute.
 
-Also warns when comparing an argument marked with the @code{nonnull}
-function attribute against null inside the function.
-
 @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
 can be disabled with the @option{-Wno-nonnull} option.
 
+@item -Wnonnull-compare
+@opindex Wnonnull-compare
+@opindex Wno-nonnull-compare
+Warn when comparing an argument marked with the @code{nonnull}
+function attribute against null inside the function.
+
+@option{-Wnonnull-compare} is included in @option{-Wall}.  It
+can be disabled with the @option{-Wno-nonnull-compare} option.
+
 @item -Wnull-dereference
 @opindex Wnull-dereference
 @opindex Wno-null-dereference
--- gcc/Makefile.in.jj	2016-02-11 20:28:54.000000000 +0100
+++ gcc/Makefile.in	2016-02-16 17:32:26.919086223 +0100
@@ -1278,6 +1278,7 @@ OBJS = \
 	gimple-pretty-print.o \
 	gimple-ssa-backprop.o \
 	gimple-ssa-isolate-paths.o \
+	gimple-ssa-nonnull-compare.o \
 	gimple-ssa-split-paths.o \
 	gimple-ssa-strength-reduction.o \
 	gimple-streamer-in.o \
--- gcc/tree-pass.h.jj	2016-01-19 13:31:07.000000000 +0100
+++ gcc/tree-pass.h	2016-02-16 17:28:39.155178102 +0100
@@ -472,6 +472,7 @@ extern gimple_opt_pass *make_pass_oacc_k
 extern simple_ipa_opt_pass *make_pass_ipa_oacc (gcc::context *ctxt);
 extern simple_ipa_opt_pass *make_pass_ipa_oacc_kernels (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_gen_hsail (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_warn_nonnull_compare (gcc::context *ctxt);
 
 /* IPA Passes */
 extern simple_ipa_opt_pass *make_pass_ipa_lower_emutls (gcc::context *ctxt);
--- gcc/passes.def.jj	2016-02-10 16:01:57.000000000 +0100
+++ gcc/passes.def	2016-02-16 17:27:46.515892676 +0100
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.
       NEXT_PASS (pass_fixup_cfg);
       NEXT_PASS (pass_init_datastructures);
       NEXT_PASS (pass_build_ssa);
+      NEXT_PASS (pass_warn_nonnull_compare);
       NEXT_PASS (pass_ubsan);
       NEXT_PASS (pass_early_warn_uninitialized);
       NEXT_PASS (pass_nothrow);
--- gcc/gimple-ssa-nonnull-compare.c.jj	2016-02-16 16:58:10.676272364 +0100
+++ gcc/gimple-ssa-nonnull-compare.c	2016-02-16 17:46:19.788692474 +0100
@@ -0,0 +1,150 @@
+/* -Wnonnull-compare warning support.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "tree-pass.h"
+#include "ssa.h"
+#include "diagnostic-core.h"
+#include "tree-dfa.h"
+
+/* Warn about comparison of nonnull_arg_p argument initial values
+   with NULL.  */
+
+static void
+do_warn_nonnull_compare (function *fun, tree arg)
+{
+  if (!POINTER_TYPE_P (TREE_TYPE (arg))
+      && TREE_CODE (TREE_TYPE (arg)) != OFFSET_TYPE)
+    return;
+
+  if (!nonnull_arg_p (arg))
+    return;
+
+  tree d = ssa_default_def (fun, arg);
+  if (d == NULL_TREE)
+    return;
+
+  use_operand_p use_p;
+  imm_use_iterator iter;
+
+  FOR_EACH_IMM_USE_FAST (use_p, iter, d)
+    {
+      gimple *stmt = USE_STMT (use_p);
+      tree op = NULL_TREE;
+      location_t loc = gimple_location (stmt);
+      if (gimple_code (stmt) == GIMPLE_COND)
+	switch (gimple_cond_code (stmt))
+	  {
+	  case EQ_EXPR:
+	  case NE_EXPR:
+	    if (gimple_cond_lhs (stmt) == d)
+	      op = gimple_cond_rhs (stmt);
+	    break;
+	  default:
+	    break;
+	  }
+      else if (is_gimple_assign (stmt))
+	switch (gimple_assign_rhs_code (stmt))
+	  {
+	  case EQ_EXPR:
+	  case NE_EXPR:
+	    if (gimple_assign_rhs1 (stmt) == d)
+	      op = gimple_assign_rhs2 (stmt);
+	    break;
+	  case COND_EXPR:
+	    switch (TREE_CODE (gimple_assign_rhs1 (stmt)))
+	      {
+	      case EQ_EXPR:
+	      case NE_EXPR:
+		op = gimple_assign_rhs1 (stmt);
+		if (TREE_OPERAND (op, 0) != d)
+		  {
+		    op = NULL_TREE;
+		    break;
+		  }
+		loc = EXPR_LOC_OR_LOC (op, loc);
+		op = TREE_OPERAND (op, 1);
+		break;
+	      default:
+		break;
+	      }
+	    break;
+	  default:
+	    break;
+	  }
+      if (op
+	  && (POINTER_TYPE_P (TREE_TYPE (arg))
+	      ? integer_zerop (op) : integer_minus_onep (op)))
+	warning_at (loc, OPT_Wnonnull_compare,
+		    "nonnull argument %qD compared to NULL", arg);
+    }
+}
+
+namespace {
+
+const pass_data pass_data_warn_nonnull_compare =
+{
+  GIMPLE_PASS, /* type */
+  "*nonnullcmp", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_NONE, /* tv_id */
+  PROP_ssa, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0, /* todo_flags_finish */
+};
+
+class pass_warn_nonnull_compare : public gimple_opt_pass
+{
+public:
+  pass_warn_nonnull_compare (gcc::context *ctxt)
+    : gimple_opt_pass (pass_data_warn_nonnull_compare, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  virtual bool gate (function *) { return warn_nonnull_compare; }
+
+  virtual unsigned int execute (function *);
+
+}; // class pass_warn_nonnull_compare
+
+unsigned int
+pass_warn_nonnull_compare::execute (function *fun)
+{
+  if (fun->static_chain_decl)
+    do_warn_nonnull_compare (fun, fun->static_chain_decl);
+
+  for (tree arg = DECL_ARGUMENTS (cfun->decl); arg; arg = DECL_CHAIN (arg))
+    do_warn_nonnull_compare (fun, arg);
+  return 0;
+}
+
+} // anon namespace
+
+gimple_opt_pass *
+make_pass_warn_nonnull_compare (gcc::context *ctxt)
+{
+  return new pass_warn_nonnull_compare (ctxt);
+}
--- gcc/c-family/c.opt.jj	2016-02-16 16:14:40.895070404 +0100
+++ gcc/c-family/c.opt	2016-02-16 17:26:55.347587282 +0100
@@ -677,6 +677,10 @@ Wnonnull
 C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
 ;
 
+Wnonnull-compare
+C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
+;
+
 Wnormalized
 C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
 ;
--- gcc/c/c-typeck.c.jj	2016-02-16 16:14:40.636073975 +0100
+++ gcc/c/c-typeck.c	2016-02-16 17:26:55.350587242 +0100
@@ -11086,11 +11086,6 @@ build_binary_op (location_t location, en
 	short_compare = 1;
       else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op0) == PARM_DECL && nonnull_arg_p (op0))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op0);
-
 	  if (TREE_CODE (op0) == ADDR_EXPR
 	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
 	    {
@@ -11111,11 +11106,6 @@ build_binary_op (location_t location, en
 	}
       else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op1) == PARM_DECL && nonnull_arg_p (op1))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op1);
-
 	  if (TREE_CODE (op1) == ADDR_EXPR
 	      && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
 	    {
--- gcc/cp/typeck.c.jj	2016-02-16 16:14:40.657073685 +0100
+++ gcc/cp/typeck.c	2016-02-16 17:26:55.352587215 +0100
@@ -4514,11 +4514,6 @@ cp_build_binary_op (location_t location,
 	       || (code0 == POINTER_TYPE
 		   && TYPE_PTR_P (type1) && integer_zerop (op1)))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op0) == PARM_DECL && nonnull_arg_p (op0))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op0);
-
 	  if (TYPE_PTR_P (type1))
 	    result_type = composite_pointer_type (type0, type1, op0, op1,
 						  CPO_COMPARISON, complain);
@@ -4558,11 +4553,6 @@ cp_build_binary_op (location_t location,
 	       || (code1 == POINTER_TYPE
 		   && TYPE_PTR_P (type0) && integer_zerop (op0)))
 	{
-	  if (warn_nonnull
-	      && TREE_CODE (op1) == PARM_DECL && nonnull_arg_p (op1))
-	    warning_at (location, OPT_Wnonnull,
-			"nonnull argument %qD compared to NULL", op1);
-
 	  if (TYPE_PTR_P (type0))
 	    result_type = composite_pointer_type (type0, type1, op0, op1,
 						  CPO_COMPARISON, complain);
--- gcc/testsuite/c-c++-common/nonnull-1.c.jj	2016-02-16 16:14:40.310078470 +0100
+++ gcc/testsuite/c-c++-common/nonnull-1.c	2016-02-16 17:26:55.352587215 +0100
@@ -1,7 +1,7 @@
 /* Test for the bad usage of "nonnull" function attribute parms.  */
 /*  */
 /* { dg-do compile } */
-/* { dg-options "-Wnonnull" } */
+/* { dg-options "-Wnonnull-compare" } */
 
 #include <stddef.h>
 #include <stdlib.h>
--- gcc/testsuite/c-c++-common/nonnull-2.c.jj	2016-02-16 17:26:55.353587201 +0100
+++ gcc/testsuite/c-c++-common/nonnull-2.c	2016-02-16 17:26:55.353587201 +0100
@@ -0,0 +1,26 @@
+/* Test for the bad usage of "nonnull" function attribute parms.  */
+/* { dg-do compile } */
+/* { dg-options "-Wnonnull-compare" } */
+
+void bar (char **);
+
+__attribute__((nonnull (1, 3))) int
+foo (char *cp1, char *cp2, char *cp3, char *cp4)
+{
+  if (cp1 == (char *) 0) /* { dg-warning "nonnull argument" "cp1 compared to NULL" } */
+    return 1;
+
+  cp1 = cp2;
+  if (cp1 == (char *) 0) /* { dg-bogus "nonnull argument" } */
+    return 2;
+
+  if (!cp4)	   /* { dg-bogus "nonnull argument" } */
+    return 3;
+
+  char **p = &cp3;
+  bar (p);
+  if (cp3 == (char *) 0) /* { dg-bogus "nonnull argument" } */
+    return 4;
+
+  return 5;
+}

	Jakub

Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Jeff Law]

On 02/16/2016 11:21 AM, Jakub Jelinek wrote:
> On Tue, Feb 16, 2016 at 11:12:27AM -0700, Jeff Law wrote:
>>> Not sure if it matters but wouldn't walking over function parameters
>>> and their default def SSA names immediate uses be cheaper than
>>> matching all conditions?  Esp. as nonnull_arg_p walks over all
>>> DECL_ARGUMENTS (up to the searched index) over and over again?
>>> [we should simply set a flag on the PARM_DECL!]
>> Seems like a waste to burn a flag bit for something like this.
>>
>> We're already walking all the statements in this code so it's really just
>> the cost of testing if the existing statement is "interesting" for this
>> warning.  I guess you're suggesting jakub walk the arguments and if one is a
>> marked as non-null, then walk the immediate uses.  Yea, that's probably
>> faster than doing the test for every statement.
>
> I'm already bootstrapping/regtesting following variant.
>
>> Swapping the nonnull_arg_p and the test for whether or not op1 is 0 would be
>> a slight help as well, particularly if there was machine generated code with
>> oodles of arguments.
>>
>> I'm not sure why we're testing OFFSET_TYPE against -1.
>
> That is because NULL pointers to member are represented as -1 in the middle
> end.
>
> 2016-02-16  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR c/69835
> 	* common.opt (Wnonnull-compare): New warning.
> 	* doc/invoke.texi (-Wnonnull): Remove text about comparison
> 	of arguments against NULL.
> 	(-Wnonnull-compare): Document.
> 	* Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o.
> 	* tree-pass.h (make_pass_warn_nonnull_compare): Declare.
> 	* passes.def (pass_warn_nonnull_compare): Add.
> 	* gimple-ssa-nonnull-compare.c: New file.
> c-family/
> 	* c.opt (Wnonnull-compare): Enable for -Wall.
> c/
> 	* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
> cp/
> 	* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
> testsuite/
> 	* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
> 	-Wnonnull in dg-options.
> 	* c-c++-common/nonnull-2.c: New test.
That works for me.

FWIW I'm certainly not a fan of doing this stuff on the FEs; too many 
false positives and too easy to miss valid warnings.  So I'm in full 
support to pushing this a bit further back in the pipeline.

Jeff

Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Jakub Jelinek]

On Tue, Feb 16, 2016 at 07:21:49PM +0100, Jakub Jelinek wrote:
> I'm already bootstrapping/regtesting following variant.
> 
> 2016-02-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/69835
> 	* common.opt (Wnonnull-compare): New warning.
> 	* doc/invoke.texi (-Wnonnull): Remove text about comparison
> 	of arguments against NULL.
> 	(-Wnonnull-compare): Document.
> 	* Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o.
> 	* tree-pass.h (make_pass_warn_nonnull_compare): Declare.
> 	* passes.def (pass_warn_nonnull_compare): Add.
> 	* gimple-ssa-nonnull-compare.c: New file.
> c-family/
> 	* c.opt (Wnonnull-compare): Enable for -Wall.
> c/
> 	* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
> cp/
> 	* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
> testsuite/
> 	* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
> 	-Wnonnull in dg-options.
> 	* c-c++-common/nonnull-2.c: New test.

Now successfully bootstrapped/regtested on both x86_64-linux and i686-linux.
Ok for trunk?

	Jakub

Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)

[Jeff Law]

On 02/16/2016 01:01 PM, Jakub Jelinek wrote:
> On Tue, Feb 16, 2016 at 07:21:49PM +0100, Jakub Jelinek wrote:
>> I'm already bootstrapping/regtesting following variant.
>>
>> 2016-02-16  Jakub Jelinek  <jakub@redhat.com>
>>
>> 	PR c/69835
>> 	* common.opt (Wnonnull-compare): New warning.
>> 	* doc/invoke.texi (-Wnonnull): Remove text about comparison
>> 	of arguments against NULL.
>> 	(-Wnonnull-compare): Document.
>> 	* Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o.
>> 	* tree-pass.h (make_pass_warn_nonnull_compare): Declare.
>> 	* passes.def (pass_warn_nonnull_compare): Add.
>> 	* gimple-ssa-nonnull-compare.c: New file.
>> c-family/
>> 	* c.opt (Wnonnull-compare): Enable for -Wall.
>> c/
>> 	* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
>> cp/
>> 	* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
>> testsuite/
>> 	* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
>> 	-Wnonnull in dg-options.
>> 	* c-c++-common/nonnull-2.c: New test.
>
> Now successfully bootstrapped/regtested on both x86_64-linux and i686-linux.
> Ok for trunk?
Yes.  Sorry I wasn't explicit.

jeff