Re: [RFC] Support for nonzero attribute

[Eric Gallager <egall@gwmail.gwu.edu>]

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)
>  ;
>
>
>