[RFC] Support for nonzero attribute

[RFC] Support for nonzero attribute

[Miika]

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 <stdio.h>
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 == 0);
        printf("%d\n", b == 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) [-Wnonzero]
   11 |         foo(0, NONE);
      |         ^~~
foo.c:11:9: warning: zero argument where nonzero required (argument 2) [-Wnonzero]
foo.c: In function 'foo':
foo.c:6:9: warning: 'nonzero' argument 'd' compared to 0 [-Wnonzero-compare]
    6 |         printf("%d\n", d == 0);
      |         ^~~~~~~~~~~~~~~~~~~~~~
foo.c:7:9: warning: 'nonzero' argument 'b' compared to 0 [-Wnonzero-compare]
    7 |         printf("%d\n", b == NONE);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~


I attached a diff of my POC implementation. It's basically a copied and modified
version of nonnull attribute. The diff is fairly big and doesn't contain any tests.
I'm more than happy to figure out tests for it if people think that supporting
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 = \
 	gimple-ssa-evrp-analyze.o \
 	gimple-ssa-isolate-paths.o \
 	gimple-ssa-nonnull-compare.o \
+	gimple-ssa-nonzero-compare.o \
 	gimple-ssa-split-paths.o \
 	gimple-ssa-store-merging.o \
 	gimple-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,
 					  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[] =
 			      handle_tls_model_attribute, NULL },
   { "nonnull",                0, -1, false, true, true, false,
 			      handle_nonnull_attribute, NULL },
+  { "nonzero",                0, -1, false, true, true, false,
+			      handle_nonzero_attribute, NULL },
   { "nonstring",              0, 0, true, false, false, false,
 			      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,
+			  tree args, int ARG_UNUSED (flags),
+			  bool *no_add_attrs)
+{
+  tree type = *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)
+	  && (!TYPE_ATTRIBUTES (type)
+	      || !lookup_attribute ("type generic", TYPE_ATTRIBUTES (type))))
+	{
+	  error ("%qE attribute without arguments on a non-prototype",
+		 name);
+	  *no_add_attrs = true;
+	}
+      return NULL_TREE;
+    }
+
+  for (int i = 1; args; ++i)
+    {
+      tree pos = TREE_VALUE (args);
+      /* NEXT is zero when the attribute includes just one argument.
+	 That's used to tell positional_argument to avoid mentioning
+	 the argument number in diagnostics (since there's just one
+	 mentioning it is unnecessary and coule be confusing).  */
+      tree next = TREE_CHAIN (args);
+      if (tree val = positional_argument (type, name, pos, INTEGER_TYPE,
+					  next || i > 1 ? i : 0))
+	TREE_VALUE (args) = val;
+      else
+	{
+	  *no_add_attrs = true;
+	  break;
+	}
+      args = 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 *argarray)
+{
+  tree a;
+  int i;
+
+  attrs = lookup_attribute ("nonzero", attrs);
+  if (attrs == NULL_TREE)
+    return false;
+
+  a = 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) != NULL_TREE)
+    do
+      a = lookup_attribute ("nonzero", TREE_CHAIN (a));
+    while (a != NULL_TREE && TREE_VALUE (a) != NULL_TREE);
+
+  struct nonzero_arg_ctx ctx = { loc, false };
+  if (a != NULL_TREE)
+    for (i = 0; i < nargs; i++)
+      check_function_arguments_recurse (check_nonzero_arg, &ctx, argarray[i],
+					i + 1);
+  else
+    {
+      /* Walk the argument list.  If we encounter an argument number we
+	 should check for non-zero, do it.  */
+      for (i = 0; i < nargs; i++)
+	{
+	  for (a = attrs; ; a = TREE_CHAIN (a))
+	    {
+	      a = lookup_attribute ("nonzero", a);
+	      if (a == NULL_TREE || nonzero_check_p (TREE_VALUE (a), i + 1))
+		break;
+	    }
+
+	  if (a != NULL_TREE)
+	    check_function_arguments_recurse (check_nonzero_arg, &ctx,
+					      argarray[i], i + 1);
+	}
+    }
+  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 HOST_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 = 0;
+
+  for (; args; args = TREE_CHAIN (args))
+    {
+      bool found = get_attribute_operand (TREE_VALUE (args), &arg_num);
+
+      gcc_assert (found);
+
+      if (arg_num == param_num)
+	return 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 = (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 = TREE_CODE (TREE_TYPE (param));
+  if (type != INTEGER_TYPE && type != 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 "
+		  "required (argument %lu)", (unsigned long) param_num);
+      pctx->warned_p = true;
+    }
+}
+
 /* Helper for attribute handling; fetch the operand number from
    the attribute argument list.  */

@@ -5703,7 +5812,7 @@ attribute_fallthrough_p (tree attr)
 \f
 /* 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 = check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype),
 				       nargs, argarray);

+  if (warn_nonzero)
+    warned_p = check_function_nonzero (loc, TYPE_ATTRIBUTES (fntype),
+				       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=,warn_format >= 1,0)
+Warn about zero being passed to argument slots marked as requiring non-zero.
+
+Wnonzero
+C LangEnabledBy(C,Wall)
+;
+
+Wnonzero-compare
+C LangEnabledBy(C ObjC C++ ObjC++,Wall)
+;
+
 Wnormalized
 C ObjC C++ ObjC++ Warning Alias(Wnormalized=,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 behavior.
diff --git a/gcc/gimple-ssa-nonzero-compare.c b/gcc/gimple-ssa-nonzero-compare.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 <nykseli@protonmail.com>
+
+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 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)) != OFFSET_TYPE)
+    return;
+
+  if (!nonzero_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
+	  && (INTEGRAL_TYPE_P (TREE_TYPE (arg))
+	      ? integer_zerop (op) : integer_minus_onep (op))
+	  && !gimple_no_warning_p (stmt))
+	warning_at (loc, OPT_Wnonzero_compare,
+		    "%<nonzero%> argument %qD compared to 0", arg);
+    }
+}
+
+namespace {
+
+const pass_data pass_data_warn_nonzero_compare =
+{
+  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 = DECL_ARGUMENTS (cfun->decl); arg; arg = 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::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);
+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) == PARM_DECL
+	      && (INTEGRAL_TYPE_P (TREE_TYPE (arg))
+		  || TREE_CODE (TREE_TYPE (arg)) == OFFSET_TYPE));
+
+  fntype = TREE_TYPE (cfun->decl);
+  for (attrs = TYPE_ATTRIBUTES (fntype); attrs; attrs = TREE_CHAIN (attrs))
+    {
+      attrs = lookup_attribute ("nonzero", attrs);
+
+      /* If "nonzero" wasn't specified, we know nothing about the argument.  */
+      if (attrs == NULL_TREE)
+	return false;
+
+      /* If "nonzero" applies to all the arguments, then ARG is non-null.  */
+      if (TREE_VALUE (attrs) == NULL_TREE)
+	return true;
+
+      /* Get the position number for ARG in the function signature.  */
+      for (arg_num = 1, t = DECL_ARGUMENTS (cfun->decl);
+	   t;
+	   t = DECL_CHAIN (t), arg_num++)
+	{
+	  if (t == arg)
+	    break;
+	}
+
+      gcc_assert (t == arg);
+
+      /* Now see if ARG_NUM is mentioned in the nonzero list.  */
+      for (t = TREE_VALUE (attrs); t; t = TREE_CHAIN (t))
+	{
+	  if (compare_tree_int (TREE_VALUE (t), arg_num) == 0)
+	    return true;
+	}
+    }
+
+  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);

Re: [RFC] Support for nonzero attribute

[Jakub Jelinek]

On Fri, Jun 03, 2022 at 04:34:48PM +0000, Miika via Gcc wrote:
> 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.

