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 <ja...@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
