| Message ID | 20160216152925.GU3017@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
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 > >
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
--- 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; +}