NULL/nullptr is very special pointer (at it doesn't point to any object),
while integer with value 0 is just one of many possible values.
For some functions, 0 could be a value it wants to avoid, for others
such value could be -1, negative value, positive, whatever else...
IMHO if we want to add anything like this, it should be more generic,
specify that a particular argument must have value in a specific range.

	Jakub

[RFC] Support for nonzero attribute

[Miika]

Thank you for the feedback!

On Friday, June 3rd, 2022 at 7:45 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> For some functions, 0 could be a value it wants to avoid, for others
> such value could be -1, negative value, positive, whatever else...
> IMHO if we want to add anything like this, it should be more generic,
> specify that a particular argument must have value in a specific range.

That's a really good point. Making it generic makes a lot more sense.
I'll try to design some kind of a range attribute and see how it feels.

[RFC] Support for nonzero attribute

[Miika]

Based on Jakub's and Yair's comments I created a new attribute "inrange".
Inrage takes three arguments, pos min and max.
Pos being the argument position in the function, and min and max defines the
range of valid integer. Both min and max are inclusive and work with enums.
Warnings are enabled with the new flag: "-Winrange".

The attribute definition would look something like this:
inrange(pos, min, max)


So for example compiling this with "gcc foo.c -Winrange":

#include <stdio.h>
void foo(int d) __attribute__ ((inrange (1, 100, 200)));
void foo(int d) {
        printf("%d\n", d == 0);
}

int main() {
        foo(0); // warning
        foo(100); // no warning
}

Would give the following error:

foo.c: In function 'main':
foo.c:8:9: warning: argument in position 1 not in rage of 100..200 [-Winrange]
    8 |         foo(0); // warning
      |         ^~~


I thought about having separate minval and maxval attributes but I personally
prefer that min and max values have to be defined explicitly.

If this looks good, I could look into applying inrange to types and variables
and after that I could start looking into optimization.

Patch for adding inrange is attached below

Miika

---
diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
index 3239311b5a4..2f5732b3ed2 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_INRANGE, "inrange")
 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..d6dc9c37723 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,
 					  bool *);
 static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
+static tree handle_inrange_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[] =
 			      handle_tls_model_attribute, NULL },
   { "nonnull",                0, -1, false, true, true, false,
 			      handle_nonnull_attribute, NULL },
