From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-4027.protonmail.ch (mail-4027.protonmail.ch [185.70.40.27]) by sourceware.org (Postfix) with ESMTPS id 6A1CB3851C0C for ; Fri, 3 Jun 2022 16:35:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6A1CB3851C0C Date: Fri, 03 Jun 2022 16:34:48 +0000 To: "gcc@gcc.gnu.org" From: Miika Reply-To: Miika Subject: [RFC] Support for nonzero attribute Message-ID: Feedback-ID: 17471336:user:proton MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Jun 2022 16:35:04 -0000 Hello, I would like to add support for new attribute: nonzero. Nonzero attribute works the same way as nonnull but instead of checking for NULL, it checks for integer or enum with value 0. Nonzero attribute would issue warnings with new compiler flag -Wnonzero and -Wnonzero-compare. Nonzero could be useful when user wants to make sure that for example enum with value of 0 is not used or flag argument is not set to 0. For example compiling following code with "gcc -Wnonzero -Wnonzero-compare = foo.c" #include enum bar{NONE, SOME}; void foo(int d, enum bar b) __attribute__ ((nonzero (1, 2))); void foo(int d, enum bar b) { printf("%d\n", d =3D=3D 0); printf("%d\n", b =3D=3D NONE); } int main() { foo(0, NONE); } Would give the following error foo.c: In function 'main': foo.c:11:9: warning: zero argument where nonzero required (argument 1) [-Wn= onzero] 11 | foo(0, NONE); | ^~~ foo.c:11:9: warning: zero argument where nonzero required (argument 2) [-Wn= onzero] foo.c: In function 'foo': foo.c:6:9: warning: 'nonzero' argument 'd' compared to 0 [-Wnonzero-compare= ] 6 | printf("%d\n", d =3D=3D 0); | ^~~~~~~~~~~~~~~~~~~~~~ foo.c:7:9: warning: 'nonzero' argument 'b' compared to 0 [-Wnonzero-compare= ] 7 | printf("%d\n", b =3D=3D NONE); | ^~~~~~~~~~~~~~~~~~~~~~~~~ I attached a diff of my POC implementation. It's basically a copied and mod= ified version of nonnull attribute. The diff is fairly big and doesn't contain an= y tests. I'm more than happy to figure out tests for it if people think that support= ing nonzero attribute is a good idea. This is my first time working with GCC so please let me know if there's any= mistakes! Best wishes, Miika Alikirri --- diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 5d05e8e0dc8..ae5db84cdfa 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1360,6 +1360,7 @@ OBJS =3D \ =09gimple-ssa-evrp-analyze.o \ =09gimple-ssa-isolate-paths.o \ =09gimple-ssa-nonnull-compare.o \ +=09gimple-ssa-nonzero-compare.o \ =09gimple-ssa-split-paths.o \ =09gimple-ssa-store-merging.o \ =09gimple-ssa-strength-reduction.o \ diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def index 3239311b5a4..04db95d27ff 100644 --- a/gcc/builtin-attrs.def +++ b/gcc/builtin-attrs.def @@ -98,6 +98,7 @@ DEF_ATTR_IDENT (ATTR_FORMAT, "format") DEF_ATTR_IDENT (ATTR_FORMAT_ARG, "format_arg") DEF_ATTR_IDENT (ATTR_MALLOC, "malloc") DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull") +DEF_ATTR_IDENT (ATTR_NONZERO, "nonzero") DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn") DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow") DEF_ATTR_IDENT (ATTR_LEAF, "leaf") diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index ac936d5bbbb..b76e56b14b9 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -119,6 +119,7 @@ static tree handle_novops_attribute (tree *, tree, tree= , int, bool *); static tree handle_vector_size_attribute (tree *, tree, tree, int, =09=09=09=09=09 bool *); static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *); +static tree handle_nonzero_attribute (tree *, tree, tree, int, bool *); static tree handle_nonstring_attribute (tree *, tree, tree, int, bool *); static tree handle_nothrow_attribute (tree *, tree, tree, int, bool *); static tree handle_cleanup_attribute (tree *, tree, tree, int, bool *); @@ -379,6 +380,8 @@ const struct attribute_spec c_common_attribute_table[] = =3D =09=09=09 handle_tls_model_attribute, NULL }, { "nonnull", 0, -1, false, true, true, false, =09=09=09 handle_nonnull_attribute, NULL }, + { "nonzero", 0, -1, false, true, true, false, +=09=09=09 handle_nonzero_attribute, NULL }, { "nonstring", 0, 0, true, false, false, false, =09=09=09 handle_nonstring_attribute, NULL }, { "nothrow", 0, 0, true, false, false, false, @@ -3754,6 +3757,55 @@ handle_nonnull_attribute (tree *node, tree name, return NULL_TREE; } +/* Handle the "nonzero" attribute. */ + +static tree +handle_nonzero_attribute (tree *node, tree name, +=09=09=09 tree args, int ARG_UNUSED (flags), +=09=09=09 bool *no_add_attrs) +{ + tree type =3D *node; + + /* If no arguments are specified, all int arguments should be + non-zero. Verify a full prototype is given so that the arguments + will have the correct types when we actually check them later. + Avoid diagnosing type-generic built-ins since those have no + prototype. */ + if (!args) + { + if (!prototype_p (type) +=09 && (!TYPE_ATTRIBUTES (type) +=09 || !lookup_attribute ("type generic", TYPE_ATTRIBUTES (type)))) +=09{ +=09 error ("%qE attribute without arguments on a non-prototype", +=09=09 name); +=09 *no_add_attrs =3D true; +=09} + return NULL_TREE; + } + + for (int i =3D 1; args; ++i) + { + tree pos =3D TREE_VALUE (args); + /* NEXT is zero when the attribute includes just one argument. +=09 That's used to tell positional_argument to avoid mentioning +=09 the argument number in diagnostics (since there's just one +=09 mentioning it is unnecessary and coule be confusing). */ + tree next =3D TREE_CHAIN (args); + if (tree val =3D positional_argument (type, name, pos, INTEGER_TYPE, +=09=09=09=09=09 next || i > 1 ? i : 0)) +=09TREE_VALUE (args) =3D val; + else +=09{ +=09 *no_add_attrs =3D true; +=09 break; +=09} + args =3D next; + } + + return NULL_TREE; +} + /* Handle the "nonstring" variable attribute. */ static tree diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 20258c331af..5646beae512 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -315,6 +315,9 @@ static tree check_case_value (location_t, tree); static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT); static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT); +static void check_nonzero_arg (void *, tree, unsigned HOST_WIDE_INT); +static bool nonzero_check_p (tree, unsigned HOST_WIDE_INT); + /* Reserved words. The third field is a mask: keywords are disabled if they match the mask. @@ -5342,6 +5345,65 @@ check_function_nonnull (location_t loc, tree attrs, = int nargs, tree *argarray) return ctx.warned_p; } +/* Data to communicate through check_function_arguments_recurse between + check_function_nonzero and check_nonzero_arg. */ + +struct nonzero_arg_ctx +{ + location_t loc; + bool warned_p; +}; + +/* Check the argument list of a function call for null in argument slots + that are marked as requiring a non-null pointer argument. The NARGS + arguments are passed in the array ARGARRAY. Return true if we have + warned. */ + +static bool +check_function_nonzero (location_t loc, tree attrs, int nargs, tree *argar= ray) +{ + tree a; + int i; + + attrs =3D lookup_attribute ("nonzero", attrs); + if (attrs =3D=3D NULL_TREE) + return false; + + a =3D attrs; + /* See if any of the nonzero attributes has no arguments. If so, + then every pointer argument is checked (in which case the check + for pointer type is done in check_nonzero_arg). */ + if (TREE_VALUE (a) !=3D NULL_TREE) + do + a =3D lookup_attribute ("nonzero", TREE_CHAIN (a)); + while (a !=3D NULL_TREE && TREE_VALUE (a) !=3D NULL_TREE); + + struct nonzero_arg_ctx ctx =3D { loc, false }; + if (a !=3D NULL_TREE) + for (i =3D 0; i < nargs; i++) + check_function_arguments_recurse (check_nonzero_arg, &ctx, argarray[= i], +=09=09=09=09=09i + 1); + else + { + /* Walk the argument list. If we encounter an argument number we +=09 should check for non-zero, do it. */ + for (i =3D 0; i < nargs; i++) +=09{ +=09 for (a =3D attrs; ; a =3D TREE_CHAIN (a)) +=09 { +=09 a =3D lookup_attribute ("nonzero", a); +=09 if (a =3D=3D NULL_TREE || nonzero_check_p (TREE_VALUE (a), i + 1)= ) +=09=09break; +=09 } + +=09 if (a !=3D NULL_TREE) +=09 check_function_arguments_recurse (check_nonzero_arg, &ctx, +=09=09=09=09=09 argarray[i], i + 1); +=09} + } + return ctx.warned_p; +} + /* Check that the Nth argument of a function call (counting backwards from the end) is a (pointer)0. The NARGS arguments are passed in the array ARGARRAY. */ @@ -5503,6 +5565,53 @@ check_nonnull_arg (void *ctx, tree param, unsigned H= OST_WIDE_INT param_num) } } +/* Helper for check_function_nonzero; given a list of operands which + must be non-zero in ARGS, determine if operand PARAM_NUM should be + checked. */ + +static bool +nonzero_check_p (tree args, unsigned HOST_WIDE_INT param_num) +{ + unsigned HOST_WIDE_INT arg_num =3D 0; + + for (; args; args =3D TREE_CHAIN (args)) + { + bool found =3D get_attribute_operand (TREE_VALUE (args), &arg_num); + + gcc_assert (found); + + if (arg_num =3D=3D param_num) +=09return true; + } + return false; +} + +/* Check that the function argument PARAM (which is operand number + PARAM_NUM) is non-zero. This is called by check_function_nonzero + via check_function_arguments_recurse. */ + +static void +check_nonzero_arg (void *ctx, tree param, unsigned HOST_WIDE_INT param_num= ) +{ + struct nonzero_arg_ctx *pctx =3D (struct nonzero_arg_ctx *) ctx; + + /* Just skip checking the argument if it's not a int or num. This can + happen if the "nonzero" attribute was given without an operand + list (which means to check every int argument). */ + + enum tree_code type =3D TREE_CODE (TREE_TYPE (param)); + if (type !=3D INTEGER_TYPE && type !=3D ENUMERAL_TYPE) + return; + + /* Diagnose the simple cases of zero arguments. */ + if (integer_zerop (fold_for_warn (param))) + { + warning_at (pctx->loc, OPT_Wnonzero, "zero argument where nonzero " +=09=09 "required (argument %lu)", (unsigned long) param_num); + pctx->warned_p =3D true; + } +} + /* Helper for attribute handling; fetch the operand number from the attribute argument list. */ @@ -5703,7 +5812,7 @@ attribute_fallthrough_p (tree attr) =0C /* Check for valid arguments being passed to a function with FNTYPE. There are NARGS arguments in the array ARGARRAY. LOC should be used - for diagnostics. Return true if either -Wnonnull or -Wrestrict has + for diagnostics. Return true if -Wnonzero, -Wnonnull or -Wrestrict has been issued. The arguments in ARGARRAY may not have been folded yet (e.g. for C++, @@ -5723,6 +5832,10 @@ check_function_arguments (location_t loc, const_tree= fndecl, const_tree fntype, warned_p =3D check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype), =09=09=09=09 nargs, argarray); + if (warn_nonzero) + warned_p =3D check_function_nonzero (loc, TYPE_ATTRIBUTES (fntype), +=09=09=09=09 nargs, argarray); + /* Check for errors in format strings. */ if (warn_format || warn_suggest_attribute_format) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index c49da99d395..49344f0af7f 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -932,6 +932,18 @@ Wnonnull-compare C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall) ; +Wnonzero +C Var(warn_nonzero) Warning LangEnabledBy(C,Wformat=3D,warn_format >=3D 1,= 0) +Warn about zero being passed to argument slots marked as requiring non-zer= o. + +Wnonzero +C LangEnabledBy(C,Wall) +; + +Wnonzero-compare +C LangEnabledBy(C ObjC C++ ObjC++,Wall) +; + Wnormalized C ObjC C++ ObjC++ Warning Alias(Wnormalized=3D,nfc,none) ; diff --git a/gcc/common.opt b/gcc/common.opt index 3ec7743eae8..f1d86920a84 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -643,6 +643,10 @@ Wnonnull-compare Var(warn_nonnull_compare) Warning Warn if comparing pointer parameter with nonnull attribute with NULL. +Wnonzero-compare +Var(warn_nonzero_compare) Warning +Warn if comparing pointer parameter with nonzero attribute with 0. + Wnull-dereference Common Var(warn_null_dereference) Warning Warn if dereferencing a NULL pointer may lead to erroneous or undefined be= havior. diff --git a/gcc/gimple-ssa-nonzero-compare.c b/gcc/gimple-ssa-nonzero-comp= are.c new file mode 100644 index 00000000000..8fd27ec5790 --- /dev/null +++ b/gcc/gimple-ssa-nonzero-compare.c @@ -0,0 +1,152 @@ +/* -Wnonzero-compare warning support. + Copyright (C) 2022 Free Software Foundation, Inc. + Contributed by Miika Alikirri + +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 +. */ + +#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 nonzero_arg_p argument initial values + with 0. */ + +static void +do_warn_nonzero_compare (function *fun, tree arg) +{ + if (!INTEGRAL_TYPE_P (TREE_TYPE (arg)) + && TREE_CODE (TREE_TYPE (arg)) !=3D OFFSET_TYPE) + return; + + if (!nonzero_arg_p (arg)) + return; + + tree d =3D ssa_default_def (fun, arg); + if (d =3D=3D NULL_TREE) + return; + + use_operand_p use_p; + imm_use_iterator iter; + + FOR_EACH_IMM_USE_FAST (use_p, iter, d) + { + gimple *stmt =3D USE_STMT (use_p); + tree op =3D NULL_TREE; + location_t loc =3D gimple_location (stmt); + if (gimple_code (stmt) =3D=3D GIMPLE_COND) +=09switch (gimple_cond_code (stmt)) +=09 { +=09 case EQ_EXPR: +=09 case NE_EXPR: +=09 if (gimple_cond_lhs (stmt) =3D=3D d) +=09 op =3D gimple_cond_rhs (stmt); +=09 break; +=09 default: +=09 break; +=09 } + else if (is_gimple_assign (stmt)) +=09switch (gimple_assign_rhs_code (stmt)) +=09 { +=09 case EQ_EXPR: +=09 case NE_EXPR: +=09 if (gimple_assign_rhs1 (stmt) =3D=3D d) +=09 op =3D gimple_assign_rhs2 (stmt); +=09 break; +=09 case COND_EXPR: +=09 switch (TREE_CODE (gimple_assign_rhs1 (stmt))) +=09 { +=09 case EQ_EXPR: +=09 case NE_EXPR: +=09=09op =3D gimple_assign_rhs1 (stmt); +=09=09if (TREE_OPERAND (op, 0) !=3D d) +=09=09 { +=09=09 op =3D NULL_TREE; +=09=09 break; +=09=09 } +=09=09loc =3D EXPR_LOC_OR_LOC (op, loc); +=09=09op =3D TREE_OPERAND (op, 1); +=09=09break; +=09 default: +=09=09break; +=09 } +=09 break; +=09 default: +=09 break; +=09 } + if (op +=09 && (INTEGRAL_TYPE_P (TREE_TYPE (arg)) +=09 ? integer_zerop (op) : integer_minus_onep (op)) +=09 && !gimple_no_warning_p (stmt)) +=09warning_at (loc, OPT_Wnonzero_compare, +=09=09 "% argument %qD compared to 0", arg); + } +} + +namespace { + +const pass_data pass_data_warn_nonzero_compare =3D +{ + GIMPLE_PASS, /* type */ + "*nonzerocmp", /* 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_nonzero_compare : public gimple_opt_pass +{ +public: + pass_warn_nonzero_compare (gcc::context *ctxt) + : gimple_opt_pass (pass_data_warn_nonzero_compare, ctxt) + {} + + /* opt_pass methods: */ + virtual bool gate (function *) { return warn_nonzero_compare; } + + virtual unsigned int execute (function *); + +}; // class pass_warn_nonzero_compare + +unsigned int +pass_warn_nonzero_compare::execute (function *fun) +{ + if (fun->static_chain_decl) + do_warn_nonzero_compare (fun, fun->static_chain_decl); + + for (tree arg =3D DECL_ARGUMENTS (cfun->decl); arg; arg =3D DECL_CHAIN (= arg)) + do_warn_nonzero_compare (fun, arg); + return 0; +} + +} // anon namespace + +gimple_opt_pass * +make_pass_warn_nonzero_compare (gcc::context *ctxt) +{ + return new pass_warn_nonzero_compare (ctxt); +} diff --git a/gcc/passes.def b/gcc/passes.def index 2bf2cb78fc5..e3a382a62c5 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_fixup_cfg); NEXT_PASS (pass_build_ssa); NEXT_PASS (pass_warn_nonnull_compare); + NEXT_PASS (pass_warn_nonzero_compare); NEXT_PASS (pass_early_warn_uninitialized); NEXT_PASS (pass_ubsan); NEXT_PASS (pass_nothrow); diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index a1207a20a3c..0b9012e3401 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -476,6 +476,7 @@ extern simple_ipa_opt_pass *make_pass_ipa_oacc (gcc::co= ntext *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= ); +extern gimple_opt_pass *make_pass_warn_nonzero_compare (gcc::context *ctxt= ); extern gimple_opt_pass *make_pass_sprintf_length (gcc::context *ctxt); extern gimple_opt_pass *make_pass_walloca (gcc::context *ctxt); extern gimple_opt_pass *make_pass_coroutine_lower_builtins (gcc::context *= ctxt); diff --git a/gcc/tree.c b/gcc/tree.c index 78fce74ff78..2ecfc3e19f7 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -14875,6 +14875,55 @@ nonnull_arg_p (const_tree arg) return false; } +/* Return true if ARG is marked with the nonzero attribute in the + current function signature. */ + +bool +nonzero_arg_p (const_tree arg) +{ + tree t, attrs, fntype; + unsigned HOST_WIDE_INT arg_num; + + // FIXME: support floats + gcc_assert (TREE_CODE (arg) =3D=3D PARM_DECL +=09 && (INTEGRAL_TYPE_P (TREE_TYPE (arg)) +=09=09 || TREE_CODE (TREE_TYPE (arg)) =3D=3D OFFSET_TYPE)); + + fntype =3D TREE_TYPE (cfun->decl); + for (attrs =3D TYPE_ATTRIBUTES (fntype); attrs; attrs =3D TREE_CHAIN (at= trs)) + { + attrs =3D lookup_attribute ("nonzero", attrs); + + /* If "nonzero" wasn't specified, we know nothing about the argument= . */ + if (attrs =3D=3D NULL_TREE) +=09return false; + + /* If "nonzero" applies to all the arguments, then ARG is non-null. = */ + if (TREE_VALUE (attrs) =3D=3D NULL_TREE) +=09return true; + + /* Get the position number for ARG in the function signature. */ + for (arg_num =3D 1, t =3D DECL_ARGUMENTS (cfun->decl); +=09 t; +=09 t =3D DECL_CHAIN (t), arg_num++) +=09{ +=09 if (t =3D=3D arg) +=09 break; +=09} + + gcc_assert (t =3D=3D arg); + + /* Now see if ARG_NUM is mentioned in the nonzero list. */ + for (t =3D TREE_VALUE (attrs); t; t =3D TREE_CHAIN (t)) +=09{ +=09 if (compare_tree_int (TREE_VALUE (t), arg_num) =3D=3D 0) +=09 return true; +=09} + } + + return false; +} + /* Combine LOC and BLOCK to a combined adhoc loc, retaining any range information. */ diff --git a/gcc/tree.h b/gcc/tree.h index 76014d9ce56..8ecb66fcc53 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -6183,6 +6183,7 @@ extern void gt_pch_nx (tree &); extern void gt_pch_nx (tree &, gt_pointer_operator, void *); extern bool nonnull_arg_p (const_tree); +extern bool nonzero_arg_p (const_tree); extern bool default_is_empty_record (const_tree); extern bool flexible_array_type_p (const_tree); extern HOST_WIDE_INT arg_int_size_in_bytes (const_tree);