[PATCH] add option to emit more array bounds warnigs

[PATCH] add option to emit more array bounds warnigs

[Martin Uecker]

Hi,

this proposed patch adds an option "-Warray-bounds=" in addition to
"-Warray-bound". "-Warray-bounds=1" corresponds to "-Warray-bound".
For higher warning levels more warnings about optional accesses
outside of arrays are emitted. For example, warnings for
arrays accessed through pointers are now emitted:

void foo(int (*a)[3])
{
	(*a)[4] = 1;
}

Also warnings for arrays which are the last element of a struct
are emitted, if it is not a flexible array member or does not use
the zero size extensions.

Because there is the risk of false positives, the higher warning
level is not used by default.


Martin


* gcc/tree-vrp.c (check_array_ref): Emit more warnings
       for warn_array_bounds >= 2.
* gcc/testsuite/gcc.dg/Warray-bounds-11.c: New test-case.
* gcc/c-family/c.opt: New option -Warray-bounds=.
* gcc/common.opt: New option -Warray-bounds=.
* gcc/doc/invoke.texi: Document new option.


diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 66c62fb..0d5ecfd 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -279,6 +279,10 @@ Warray-bounds
 LangEnabledBy(C ObjC C++ ObjC++,Wall)
 ; in common.opt
 
+Warray-bounds=
+LangEnabledBy(C ObjC C++ ObjC++,Wall,1,0)
+; in common.opt
+
 Wassign-intercept
 ObjC ObjC++ Var(warn_assign_intercept) Warning
 Warn whenever an Objective-C assignment is being intercepted by the garbage collector
diff --git a/gcc/common.opt b/gcc/common.opt
index b400636..d65085a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -525,6 +525,10 @@ Warray-bounds
 Common Var(warn_array_bounds) Warning
 Warn if an array is accessed out of bounds
 