+  { "inrange",                3, 3, false, true, true, false,
+			      handle_inrange_attribute, NULL },
   { "nonstring",              0, 0, true, false, false, false,
 			      handle_nonstring_attribute, NULL },
   { "nothrow",                0, 0, true,  false, false, false,
@@ -3754,6 +3757,59 @@ handle_nonnull_attribute (tree *node, tree name,
   return NULL_TREE;
 }

+/* Handle the "inrange" attribute.  */
+
+static tree
+handle_inrange_attribute (tree *node, tree name,
+			  tree args, int ARG_UNUSED (flags),
+			  bool *no_add_attrs)
+{
+  tree type = *node;
+
+  /* Test the position argument  */
+  tree pos = TREE_VALUE (args);
+  if (!positional_argument (type, name, pos, INTEGER_TYPE, 0))
+    *no_add_attrs = true;
+
+  /* Make sure that range args are INTEGRALs  */
+  bool range_err = false;
+  for (tree range = TREE_CHAIN (args); range; range = TREE_CHAIN (range))
+    {
+      tree val = TREE_VALUE (range);
+      if (val && TREE_CODE (val) != IDENTIFIER_NODE
+	  && TREE_CODE (val) != FUNCTION_DECL)
+	val = default_conversion (val);
+
+      if (TREE_CODE (val) != INTEGER_CST
+	  || !INTEGRAL_TYPE_P (TREE_TYPE (val)))
+	{
+	  warning (OPT_Wattributes,
+		   "range value is not an integral constant");
+	  *no_add_attrs = true;
+	  range_err = true;
+	}
+    }
+
+  /* Test the range arg max is not smaller than min
+     if range args are integrals  */
+  if (!range_err)
+    {
+      tree range = TREE_CHAIN (args);
+      tree min = TREE_VALUE(range);
+      range = TREE_CHAIN (range);
+      tree max = TREE_VALUE(range);
+      if (!tree_int_cst_le (min, max))
+	{
+	  warning (OPT_Wattributes,
+		   "min range is bigger than max range");
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
+    }
+
+  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..8936942fec8 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5342,6 +5342,51 @@ check_function_nonnull (location_t loc, tree attrs, int nargs, tree *argarray)
   return ctx.warned_p;
 }

+
+/* Check the argument list of a function call for invalid range
+   in argument slots that are marked as requiring a specific range.
+   Return true if we have warned. */
+
+static bool
+check_function_inrange (location_t loc, tree attrs, tree *argarray)
+{
+  tree a;
+  tree param;
+  int pos;
+  HOST_WIDE_INT min;
+  HOST_WIDE_INT max;
+  HOST_WIDE_INT value;
+  bool warned = false;
+
+  attrs = lookup_attribute ("inrange", attrs);
+  if (attrs == NULL_TREE)
+    return false;
+
+  /* Walk through attributes and check range values
+     when range attribute is found  */
+
+  for (a = attrs; a ; a = TREE_CHAIN (a))
+    {
+      a = lookup_attribute ("inrange", a);
+      tree op = TREE_VALUE (a);
+      pos = TREE_INT_CST_LOW (TREE_VALUE (op));
+      op = TREE_CHAIN (op);
+      min = tree_to_shwi (TREE_VALUE (op));
+      op = TREE_CHAIN (op);
+      max = tree_to_shwi (TREE_VALUE (op));
+      param = argarray[pos - 1];
+      value = TREE_INT_CST_LOW (param);
+      if (value < min || value > max)
+	{
+	    warning_at (loc, OPT_Winrange, "argument in position %u"
+			" not in rage of %ld..%ld", pos, min, max);
+	    warned = true;
+	}
+    }
+
+  return warned;
+}
+
 /* 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.  */
@@ -5703,7 +5748,7 @@ attribute_fallthrough_p (tree attr)
 \f
 /* 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 -Winrange, -Wnonnull or -Wrestrict has
    been issued.

    The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
@@ -5723,6 +5768,10 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
     warned_p = check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype),
 				       nargs, argarray);

+  if (warn_inrange)
+    warned_p = check_function_inrange (loc, TYPE_ATTRIBUTES (fntype),
+				       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..0b9aa597c54 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -932,6 +932,14 @@ Wnonnull-compare
 C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
 ;

+Winrange
+C Var(warn_inrange) Warning LangEnabledBy(C,Wformat=,warn_format >= 1,0)
+Warn about integer not being in specified range.
+
+Winrange
+C LangEnabledBy(C,Wall)
+;
+
 Wnormalized
 C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
 ;

Re: [RFC] Support for nonzero attribute

[Ben Boeckel]

On Sun, Jun 05, 2022 at 20:09:04 +0000, Miika via Gcc wrote:
> Based on Jakub's and Yair's comments I created a new attribute "inrange".
> Inrage takes three arguments, pos min and max.
> Pos being the argument position in the function, and min and max defines the
> range of valid integer. Both min and max are inclusive and work with enums.
> Warnings are enabled with the new flag: "-Winrange".

Is this something that could be applied to variables or types (I've not
much experience with GCC attribute internal mechanisms, so maybe not)?

For example:

```
// This variable must be in range.
int percentage __attribute__((inrange(0, 100)));
// Any variable declared through this typedef must be in range.
typedef int __attribute__((inrange(0, 100))) percentage_t;
```

--Ben

[RFC] Support for nonzero attribute

[Miika]

On Monday, June 6th, 2022 at 9:42 PM, Ben Boeckel <ben.boeckel@kitware.com> wrote:
> > Based on Jakub's and Yair's comments I created a new attribute "inrange".
> > Inrage takes three arguments, pos min and max.
> > Pos being the argument position in the function, and min and max defines the
> > range of valid integer. Both min and max are inclusive and work with enums.
> > Warnings are enabled with the new flag: "-Winrange".
>
>
> Is this something that could be applied to variables or types (I've not
> much experience with GCC attribute internal mechanisms, so maybe not)?

I took a closer look at it and looks like it can be applied.

So trying to compile this:
```
typedef int __attribute__((inrange(0, 100))) percentage_t;
int main() {
        int percentage __attribute__((inrange(0, 100))) = -1;
        percentage_t per __attribute__((inrange(0, 100))) = -1;
}
```

Would print out something like this:

foo.c: In function 'main':
foo.c:3:59: warning: inrange variable 'percentage' requires integer in rage of 0..100 [-Winrange]
    3 |         int percentage __attribute__((inrange(0, 100))) = -1;
      |                                                           ^
foo.c:4:61: warning: inrange variable 'per' requires integer in rage of 0..100 [-Winrange]
    4 |         percentage_t per __attribute__((inrange(0, 100))) = -1;
      |

Miika

Re: [RFC] Support for nonzero attribute

[Jonathan Wakely]

On Tue, 7 Jun 2022 at 20:40, Miika via Gcc <gcc@gcc.gnu.org> wrote:
>
> On Monday, June 6th, 2022 at 9:42 PM, Ben Boeckel <ben.boeckel@kitware.com> wrote:
> > > Based on Jakub's and Yair's comments I created a new attribute "inrange".
> > > Inrage takes three arguments, pos min and max.
> > > Pos being the argument position in the function, and min and max defines the
> > > range of valid integer. Both min and max are inclusive and work with enums.
> > > Warnings are enabled with the new flag: "-Winrange".
> >
> >
> > Is this something that could be applied to variables or types (I've not
> > much experience with GCC attribute internal mechanisms, so maybe not)?
>
> I took a closer look at it and looks like it can be applied.
>
> So trying to compile this:
> ```
> typedef int __attribute__((inrange(0, 100))) percentage_t;
> int main() {
>         int percentage __attribute__((inrange(0, 100))) = -1;
>         percentage_t per __attribute__((inrange(0, 100))) = -1;
> }
> ```
>
> Would print out something like this:
>
> foo.c: In function 'main':
> foo.c:3:59: warning: inrange variable 'percentage' requires integer in rage of 0..100 [-Winrange]

N.B. "rage" should be "range".

From the diagnostic it's not clear to me whether this is an inclusive
range. Is 0 allowed? Is 100 allowed?

Using [0,100] interval notation would imply both endpoints are valid,
which I think matches the semantics of your attribute. Is interval
notation sufficiently widely understood to use here?

Re: [RFC] Support for nonzero attribute

[Jonathan Wakely]

On Tue, 7 Jun 2022 at 20:44, Jonathan Wakely wrote:
>
> On Tue, 7 Jun 2022 at 20:40, Miika via Gcc <gcc@gcc.gnu.org> wrote:
> >
> > On Monday, June 6th, 2022 at 9:42 PM, Ben Boeckel <ben.boeckel@kitware.com> wrote:
> > > > Based on Jakub's and Yair's comments I created a new attribute "inrange".
> > > > Inrage takes three arguments, pos min and max.
> > > > Pos being the argument position in the function, and min and max defines the
> > > > range of valid integer. Both min and max are inclusive and work with enums.
> > > > Warnings are enabled with the new flag: "-Winrange".
> > >
> > >
> > > Is this something that could be applied to variables or types (I've not
> > > much experience with GCC attribute internal mechanisms, so maybe not)?
> >
> > I took a closer look at it and looks like it can be applied.
> >
> > So trying to compile this:
> > ```
> > typedef int __attribute__((inrange(0, 100))) percentage_t;
> > int main() {
> >         int percentage __attribute__((inrange(0, 100))) = -1;
> >         percentage_t per __attribute__((inrange(0, 100))) = -1;
> > }
> > ```
> >
> > Would print out something like this:
> >
> > foo.c: In function 'main':
> > foo.c:3:59: warning: inrange variable 'percentage' requires integer in rage of 0..100 [-Winrange]
>
> N.B. "rage" should be "range".
>
> From the diagnostic it's not clear to me whether this is an inclusive
> range. Is 0 allowed? Is 100 allowed?
>
> Using [0,100] interval notation would imply both endpoints are valid,
> which I think matches the semantics of your attribute. Is interval
> notation sufficiently widely understood to use here?

Oh, Wikipedia tells me that 0..100 already means that, as an integer interval:
https://en.wikipedia.org/wiki/Interval_(mathematics)#Integer_intervals

So maybe it's fine as-is (except for the "rage" typo).

[RFC] Support for nonzero attribute

[Miika]

On Tuesday, June 7th, 2022 at 10:46 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> On Tue, 7 Jun 2022 at 20:44, Jonathan Wakely wrote:
>
> > On Tue, 7 Jun 2022 at 20:40, Miika via Gcc gcc@gcc.gnu.org wrote:
> >
> > > On Monday, June 6th, 2022 at 9:42 PM, Ben Boeckel ben.boeckel@kitware.com wrote:
> > >
> > > > > Based on Jakub's and Yair's comments I created a new attribute "inrange".
> > > > > Inrage takes three arguments, pos min and max.
> > > > > Pos being the argument position in the function, and min and max defines the
> > > > > range of valid integer. Both min and max are inclusive and work with enums.
> > > > > Warnings are enabled with the new flag: "-Winrange".
> > > >
> > > > Is this something that could be applied to variables or types (I've not
> > > > much experience with GCC attribute internal mechanisms, so maybe not)?
> > >
> > > I took a closer look at it and looks like it can be applied.
> > >
> > > So trying to compile this:
> > > `typedef int __attribute__((inrange(0, 100))) percentage_t; int main() { int percentage __attribute__((inrange(0, 100))) = -1; percentage_t per __attribute__((inrange(0, 100))) = -1; }`
> > >
> > > Would print out something like this:
> > >
> > > foo.c: In function 'main':
> > > foo.c:3:59: warning: inrange variable 'percentage' requires integer in rage of 0..100 [-Winrange]
> >
> > N.B. "rage" should be "range".
> >
> > From the diagnostic it's not clear to me whether this is an inclusive
> > range. Is 0 allowed? Is 100 allowed?
> >
> > Using [0,100] interval notation would imply both endpoints are valid,
> > which I think matches the semantics of your attribute. Is interval
> > notation sufficiently widely understood to use here?
>
>
> Oh, Wikipedia tells me that 0..100 already means that, as an integer interval:
> https://en.wikipedia.org/wiki/Interval_(mathematics)#Integer_intervals
>
> So maybe it's fine as-is (except for the "rage" typo).

Thanks for noticing the typo. I probably need more sleep.

And I'm open to any suggestions about the diagnostic message since I'm not sure
what's the best way to explain the error to the user.

Re: [RFC] Support for nonzero attribute

[Eric Gallager]

On Sun, Jun 5, 2022 at 4:10 PM Miika via Gcc <gcc@gcc.gnu.org> wrote:
>
> Based on Jakub's and Yair's comments I created a new attribute "inrange".
> Inrage takes three arguments, pos min and max.
> Pos being the argument position in the function, and min and max defines the
> range of valid integer. Both min and max are inclusive and work with enums.
> Warnings are enabled with the new flag: "-Winrange".
>
> The attribute definition would look something like this:
> inrange(pos, min, max)
>
>
> So for example compiling this with "gcc foo.c -Winrange":
>
> #include <stdio.h>
> void foo(int d) __attribute__ ((inrange (1, 100, 200)));
> void foo(int d) {
>         printf("%d\n", d == 0);
> }
>
> int main() {
>         foo(0); // warning
>         foo(100); // no warning
> }
>
> Would give the following error:
>
> foo.c: In function 'main':
> foo.c:8:9: warning: argument in position 1 not in rage of 100..200 [-Winrange]
>     8 |         foo(0); // warning
>       |         ^~~
>
>
> I thought about having separate minval and maxval attributes but I personally
> prefer that min and max values have to be defined explicitly.
>
> If this looks good, I could look into applying inrange to types and variables
> and after that I could start looking into optimization.
>

Could you take a look at bug 78155 too? There was a request to add
something like this in that bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78155
(and I think I've seen similar requests elsewhere, too)

> Patch for adding inrange is attached below
>
> Miika
>
> ---
> diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
> index 3239311b5a4..2f5732b3ed2 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_INRANGE, "inrange")
>  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..d6dc9c37723 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,
>                                           bool *);
>  static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_inrange_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[] =
>                               handle_tls_model_attribute, NULL },
>    { "nonnull",                0, -1, false, true, true, false,
>                               handle_nonnull_attribute, NULL },
> +  { "inrange",                3, 3, false, true, true, false,
> +                             handle_inrange_attribute, NULL },
>    { "nonstring",              0, 0, true, false, false, false,
>                               handle_nonstring_attribute, NULL },
>    { "nothrow",                0, 0, true,  false, false, false,
> @@ -3754,6 +3757,59 @@ handle_nonnull_attribute (tree *node, tree name,
>    return NULL_TREE;
>  }
>
> +/* Handle the "inrange" attribute.  */
> +
> +static tree
> +handle_inrange_attribute (tree *node, tree name,
> +                         tree args, int ARG_UNUSED (flags),
> +                         bool *no_add_attrs)
> +{
> +  tree type = *node;
> +
> +  /* Test the position argument  */
> +  tree pos = TREE_VALUE (args);
> +  if (!positional_argument (type, name, pos, INTEGER_TYPE, 0))
> +    *no_add_attrs = true;
> +
> +  /* Make sure that range args are INTEGRALs  */
> +  bool range_err = false;
> +  for (tree range = TREE_CHAIN (args); range; range = TREE_CHAIN (range))
> +    {
> +      tree val = TREE_VALUE (range);
> +      if (val && TREE_CODE (val) != IDENTIFIER_NODE
> +         && TREE_CODE (val) != FUNCTION_DECL)
> +       val = default_conversion (val);
> +
> +      if (TREE_CODE (val) != INTEGER_CST
> +         || !INTEGRAL_TYPE_P (TREE_TYPE (val)))
> +       {
> +         warning (OPT_Wattributes,
> +                  "range value is not an integral constant");
> +         *no_add_attrs = true;
> +         range_err = true;
> +       }
> +    }
> +
> +  /* Test the range arg max is not smaller than min
> +     if range args are integrals  */
> +  if (!range_err)
> +    {
> +      tree range = TREE_CHAIN (args);
> +      tree min = TREE_VALUE(range);
> +      range = TREE_CHAIN (range);
> +      tree max = TREE_VALUE(range);
> +      if (!tree_int_cst_le (min, max))
> +       {
> +         warning (OPT_Wattributes,
> +                  "min range is bigger than max range");
> +         *no_add_attrs = true;
> +         return NULL_TREE;
> +       }
> +    }
> +
> +  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..8936942fec8 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -5342,6 +5342,51 @@ check_function_nonnull (location_t loc, tree attrs, int nargs, tree *argarray)
>    return ctx.warned_p;
>  }
>
> +
> +/* Check the argument list of a function call for invalid range
> +   in argument slots that are marked as requiring a specific range.
> +   Return true if we have warned. */
> +
> +static bool
> +check_function_inrange (location_t loc, tree attrs, tree *argarray)
> +{
> +  tree a;
> +  tree param;
> +  int pos;
> +  HOST_WIDE_INT min;
> +  HOST_WIDE_INT max;
> +  HOST_WIDE_INT value;
> +  bool warned = false;
> +
> +  attrs = lookup_attribute ("inrange", attrs);
> +  if (attrs == NULL_TREE)
> +    return false;
> +
> +  /* Walk through attributes and check range values
> +     when range attribute is found  */
> +
> +  for (a = attrs; a ; a = TREE_CHAIN (a))
> +    {
> +      a = lookup_attribute ("inrange", a);
> +      tree op = TREE_VALUE (a);
> +      pos = TREE_INT_CST_LOW (TREE_VALUE (op));
> +      op = TREE_CHAIN (op);
> +      min = tree_to_shwi (TREE_VALUE (op));
> +      op = TREE_CHAIN (op);
> +      max = tree_to_shwi (TREE_VALUE (op));
> +      param = argarray[pos - 1];
> +      value = TREE_INT_CST_LOW (param);
> +      if (value < min || value > max)
> +       {
> +           warning_at (loc, OPT_Winrange, "argument in position %u"
> +                       " not in rage of %ld..%ld", pos, min, max);
> +           warned = true;
> +       }
> +    }
> +
> +  return warned;
> +}
> +
>  /* 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.  */
> @@ -5703,7 +5748,7 @@ attribute_fallthrough_p (tree attr)
>
>  /* 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 -Winrange, -Wnonnull or -Wrestrict has
>     been issued.
>
>     The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
> @@ -5723,6 +5768,10 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
>      warned_p = check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype),
>                                        nargs, argarray);
>
> +  if (warn_inrange)
> +    warned_p = check_function_inrange (loc, TYPE_ATTRIBUTES (fntype),
> +                                      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..0b9aa597c54 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -932,6 +932,14 @@ Wnonnull-compare
>  C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
>  ;
>
> +Winrange
> +C Var(warn_inrange) Warning LangEnabledBy(C,Wformat=,warn_format >= 1,0)
> +Warn about integer not being in specified range.
> +
> +Winrange
> +C LangEnabledBy(C,Wall)
> +;
> +
>  Wnormalized
>  C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
>  ;
>
>
>

[RFC] Support for nonzero attribute

[Miika]

On Wednesday, June 8th, 2022 at 8:42 PM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> Could you take a look at bug 78155 too? There was a request to add
> something like this in that bug:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78155
> (and I think I've seen similar requests elsewhere, too)

I took a look at the bug and looks like the inrange attribute can be applied to
builtin functions too.

So now the example code
int main (void)
{
    __builtin_printf ("%i\n", __builtin_isalpha (999999));
}

Now gives the following error:

foo.c: In function 'main':
foo.c:3:31: warning: argument in position 1 not in rage of 0..255 [-Winrange]
    3 |     __builtin_printf ("%i\n", __builtin_isalpha (999999));
      |

Miika

Re: [RFC] Support for nonzero attribute

[Eric Gallager]

On Wed, Jun 8, 2022 at 5:00 PM Miika <nykseli@protonmail.com> wrote:
>
> On Wednesday, June 8th, 2022 at 8:42 PM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> > Could you take a look at bug 78155 too? There was a request to add
> > something like this in that bug:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78155
> > (and I think I've seen similar requests elsewhere, too)
>
> I took a look at the bug and looks like the inrange attribute can be applied to
> builtin functions too.
>
> So now the example code
> int main (void)
> {
>     __builtin_printf ("%i\n", __builtin_isalpha (999999));
> }
>
> Now gives the following error:
>
> foo.c: In function 'main':
> foo.c:3:31: warning: argument in position 1 not in rage of 0..255 [-Winrange]
>     3 |     __builtin_printf ("%i\n", __builtin_isalpha (999999));
>       |
>
> Miika

Nice, good to hear! I'm looking forward to seeing this get added!
Thanks,
Eric

Re: [RFC] Support for nonzero attribute

[Miika]

On Thursday, June 9th, 2022 at 7:36 AM, Eric Gallager <egall@gwmail.gwu.edu> wrote:
> Nice, good to hear! I'm looking forward to seeing this get added!

I'll write some tests and try to send the patches next week!

Miika

Re: [RFC] Support for nonzero attribute

[Prathamesh Kulkarni]

On Mon, 6 Jun 2022 at 01:39, Miika via Gcc <gcc@gcc.gnu.org> wrote:
>
> Based on Jakub's and Yair's comments I created a new attribute "inrange".
> Inrage takes three arguments, pos min and max.
> Pos being the argument position in the function, and min and max defines the
> range of valid integer. Both min and max are inclusive and work with enums.
> Warnings are enabled with the new flag: "-Winrange".
>
> The attribute definition would look something like this:
> inrange(pos, min, max)
>
>
> So for example compiling this with "gcc foo.c -Winrange":
>
> #include <stdio.h>
> void foo(int d) __attribute__ ((inrange (1, 100, 200)));
> void foo(int d) {
>         printf("%d\n", d == 0);
> }
>
> int main() {
>         foo(0); // warning
>         foo(100); // no warning
> }
>
> Would give the following error:
>
> foo.c: In function 'main':
> foo.c:8:9: warning: argument in position 1 not in rage of 100..200 [-Winrange]
>     8 |         foo(0); // warning
>       |         ^~~
>
>
> I thought about having separate minval and maxval attributes but I personally
> prefer that min and max values have to be defined explicitly.
>
> If this looks good, I could look into applying inrange to types and variables
> and after that I could start looking into optimization.
>
> Patch for adding inrange is attached below
Hi,
Thanks for the POC patch!
A few suggestions:

(1) It doesn't apply as-is because of transition from .c to .cc filenames.
Perhaps you're using an older version of trunk ?

(2) AFAIK, this warning will need an entry in doc/invoke.texi for
documenting it.

(3) While this patch addresses warning, I suppose it could be extended
so the tree optimizer
can take advantage of value range info provided by the attribute.
For example, the condition d > 20, could be optimized away in
following function by inferring
range from the attribute.

__attribute__((inrange (1, 10, 20)))
void foo(int d)
{
  if (d > 20)
    __builtin_abort ();
}

>
> Miika
>
> ---
> diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
> index 3239311b5a4..2f5732b3ed2 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_INRANGE, "inrange")
>  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..d6dc9c37723 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,
>                                           bool *);
>  static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_inrange_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[] =
>                               handle_tls_model_attribute, NULL },
>    { "nonnull",                0, -1, false, true, true, false,
>                               handle_nonnull_attribute, NULL },
> +  { "inrange",                3, 3, false, true, true, false,
> +                             handle_inrange_attribute, NULL },
>    { "nonstring",              0, 0, true, false, false, false,
>                               handle_nonstring_attribute, NULL },
>    { "nothrow",                0, 0, true,  false, false, false,
> @@ -3754,6 +3757,59 @@ handle_nonnull_attribute (tree *node, tree name,
>    return NULL_TREE;
>  }
>
> +/* Handle the "inrange" attribute.  */
> +
> +static tree
> +handle_inrange_attribute (tree *node, tree name,
> +                         tree args, int ARG_UNUSED (flags),
> +                         bool *no_add_attrs)
> +{
> +  tree type = *node;
> +
> +  /* Test the position argument  */
> +  tree pos = TREE_VALUE (args);
> +  if (!positional_argument (type, name, pos, INTEGER_TYPE, 0))
> +    *no_add_attrs = true;
> +
> +  /* Make sure that range args are INTEGRALs  */
> +  bool range_err = false;
> +  for (tree range = TREE_CHAIN (args); range; range = TREE_CHAIN (range))
> +    {
> +      tree val = TREE_VALUE (range);
> +      if (val && TREE_CODE (val) != IDENTIFIER_NODE
> +         && TREE_CODE (val) != FUNCTION_DECL)
> +       val = default_conversion (val);
> +
> +      if (TREE_CODE (val) != INTEGER_CST
> +         || !INTEGRAL_TYPE_P (TREE_TYPE (val)))
Um, why the check for INTEGRAL_TYPE_P here ?
IIUC, this will also accept non-constant integer values.
For eg, the following compiles without any warning:
int a;
int b;

void foo(int d) __attribute__ ((inrange (1, a, b)));
void foo(int d) {
  __builtin_printf("%d\n", d == 0);
}

Is this intended ?
> +       {
> +         warning (OPT_Wattributes,
> +                  "range value is not an integral constant");
> +         *no_add_attrs = true;
> +         range_err = true;
> +       }
> +    }
> +
> +  /* Test the range arg max is not smaller than min
> +     if range args are integrals  */
> +  if (!range_err)
> +    {
> +      tree range = TREE_CHAIN (args);
> +      tree min = TREE_VALUE(range);
> +      range = TREE_CHAIN (range);
> +      tree max = TREE_VALUE(range);
> +      if (!tree_int_cst_le (min, max))
> +       {
> +         warning (OPT_Wattributes,
> +                  "min range is bigger than max range");
> +         *no_add_attrs = true;
> +         return NULL_TREE;
> +       }
> +    }
> +
> +  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..8936942fec8 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -5342,6 +5342,51 @@ check_function_nonnull (location_t loc, tree attrs, int nargs, tree *argarray)
>    return ctx.warned_p;
>  }
>
> +
> +/* Check the argument list of a function call for invalid range
> +   in argument slots that are marked as requiring a specific range.
> +   Return true if we have warned. */
> +
> +static bool
> +check_function_inrange (location_t loc, tree attrs, tree *argarray)
> +{
> +  tree a;
> +  tree param;
> +  int pos;
> +  HOST_WIDE_INT min;
> +  HOST_WIDE_INT max;
> +  HOST_WIDE_INT value;
> +  bool warned = false;
> +
> +  attrs = lookup_attribute ("inrange", attrs);
> +  if (attrs == NULL_TREE)
> +    return false;
> +
> +  /* Walk through attributes and check range values
> +     when range attribute is found  */
> +
> +  for (a = attrs; a ; a = TREE_CHAIN (a))
> +    {
> +      a = lookup_attribute ("inrange", a);
> +      tree op = TREE_VALUE (a);
> +      pos = TREE_INT_CST_LOW (TREE_VALUE (op));
> +      op = TREE_CHAIN (op);
> +      min = tree_to_shwi (TREE_VALUE (op));
> +      op = TREE_CHAIN (op);
> +      max = tree_to_shwi (TREE_VALUE (op));
> +      param = argarray[pos - 1];
> +      value = TREE_INT_CST_LOW (param);
> +      if (value < min || value > max)
> +       {
> +           warning_at (loc, OPT_Winrange, "argument in position %u"
> +                       " not in rage of %ld..%ld", pos, min, max);
> +           warned = true;
> +       }
> +    }
> +
> +  return warned;
> +}
> +
>  /* 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.  */
> @@ -5703,7 +5748,7 @@ attribute_fallthrough_p (tree attr)
>
>  /* 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 -Winrange, -Wnonnull or -Wrestrict has
>     been issued.
>
>     The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
> @@ -5723,6 +5768,10 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
>      warned_p = check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype),
>                                        nargs, argarray);
>
> +  if (warn_inrange)
> +    warned_p = check_function_inrange (loc, TYPE_ATTRIBUTES (fntype),
> +                                      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..0b9aa597c54 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -932,6 +932,14 @@ Wnonnull-compare
>  C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
>  ;
>
> +Winrange
> +C Var(warn_inrange) Warning LangEnabledBy(C,Wformat=,warn_format >= 1,0)
> +Warn about integer not being in specified range.
> +
> +Winrange
> +C LangEnabledBy(C,Wall)
Just curious, why only C ?

Thanks,
Prathamesh
> +;
> +
>  Wnormalized
>  C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
>  ;
>
>
>

[RFC] Support for nonzero attribute

[Miika]

Thank you for the feedback!

On Sunday, June 12th, 2022 at 7:25 AM, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote:
> On Mon, 6 Jun 2022 at 01:39, Miika via Gcc gcc@gcc.gnu.org wrote:
>
> > Based on Jakub's and Yair's comments I created a new attribute "inrange".
> > Inrage takes three arguments, pos min and max.
> > Pos being the argument position in the function, and min and max defines the
> > range of valid integer. Both min and max are inclusive and work with enums.
> > Warnings are enabled with the new flag: "-Winrange".
> >
> > The attribute definition would look something like this:
> > inrange(pos, min, max)
> >
> > So for example compiling this with "gcc foo.c -Winrange":
> >
> > #include <stdio.h>
> > void foo(int d) attribute ((inrange (1, 100, 200)));
> > void foo(int d) {
> > printf("%d\n", d == 0);
> > }
> >
> > int main() {
> > foo(0); // warning
> > foo(100); // no warning
> > }
> >
> > Would give the following error:
> >
> > foo.c: In function 'main':
> > foo.c:8:9: warning: argument in position 1 not in rage of 100..200 [-Winrange]
> > 8 | foo(0); // warning
> > | ^~~
> >
> > I thought about having separate minval and maxval attributes but I personally
> > prefer that min and max values have to be defined explicitly.
> >
> > If this looks good, I could look into applying inrange to types and variables
> > and after that I could start looking into optimization.
> >
> > Patch for adding inrange is attached below
>
> Hi,
> Thanks for the POC patch!
> A few suggestions:
>
> (1) It doesn't apply as-is because of transition from .c to .cc filenames.
> Perhaps you're using an older version of trunk ?

I was using an older version. I should've created the patch based on the
master branch. My bad!

> (2) AFAIK, this warning will need an entry in doc/invoke.texi for
> documenting it.

Good point. I'll write up some documentation.

> (3) While this patch addresses warning, I suppose it could be extended
> so the tree optimizer
> can take advantage of value range info provided by the attribute.
> For example, the condition d > 20, could be optimized away in
>
> following function by inferring
> range from the attribute.
>
> attribute((inrange (1, 10, 20)))
> void foo(int d)
> {
> if (d > 20)
>
> __builtin_abort ();
> }

I agree. I'll try to add this too.

> > Miika
> >
> > ---
> > diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
> > index 3239311b5a4..2f5732b3ed2 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_INRANGE, "inrange")
> > 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..d6dc9c37723 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,
> > bool *);
> > static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
> > +static tree handle_inrange_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[] =
> > handle_tls_model_attribute, NULL },
> > { "nonnull", 0, -1, false, true, true, false,
> > handle_nonnull_attribute, NULL },
> > + { "inrange", 3, 3, false, true, true, false,
> > + handle_inrange_attribute, NULL },
> > { "nonstring", 0, 0, true, false, false, false,
> > handle_nonstring_attribute, NULL },
> > { "nothrow", 0, 0, true, false, false, false,
> > @@ -3754,6 +3757,59 @@ handle_nonnull_attribute (tree *node, tree name,
> > return NULL_TREE;
> > }
> >
> > +/* Handle the "inrange" attribute. */
> > +
> > +static tree
> > +handle_inrange_attribute (tree *node, tree name,
> > + tree args, int ARG_UNUSED (flags),
> > + bool *no_add_attrs)
> > +{
> > + tree type = node;
> > +
> > + / Test the position argument */
> > + tree pos = TREE_VALUE (args);
> > + if (!positional_argument (type, name, pos, INTEGER_TYPE, 0))
> > + no_add_attrs = true;
> > +
> > + / Make sure that range args are INTEGRALs */
> > + bool range_err = false;
> > + for (tree range = TREE_CHAIN (args); range; range = TREE_CHAIN (range))
> > + {
> > + tree val = TREE_VALUE (range);
> > + if (val && TREE_CODE (val) != IDENTIFIER_NODE
> > + && TREE_CODE (val) != FUNCTION_DECL)
> > + val = default_conversion (val);
> > +
> > + if (TREE_CODE (val) != INTEGER_CST
> > + || !INTEGRAL_TYPE_P (TREE_TYPE (val)))
>
> Um, why the check for INTEGRAL_TYPE_P here ?
> IIUC, this will also accept non-constant integer values.
> For eg, the following compiles without any warning:
> int a;
> int b;
>
> void foo(int d) attribute ((inrange (1, a, b)));
> void foo(int d) {
> __builtin_printf("%d\n", d == 0);
> }
>
> Is this intended ?

This was intended behavior but now that I think about it,
it's probably best to just use constant integers. Good catch!

> > + {
> > + warning (OPT_Wattributes,
> > + "range value is not an integral constant");
> > + no_add_attrs = true;
> > + range_err = true;
> > + }
> > + }
> > +
> > + / Test the range arg max is not smaller than min
> > + if range args are integrals */
> > + if (!range_err)
> > + {
> > + tree range = TREE_CHAIN (args);
> > + tree min = TREE_VALUE(range);
> > + range = TREE_CHAIN (range);
> > + tree max = TREE_VALUE(range);
> > + if (!tree_int_cst_le (min, max))
> > + {
> > + warning (OPT_Wattributes,
> > + "min range is bigger than max range");
> > + no_add_attrs = true;
> > + return NULL_TREE;
> > + }
> > + }
> > +
> > + 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..8936942fec8 100644
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -5342,6 +5342,51 @@ check_function_nonnull (location_t loc, tree attrs, int nargs, tree *argarray)
> > return ctx.warned_p;
> > }
> >
> > +
> > +/* Check the argument list of a function call for invalid range
> > + in argument slots that are marked as requiring a specific range.
> > + Return true if we have warned. */
> > +
> > +static bool
> > +check_function_inrange (location_t loc, tree attrs, tree argarray)
> > +{
> > + tree a;
> > + tree param;
> > + int pos;
> > + HOST_WIDE_INT min;
> > + HOST_WIDE_INT max;
> > + HOST_WIDE_INT value;
> > + bool warned = false;
> > +
> > + attrs = lookup_attribute ("inrange", attrs);
> > + if (attrs == NULL_TREE)
> > + return false;
> > +
> > + / Walk through attributes and check range values
> > + when range attribute is found /
> > +
> > + for (a = attrs; a ; a = TREE_CHAIN (a))
> > + {
> > + a = lookup_attribute ("inrange", a);
> > + tree op = TREE_VALUE (a);
> > + pos = TREE_INT_CST_LOW (TREE_VALUE (op));
> > + op = TREE_CHAIN (op);
> > + min = tree_to_shwi (TREE_VALUE (op));
> > + op = TREE_CHAIN (op);
> > + max = tree_to_shwi (TREE_VALUE (op));
> > + param = argarray[pos - 1];
> > + value = TREE_INT_CST_LOW (param);
> > + if (value < min || value > max)
> > + {
> > + warning_at (loc, OPT_Winrange, "argument in position %u"
> > + " not in rage of %ld..%ld", pos, min, max);
> > + warned = true;
> > + }
> > + }
> > +
> > + return warned;
> > +}
> > +
> > / 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. */
> > @@ -5703,7 +5748,7 @@ attribute_fallthrough_p (tree attr)
> >
> > /* 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 -Winrange, -Wnonnull or -Wrestrict has
> > been issued.
> >
> > The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
> > @@ -5723,6 +5768,10 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
> > warned_p = check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype),
> > nargs, argarray);
> >
> > + if (warn_inrange)
> > + warned_p = check_function_inrange (loc, TYPE_ATTRIBUTES (fntype),
> > + 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..0b9aa597c54 100644
> > --- a/gcc/c-family/c.opt
> > +++ b/gcc/c-family/c.opt
> > @@ -932,6 +932,14 @@ Wnonnull-compare
> > C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
> > ;
> >
> > +Winrange
> > +C Var(warn_inrange) Warning LangEnabledBy(C,Wformat=,warn_format >= 1,0)
> > +Warn about integer not being in specified range.
> > +
> > +Winrange
> > +C LangEnabledBy(C,Wall)
>
> Just curious, why only C ?

I wasn't sure how much extra work it would've been to support ObjC C++ and ObjC++
so I introduced the concept only with C support. Now that I know that this
is a wanted feature, I'll add support for the other languages too.

> Thanks,
> Prathamesh
>
> > +;
> > +
> > Wnormalized
> > C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
> > ;

Re: [RFC] Support for nonzero attribute

[Martin Sebor]

On 6/13/22 06:55, Miika via Gcc wrote:
> Thank you for the feedback!
> 
> On Sunday, June 12th, 2022 at 7:25 AM, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote:
>> On Mon, 6 Jun 2022 at 01:39, Miika via Gcc gcc@gcc.gnu.org wrote:
>>
>>> Based on Jakub's and Yair's comments I created a new attribute "inrange".
>>> Inrage takes three arguments, pos min and max.
>>> Pos being the argument position in the function, and min and max defines the
>>> range of valid integer. Both min and max are inclusive and work with enums.
>>> Warnings are enabled with the new flag: "-Winrange".
>>>
>>> The attribute definition would look something like this:
>>> inrange(pos, min, max)
>>>
>>> So for example compiling this with "gcc foo.c -Winrange":
>>>
>>> #include <stdio.h>
>>> void foo(int d) attribute ((inrange (1, 100, 200)));
>>> void foo(int d) {
>>> printf("%d\n", d == 0);
>>> }
>>>
>>> int main() {
>>> foo(0); // warning
>>> foo(100); // no warning
>>> }
>>>
>>> Would give the following error:
>>>
>>> foo.c: In function 'main':
>>> foo.c:8:9: warning: argument in position 1 not in rage of 100..200 [-Winrange]
>>> 8 | foo(0); // warning
>>> | ^~~
>>>
>>> I thought about having separate minval and maxval attributes but I personally
>>> prefer that min and max values have to be defined explicitly.
>>>
>>> If this looks good, I could look into applying inrange to types and variables
>>> and after that I could start looking into optimization.
>>>
>>> Patch for adding inrange is attached below
>>
>> Hi,
>> Thanks for the POC patch!
>> A few suggestions:
>>
>> (1) It doesn't apply as-is because of transition from .c to .cc filenames.
>> Perhaps you're using an older version of trunk ?
> 
> I was using an older version. I should've created the patch based on the
> master branch. My bad!
> 
>> (2) AFAIK, this warning will need an entry in doc/invoke.texi for
>> documenting it.
> 
> Good point. I'll write up some documentation.
> 
>> (3) While this patch addresses warning, I suppose it could be extended
>> so the tree optimizer
>> can take advantage of value range info provided by the attribute.
>> For example, the condition d > 20, could be optimized away in
>>
>> following function by inferring
>> range from the attribute.
>>
>> attribute((inrange (1, 10, 20)))
>> void foo(int d)
>> {
>> if (d > 20)
>>
>> __builtin_abort ();
>> }
> 
> I agree. I'll try to add this too.
> 
>>> Miika
>>>
>>> ---
>>> diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
>>> index 3239311b5a4..2f5732b3ed2 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_INRANGE, "inrange")
>>> 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..d6dc9c37723 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,
>>> bool *);
>>> static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
>>> +static tree handle_inrange_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[] =
>>> handle_tls_model_attribute, NULL },
>>> { "nonnull", 0, -1, false, true, true, false,
>>> handle_nonnull_attribute, NULL },
>>> + { "inrange", 3, 3, false, true, true, false,
>>> + handle_inrange_attribute, NULL },
>>> { "nonstring", 0, 0, true, false, false, false,
>>> handle_nonstring_attribute, NULL },
>>> { "nothrow", 0, 0, true, false, false, false,
>>> @@ -3754,6 +3757,59 @@ handle_nonnull_attribute (tree *node, tree name,
>>> return NULL_TREE;
>>> }
>>>
>>> +/* Handle the "inrange" attribute. */
>>> +
>>> +static tree
>>> +handle_inrange_attribute (tree *node, tree name,
>>> + tree args, int ARG_UNUSED (flags),
>>> + bool *no_add_attrs)
>>> +{
>>> + tree type = node;
>>> +
>>> + / Test the position argument */
>>> + tree pos = TREE_VALUE (args);
>>> + if (!positional_argument (type, name, pos, INTEGER_TYPE, 0))
>>> + no_add_attrs = true;
>>> +
>>> + / Make sure that range args are INTEGRALs */
>>> + bool range_err = false;
>>> + for (tree range = TREE_CHAIN (args); range; range = TREE_CHAIN (range))
>>> + {
>>> + tree val = TREE_VALUE (range);
>>> + if (val && TREE_CODE (val) != IDENTIFIER_NODE
>>> + && TREE_CODE (val) != FUNCTION_DECL)
>>> + val = default_conversion (val);
>>> +
>>> + if (TREE_CODE (val) != INTEGER_CST
>>> + || !INTEGRAL_TYPE_P (TREE_TYPE (val)))
>>
>> Um, why the check for INTEGRAL_TYPE_P here ?
>> IIUC, this will also accept non-constant integer values.
>> For eg, the following compiles without any warning:
>> int a;
>> int b;
>>
>> void foo(int d) attribute ((inrange (1, a, b)));
>> void foo(int d) {
>> __builtin_printf("%d\n", d == 0);
>> }
>>
>> Is this intended ?
> 
> This was intended behavior but now that I think about it,
> it's probably best to just use constant integers. Good catch!
> 
>>> + {
>>> + warning (OPT_Wattributes,
>>> + "range value is not an integral constant");
>>> + no_add_attrs = true;
>>> + range_err = true;
>>> + }
>>> + }
>>> +
>>> + / Test the range arg max is not smaller than min
>>> + if range args are integrals */
>>> + if (!range_err)
>>> + {
>>> + tree range = TREE_CHAIN (args);
>>> + tree min = TREE_VALUE(range);
>>> + range = TREE_CHAIN (range);
>>> + tree max = TREE_VALUE(range);
>>> + if (!tree_int_cst_le (min, max))
>>> + {
>>> + warning (OPT_Wattributes,
>>> + "min range is bigger than max range");
>>> + no_add_attrs = true;
>>> + return NULL_TREE;
>>> + }
>>> + }
>>> +
>>> + 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..8936942fec8 100644
>>> --- a/gcc/c-family/c-common.c
>>> +++ b/gcc/c-family/c-common.c
>>> @@ -5342,6 +5342,51 @@ check_function_nonnull (location_t loc, tree attrs, int nargs, tree *argarray)
>>> return ctx.warned_p;
>>> }
>>>
>>> +
>>> +/* Check the argument list of a function call for invalid range
>>> + in argument slots that are marked as requiring a specific range.
>>> + Return true if we have warned. */
>>> +
>>> +static bool
>>> +check_function_inrange (location_t loc, tree attrs, tree argarray)
>>> +{
>>> + tree a;
>>> + tree param;
>>> + int pos;
>>> + HOST_WIDE_INT min;
>>> + HOST_WIDE_INT max;
>>> + HOST_WIDE_INT value;
>>> + bool warned = false;
>>> +
>>> + attrs = lookup_attribute ("inrange", attrs);
>>> + if (attrs == NULL_TREE)
>>> + return false;
>>> +
>>> + / Walk through attributes and check range values
>>> + when range attribute is found /
>>> +
>>> + for (a = attrs; a ; a = TREE_CHAIN (a))
>>> + {
>>> + a = lookup_attribute ("inrange", a);
>>> + tree op = TREE_VALUE (a);
>>> + pos = TREE_INT_CST_LOW (TREE_VALUE (op));
>>> + op = TREE_CHAIN (op);
>>> + min = tree_to_shwi (TREE_VALUE (op));
>>> + op = TREE_CHAIN (op);
>>> + max = tree_to_shwi (TREE_VALUE (op));
>>> + param = argarray[pos - 1];
>>> + value = TREE_INT_CST_LOW (param);
>>> + if (value < min || value > max)
>>> + {
>>> + warning_at (loc, OPT_Winrange, "argument in position %u"
>>> + " not in rage of %ld..%ld", pos, min, max);
>>> + warned = true;
>>> + }
>>> + }
>>> +
>>> + return warned;
>>> +}
>>> +
>>> / 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. */
>>> @@ -5703,7 +5748,7 @@ attribute_fallthrough_p (tree attr)
>>>
>>> /* 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 -Winrange, -Wnonnull or -Wrestrict has
>>> been issued.
>>>
>>> The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
>>> @@ -5723,6 +5768,10 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
>>> warned_p = check_function_nonnull (loc, TYPE_ATTRIBUTES (fntype),
>>> nargs, argarray);
>>>
>>> + if (warn_inrange)
>>> + warned_p = check_function_inrange (loc, TYPE_ATTRIBUTES (fntype),
>>> + 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..0b9aa597c54 100644
>>> --- a/gcc/c-family/c.opt
>>> +++ b/gcc/c-family/c.opt
>>> @@ -932,6 +932,14 @@ Wnonnull-compare
>>> C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
>>> ;
>>>
>>> +Winrange
>>> +C Var(warn_inrange) Warning LangEnabledBy(C,Wformat=,warn_format >= 1,0)
>>> +Warn about integer not being in specified range.
>>> +
>>> +Winrange
>>> +C LangEnabledBy(C,Wall)
>>
>> Just curious, why only C ?
> 
> I wasn't sure how much extra work it would've been to support ObjC C++ and ObjC++
> so I introduced the concept only with C support. Now that I know that this
> is a wanted feature, I'll add support for the other languages too.

The attribute handling can be (and to benefit optimization needs
to be) fully implemented in the middle end.  There is no need to
change any of the front ends.

Martin


> 
>> Thanks,
>> Prathamesh
>>
>>> +;
>>> +
>>> Wnormalized
>>> C ObjC C++ ObjC++ Warning Alias(Wnormalized=,nfc,none)
>>> ;

[RFC] Support for nonzero attribute

[Yair Lenga]

Before becoming a "C" programmer, I spent few years building simulations in
Pascal. I still remember (and long for) the ability to define integer with
range constraints:

var foobar: 10..50 ;         // Accept 10, 11, 12, ..., 49, 50

The specific non-zero constraint is a specific implementation of the range
operator (with some exception see below). Wanted to suggest going for
more ambitious goal: add min and max attributes to (integer) types and
variables. This will address the specific case of non-zero, but has a lot
of potential to be built upon: can be used for compile time testing, run
time parameter checking, storage optimization (similar to packed), run time
optimization (e.g. eliminating runtime tests), .... Also expected range
information can have a positive impact on code safety/validation.

typedef int postivieInt __attribute__ (minValue(1), maxValue(INTMAX) ;
typedef int foobar __attribute__ ((minValue(10), maxValue(50)) ;

If this can be implemented, it will provide for much more
flexibility (e.g., ability to specify that any specific parameter must be
non-zero).

int foo (int x __attribute__ (minValue(1)), int y, int z __attribute__
(minValue(1))  ;

int foo (positiveInt x, int y, positiveInt y) ;

Assuming this can be implemented, compile time tests should be automatic,
whenever possible. Run time tests should be enabled with flags (to allow
optimized code to run without expensive run time tests).

Note1:
While for many use cases non-zero (including  forcing ENUM value, and
minValue(1) are the same, the above does not cover the user case where a
signed int does not accept a zero. For this use case, I believe the nonZero
attribute is still needed.

typedef int limitedInt __attribute((minValue(-20)), maxValue(+20), nonZero)

I do recall that few other languages had similar abilities (Ada, Java (via
annotations), ...)

Yair


>
>
>
> ---------- Forwarded message ----------
> From: Miika <nykseli@protonmail.com>
> To: "gcc@gcc.gnu.org" <gcc@gcc.gnu.org>
> Cc:
> Bcc:
> Date: Fri, 03 Jun 2022 16:34:48 +0000
> Subject: [RFC] Support for nonzero attribute
> 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 <stdio.h>
> 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 == 0);
>         printf("%d\n", b == 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)
> [-Wnonzero]
>    11 |         foo(0, NONE);
>       |         ^~~
> ...

[RFC] Support for nonzero attribute

[Miika]

On Saturday, June 4th, 2022 at 1:26 PM, Yair Lenga via Gcc <gcc@gcc.gnu.org> wrote:
> The specific non-zero constraint is a specific implementation of the range
> operator (with some exception see below). Wanted to suggest going for
> more ambitious goal: add min and max attributes to (integer) types and
> variables. This will address the specific case of non-zero, but has a lot
> of potential to be built upon: can be used for compile time testing, run
> time parameter checking, storage optimization (similar to packed), run time
> optimization (e.g. eliminating runtime tests), .... Also expected range
> information can have a positive impact on code safety/validation.


I like this idea a lot too. I'll definitely look into adding a "range"
variable attribute after the work with function attributes is done.
I'm not that familiar with GCC's optimizer but basic compiler warnings
should be fairly easy to implement.

Miika

Re: [RFC] Support for nonzero attribute

[Yair Lenga]

Static checks will be a good starting point!

On Sat, Jun 4, 2022 at 7:55 AM Miika <nykseli@protonmail.com> wrote:

> On Saturday, June 4th, 2022 at 1:26 PM, Yair Lenga via Gcc <
> gcc@gcc.gnu.org> wrote:
> > The specific non-zero constraint is a specific implementation of the
> range
> > operator (with some exception see below). Wanted to suggest going for
> > more ambitious goal: add min and max attributes to (integer) types and
> > variables. This will address the specific case of non-zero, but has a lot
> > of potential to be built upon: can be used for compile time testing, run
> > time parameter checking, storage optimization (similar to packed), run
> time
> > optimization (e.g. eliminating runtime tests), .... Also expected range
> > information can have a positive impact on code safety/validation.
>
>
> I like this idea a lot too. I'll definitely look into adding a "range"
> variable attribute after the work with function attributes is done.
> I'm not that familiar with GCC's optimizer but basic compiler warnings
> should be fairly easy to implement.
>
> Miika
>

Re: [RFC] Support for nonzero attribute

[Richard Biener]

On Sat, Jun 4, 2022 at 12:27 PM Yair Lenga via Gcc <gcc@gcc.gnu.org> wrote:
>
> Before becoming a "C" programmer, I spent few years building simulations in
> Pascal. I still remember (and long for) the ability to define integer with
> range constraints:
>
> var foobar: 10..50 ;         // Accept 10, 11, 12, ..., 49, 50

Just noting this is a range on a variable declaration while ...

> The specific non-zero constraint is a specific implementation of the range
> operator (with some exception see below). Wanted to suggest going for
> more ambitious goal: add min and max attributes to (integer) types and
> variables. This will address the specific case of non-zero, but has a lot
> of potential to be built upon: can be used for compile time testing, run
> time parameter checking, storage optimization (similar to packed), run time
> optimization (e.g. eliminating runtime tests), .... Also expected range
> information can have a positive impact on code safety/validation.
>
> typedef int postivieInt __attribute__ (minValue(1), maxValue(INTMAX) ;
> typedef int foobar __attribute__ ((minValue(10), maxValue(50)) ;

... this would be on a type.  GCC internally has TYPE_{MIN,MAX}_VALUE
but no such thing on declarations which means that either the
attribute should be restricted to types or it would need to create distinct
types on-the-fly when applied to declarations.  I'm sure Ada supports something
similar btw.

Richard.

> If this can be implemented, it will provide for much more
> flexibility (e.g., ability to specify that any specific parameter must be
> non-zero).
>
> int foo (int x __attribute__ (minValue(1)), int y, int z __attribute__
> (minValue(1))  ;
>
> int foo (positiveInt x, int y, positiveInt y) ;
>
> Assuming this can be implemented, compile time tests should be automatic,
> whenever possible. Run time tests should be enabled with flags (to allow
> optimized code to run without expensive run time tests).
>
> Note1:
> While for many use cases non-zero (including  forcing ENUM value, and
> minValue(1) are the same, the above does not cover the user case where a
> signed int does not accept a zero. For this use case, I believe the nonZero
> attribute is still needed.
>
> typedef int limitedInt __attribute((minValue(-20)), maxValue(+20), nonZero)
>
> I do recall that few other languages had similar abilities (Ada, Java (via
> annotations), ...)
>
> Yair
>
>
> >
> >
> >
> > ---------- Forwarded message ----------
> > From: Miika <nykseli@protonmail.com>
> > To: "gcc@gcc.gnu.org" <gcc@gcc.gnu.org>
> > Cc:
> > Bcc:
> > Date: Fri, 03 Jun 2022 16:34:48 +0000
> > Subject: [RFC] Support for nonzero attribute
> > 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 <stdio.h>
> > 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 == 0);
> >         printf("%d\n", b == 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)
> > [-Wnonzero]
> >    11 |         foo(0, NONE);
> >       |         ^~~
> > ...