[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
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 ac936d5..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_TR
[RFC] Support for nonzero attribute
Thank you for the feedback! On Friday, June 3rd, 2022 at 7:45 PM, Jakub Jelinek 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
On Saturday, June 4th, 2022 at 1:26 PM, Yair Lenga via Gcc 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
[RFC] Support for nonzero attribute
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
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 ac936d5..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)
retur
[RFC] Support for nonzero attribute
On Monday, June 6th, 2022 at 9:42 PM, Ben Boeckel
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
[RFC] Support for nonzero attribute
On Tuesday, June 7th, 2022 at 10:46 PM, Jonathan Wakely
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.boec...@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.
[RFC] Support for nonzero attribute
On Wednesday, June 8th, 2022 at 8:42 PM, Eric Gallager
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 (99));
}
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 (99));
|
Miika
Re: [RFC] Support for nonzero attribute
On Thursday, June 9th, 2022 at 7:36 AM, Eric Gallager 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
[RFC] Support for nonzero attribute
Thank you for the feedback!
On Sunday, June 12th, 2022 at 7:25 AM, Prathamesh Kulkarni
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
> > 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 ac936d5..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. */
> > +
&g