+Warray-bounds=
+Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1) Warning
 Warn about inappropriate attribute usage
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 57666db..afc4be8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -239,7 +239,7 @@ Objective-C and Objective-C++ Dialects}.
 @gccoptlist{-fsyntax-only  -fmax-errors=@var{n}  -Wpedantic @gol
 -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
--Waggressive-loop-optimizations -Warray-bounds @gol
+-Waggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
 -Wbool-compare @gol
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc90-c99-compat -Wc99-c11-compat @gol
@@ -3344,7 +3344,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 @option{-Wall} turns on the following warning flags:
 
 @gccoptlist{-Waddress   @gol
--Warray-bounds @r{(only with} @option{-O2}@r{)}  @gol
+-Warray-bounds=1 @r{(only with} @option{-O2}@r{)}  @gol
 -Wc++11-compat  @gol
 -Wchar-subscripts  @gol
 -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
@@ -4239,12 +4239,26 @@ hiearchy graph is more complete. It is recommended to first consider suggestins
 of @option{-Wsuggest-final-types} and then rebuild with new annotations.
 
 @item -Warray-bounds
+@itemx -Warray-bounds=@var{n}
 @opindex Wno-array-bounds
 @opindex Warray-bounds
 This option is only active when @option{-ftree-vrp} is active
 (default for @option{-O2} and above). It warns about subscripts to arrays
 that are always out of bounds. This warning is enabled by @option{-Wall}.
 
+@table @gcctabopt
+@item -Warray-bounds=1
+This is the warning level of @option{-Warray-bounds} and is enabled
+by @option{-Wall}; higher levels are not, and must be explicitly requested.
+
+@item -Warray-bounds=2
+This warning level also warns about out of bounds access for
+arrays at the end of a struct and for arrays accessed through
+pointers. This warning level may give a larger number of
+false positives and is deactivated by default.
+@end table
+
+
 @item -Wbool-compare
 @opindex Wno-bool-compare
 @opindex Wbool-compare
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-11.c b/gcc/testsuite/gcc.dg/Warray-bounds-11.c
new file mode 100644
index 0000000..2e68498
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-11.c
@@ -0,0 +1,96 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -Warray-bounds=2" } */
+
+extern void* malloc(unsigned long x);
+
+int e[3];
+
+struct f { int f[3]; };
+
+extern void bar(int v[]);
+
+struct h {
+
+	int i;
+	int j[];
+};
+
+struct h0 {
+
+	int i;
+	int j[0];
+};
+
+struct h0b {
+
+	int i;
+	int j[0];
+	int k;
+};
+
+struct h1 {
+
+	int i;
+	int j[1];
+};
+
+struct h1b {
+
+	int i;
+	int j[1];
+	int k;
+};
+
+struct h3 {
+
+	int i;
+	int j[3];
+};
+
+struct h3b {
+
+	int i;
+	int j[3];
+	int k;
+};
+
+void foo(int (*a)[3])
+{
+	(*a)[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+	a[0][0] = 1;	// ok
+	a[1][0] = 1;	// ok
+	a[1][4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	int c[3] = { 0 };
+
+	c[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	e[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	struct f f;
+	f.f[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	struct h* h = malloc(sizeof(struct h) + 3 * sizeof(int));
+	struct h0* h0 = malloc(sizeof(struct h0) + 3 * sizeof(int));
+	struct h1* h1 = malloc(sizeof(struct h1) + 3 * sizeof(int));
+	struct h3* h3 = malloc(sizeof(struct h3));
+
+	h->j[4] = 1;	// flexible array member
+	h0->j[4] = 1;	// zero-sized array extension
+	h1->j[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+	h3->j[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	struct h0b* h0b = malloc(sizeof(struct h) + 3 * sizeof(int));
+	struct h1b* h1b = malloc(sizeof(struct h1b) + 3 * sizeof(int));
+	struct h3b* h3b = malloc(sizeof(struct h3b));
+//	h0b->j[4] = 1;
+	h1b->j[4] = 1;;	/* { dg-warning "subscript is above array bound" } */
+	h3b->j[4] = 1;;	/* { dg-warning "subscript is above array bound" } */
+
+	// make sure nothing gets optimized away
+	bar(*a);
+	bar(c);
+	bar(e);
+	bar(f.f);
+}
+
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 4e4ebe0..7a4bb9d 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -6356,7 +6356,8 @@ check_array_ref (location_t location, tree ref, bool ignore_off_by_one)
   /* Accesses to trailing arrays via pointers may access storage
      beyond the types array bounds.  */
   base = get_base_address (ref);
-  if (base && TREE_CODE (base) == MEM_REF)
+  if ((warn_array_bounds < 2)
+      && base && TREE_CODE (base) == MEM_REF)
     {
       tree cref, next = NULL_TREE;
 

[PING] Re: [PATCH] add option to emit more array bounds warnigs

[Martin Uecker]

Please consider this patch. The additional warnings would be useful 
IMHO, are also emitted by clang, and the change seems trivial.

Previous discussion about potential false positives:
https://gcc.gnu.org/ml/gcc/2014-11/msg00114.html


Tue, 11 Nov 2014 22:13:20 -0800
Martin Uecker <uecker@eecs.berkeley.edu>:

> 
> Hi,
> 
> this proposed patch adds an option "-Warray-bounds=" in addition to
> "-Warray-bound". "-Warray-bounds=1" corresponds to "-Warray-bound".
> For higher warning levels more warnings about optional accesses
> outside of arrays are emitted. For example, warnings for
> arrays accessed through pointers are now emitted:
> 
> void foo(int (*a)[3])
> {
> 	(*a)[4] = 1;
> }
> 
> Also warnings for arrays which are the last element of a struct
> are emitted, if it is not a flexible array member or does not use
> the zero size extensions.
> 
> Because there is the risk of false positives, the higher warning
> level is not used by default.
> 
> 
> Martin

 

* gcc/tree-vrp.c (check_array_ref): Emit more warnings
       for warn_array_bounds >= 2.
* gcc/testsuite/gcc.dg/Warray-bounds-11.c: New test-case.
* gcc/c-family/c.opt: New option -Warray-bounds=.
* gcc/common.opt: New option -Warray-bounds=.
* gcc/doc/invoke.texi: Document new option.



diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 064c69e..e61fc56 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -279,6 +279,10 @@ Warray-bounds
 LangEnabledBy(C ObjC C++ ObjC++,Wall)
 ; in common.opt
 
+Warray-bounds=
+LangEnabledBy(C ObjC C++ ObjC++,Wall,1,0)
+; in common.opt
+
 Wassign-intercept
 ObjC ObjC++ Var(warn_assign_intercept) Warning
 Warn whenever an Objective-C assignment is being intercepted by the garbage collector
diff --git a/gcc/common.opt b/gcc/common.opt
index e104269..3d19875 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -529,6 +529,10 @@ Warray-bounds
 Common Var(warn_array_bounds) Warning
 Warn if an array is accessed out of bounds
 
+Warray-bounds=
+Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1) Warning
 Warn about inappropriate attribute usage
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ed23f6f..f20cbf0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -240,7 +240,7 @@ Objective-C and Objective-C++ Dialects}.
 @gccoptlist{-fsyntax-only  -fmax-errors=@var{n}  -Wpedantic @gol
 -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
--Waggressive-loop-optimizations -Warray-bounds @gol
+-Waggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
 -Wbool-compare @gol
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc90-c99-compat -Wc99-c11-compat @gol
@@ -3382,7 +3382,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 @option{-Wall} turns on the following warning flags:
 
 @gccoptlist{-Waddress   @gol
--Warray-bounds @r{(only with} @option{-O2}@r{)}  @gol
+-Warray-bounds=1 @r{(only with} @option{-O2}@r{)}  @gol
 -Wc++11-compat  -Wc++14-compat@gol
 -Wchar-subscripts  @gol
 -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
@@ -4296,12 +4296,26 @@ Warn about overriding virtual functions that are not marked with the override
 keyword.
 
 @item -Warray-bounds
+@itemx -Warray-bounds=@var{n}
 @opindex Wno-array-bounds
 @opindex Warray-bounds
 This option is only active when @option{-ftree-vrp} is active
 (default for @option{-O2} and above). It warns about subscripts to arrays
 that are always out of bounds. This warning is enabled by @option{-Wall}.
 
+@table @gcctabopt
+@item -Warray-bounds=1
+This is the warning level of @option{-Warray-bounds} and is enabled
+by @option{-Wall}; higher levels are not, and must be explicitly requested.
+
+@item -Warray-bounds=2
+This warning level also warns about out of bounds access for
+arrays at the end of a struct and for arrays accessed through
+pointers. This warning level may give a larger number of
+false positives and is deactivated by default.
+@end table
+
+
 @item -Wbool-compare
 @opindex Wno-bool-compare
 @opindex Wbool-compare
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-11.c b/gcc/testsuite/gcc.dg/Warray-bounds-11.c
new file mode 100644
index 0000000..2e68498
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-11.c
@@ -0,0 +1,96 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -Warray-bounds=2" } */
+
+extern void* malloc(unsigned long x);
+
+int e[3];
+
+struct f { int f[3]; };
+
+extern void bar(int v[]);
+
+struct h {
+
+	int i;
+	int j[];
+};
+
+struct h0 {
+
+	int i;
+	int j[0];
+};
+
+struct h0b {
+
+	int i;
+	int j[0];
+	int k;
+};
+
+struct h1 {
+
+	int i;
+	int j[1];
+};
+
+struct h1b {
+
+	int i;
+	int j[1];
+	int k;
+};
+
+struct h3 {
+
+	int i;
+	int j[3];
+};
+
+struct h3b {
+
+	int i;
+	int j[3];
+	int k;
+};
+
+void foo(int (*a)[3])
+{
+	(*a)[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+	a[0][0] = 1;	// ok
+	a[1][0] = 1;	// ok
+	a[1][4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	int c[3] = { 0 };
+
+	c[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	e[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	struct f f;
+	f.f[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	struct h* h = malloc(sizeof(struct h) + 3 * sizeof(int));
+	struct h0* h0 = malloc(sizeof(struct h0) + 3 * sizeof(int));
+	struct h1* h1 = malloc(sizeof(struct h1) + 3 * sizeof(int));
+	struct h3* h3 = malloc(sizeof(struct h3));
+
+	h->j[4] = 1;	// flexible array member
+	h0->j[4] = 1;	// zero-sized array extension
+	h1->j[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+	h3->j[4] = 1;	/* { dg-warning "subscript is above array bound" } */
+
+	struct h0b* h0b = malloc(sizeof(struct h) + 3 * sizeof(int));
+	struct h1b* h1b = malloc(sizeof(struct h1b) + 3 * sizeof(int));
+	struct h3b* h3b = malloc(sizeof(struct h3b));
+//	h0b->j[4] = 1;
+	h1b->j[4] = 1;;	/* { dg-warning "subscript is above array bound" } */
+	h3b->j[4] = 1;;	/* { dg-warning "subscript is above array bound" } */
+
+	// make sure nothing gets optimized away
+	bar(*a);
+	bar(c);
+	bar(e);
+	bar(f.f);
+}
+
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index ae1da46..d20b627 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -6498,7 +6498,8 @@ check_array_ref (location_t location, tree ref, bool ignore_off_by_one)
   /* Accesses to trailing arrays via pointers may access storage
      beyond the types array bounds.  */
   base = get_base_address (ref);
-  if (base && TREE_CODE (base) == MEM_REF)
+  if ((warn_array_bounds < 2)
+      && base && TREE_CODE (base) == MEM_REF)
     {
       tree cref, next = NULL_TREE;
 

Re: [PATCH] add option to emit more array bounds warnigs

[Jeff Law]

On 11/11/14 23:13, Martin Uecker wrote:
>
> Hi,
>
> this proposed patch adds an option "-Warray-bounds=" in addition to
> "-Warray-bound". "-Warray-bounds=1" corresponds to "-Warray-bound".
> For higher warning levels more warnings about optional accesses
> outside of arrays are emitted. For example, warnings for
> arrays accessed through pointers are now emitted:
>
> void foo(int (*a)[3])
> {
> 	(*a)[4] = 1;
> }
>
> Also warnings for arrays which are the last element of a struct
> are emitted, if it is not a flexible array member or does not use
> the zero size extensions.
>
> Because there is the risk of false positives, the higher warning
> level is not used by default.
>
>
> Martin
>
>
> * gcc/tree-vrp.c (check_array_ref): Emit more warnings
>         for warn_array_bounds >= 2.
> * gcc/testsuite/gcc.dg/Warray-bounds-11.c: New test-case.
> * gcc/c-family/c.opt: New option -Warray-bounds=.
> * gcc/common.opt: New option -Warray-bounds=.
> * gcc/doc/invoke.texi: Document new option.
Has this patch been bootstrapped and regression tested, if so on what 
platform.

Given the new warnings (as implemented by the patch) are not enabled by 
default, I'm inclined to approve once Martin verifies things via 
bootstrap and regression test.

jeff

Re: [PATCH] add option to emit more array bounds warnigs

[Martin Uecker]

Mon, 12 Jan 2015 11:00:44 -0700
Jeff Law <law@redhat.com>:
> On 11/11/14 23:13, Martin Uecker wrote:

...

> >
> >
> > * gcc/tree-vrp.c (check_array_ref): Emit more warnings
> >         for warn_array_bounds >= 2.
> > * gcc/testsuite/gcc.dg/Warray-bounds-11.c: New test-case.
> > * gcc/c-family/c.opt: New option -Warray-bounds=.
> > * gcc/common.opt: New option -Warray-bounds=.
> > * gcc/doc/invoke.texi: Document new option.
> Has this patch been bootstrapped and regression tested, if so on what 
> platform.

x86_64-unknown-linux-gnu

> Given the new warnings (as implemented by the patch) are not enabled by 
> default, I'm inclined to approve once Martin verifies things via 
> bootstrap and regression test.

Thank you,

Martin

 

Re: [PATCH] add option to emit more array bounds warnigs

[Jeff Law]

On 01/13/15 10:34, Martin Uecker wrote:
>
> Mon, 12 Jan 2015 11:00:44 -0700
> Jeff Law <law@redhat.com>:
>> On 11/11/14 23:13, Martin Uecker wrote:
>
> ...
>
>>>
>>>
>>> * gcc/tree-vrp.c (check_array_ref): Emit more warnings
>>>          for warn_array_bounds >= 2.
>>> * gcc/testsuite/gcc.dg/Warray-bounds-11.c: New test-case.
>>> * gcc/c-family/c.opt: New option -Warray-bounds=.
>>> * gcc/common.opt: New option -Warray-bounds=.
>>> * gcc/doc/invoke.texi: Document new option.
>> Has this patch been bootstrapped and regression tested, if so on what
>> platform.
>
> x86_64-unknown-linux-gnu
Approved.  Please install on the trunk.  Sorry about the delays.

Thanks,

Jeff

Re: [PATCH] add option to emit more array bounds warnigs

[Martin Uecker]

Jeff Law <law@redhat.com>:
> On 01/13/15 10:34, Martin Uecker wrote:
> > Mon, 12 Jan 2015 11:00:44 -0700
> > Jeff Law <law@redhat.com>:
> >> On 11/11/14 23:13, Martin Uecker wrote:

...

> >> Has this patch been bootstrapped and regression tested, if so on what
> >> platform.
> >
> > x86_64-unknown-linux-gnu
> Approved.  Please install on the trunk.  Sorry about the delays.

I don't have write access ;-(

Martin

Re: [PATCH] add option to emit more array bounds warnigs

[Jeff Law]

On 01/13/15 17:40, Martin Uecker wrote:
> Jeff Law <law@redhat.com>:
>> On 01/13/15 10:34, Martin Uecker wrote:
>>> Mon, 12 Jan 2015 11:00:44 -0700
>>> Jeff Law <law@redhat.com>:
>>>> On 11/11/14 23:13, Martin Uecker wrote:
>
> ...
>
>>>> Has this patch been bootstrapped and regression tested, if so on what
>>>> platform.
>>>
>>> x86_64-unknown-linux-gnu
>> Approved.  Please install on the trunk.  Sorry about the delays.
>
> I don't have write access ;-(
I fixed up the ChangeLog entries and installed the patch for you.

If you plan to contribute regularly, you should go ahead and apply for 
write access to the repository so that you'll be able to commit your own 
patches once they're approved.

You'll also need to make sure you have an assignment on file with the 
FSF.    That patch was pretty small (the testcase was larger than the 
patch itself, which I always like :-) so I didn't request an assignment. 
  Further submissions likely will require an assignment.

Thanks,
jeff

Re: [PATCH] add option to emit more array bounds warnigs

[Martin Uecker]

Jeff Law <law@redhat.com>:
> On 01/13/15 17:40, Martin Uecker wrote:
> > Jeff Law <law@redhat.com>:
> >> On 01/13/15 10:34, Martin Uecker wrote:
> >>> Mon, 12 Jan 2015 11:00:44 -0700
> >>> Jeff Law <law@redhat.com>:
> >>>> On 11/11/14 23:13, Martin Uecker wrote:
> >
> > ...
> >
> >>>> Has this patch been bootstrapped and regression tested, if so on what
> >>>> platform.
> >>>
> >>> x86_64-unknown-linux-gnu
> >> Approved.  Please install on the trunk.  Sorry about the delays.
> >
> > I don't have write access ;-(
> I fixed up the ChangeLog entries and installed the patch for you.

Thank you, Jeff!

> If you plan to contribute regularly, you should go ahead and apply for 
> write access to the repository so that you'll be able to commit your own 
> patches once they're approved.

I put a request in with you as sponsor (hope this is ok).

> You'll also need to make sure you have an assignment on file with the 
> FSF.    That patch was pretty small (the testcase was larger than the 
> patch itself, which I always like :-) so I didn't request an assignment. 
>   Further submissions likely will require an assignment.

I already have an assignment on file.

Martin

Re: [PATCH] add option to emit more array bounds warnigs

[Jeff Law]

On 01/14/15 00:48, Martin Uecker wrote:
>
>> If you plan to contribute regularly, you should go ahead and apply for
>> write access to the repository so that you'll be able to commit your own
>> patches once they're approved.
>
> I put a request in with you as sponsor (hope this is ok).
Of course.

>
>> You'll also need to make sure you have an assignment on file with the
>> FSF.    That patch was pretty small (the testcase was larger than the
>> patch itself, which I always like :-) so I didn't request an assignment.
>>    Further submissions likely will require an assignment.
>
> I already have an assignment on file.
Excellent.

Jeff