Created attachment 35698 [details] test Compiling the following code on GCC 5.1.0 produces a bogus out of bounds warning: ------- #include <inttypes.h> // Simple test file to trigger this bug // Two things noticed: // 1. commenting out const char* bar results in no warning // 2. Changing the "loop" in run_foo to a single, direct control // results in no warning // (These were done independently of each other, starting with this file) typedef struct foo { uint8_t foo_size; int buf[4]; const char* bar; } foo; const foo *get_foo(int index); static int foo_loop(const foo *myfoo) { int i; if (myfoo->foo_size < 3) return 0; for (i = 0; i < myfoo->foo_size; i++) { if (myfoo->buf[i] != 1) return 0; } return 1; } static int run_foo(void) { int i; for (i = 0; i < 1; i++) { const foo *myfoo = get_foo(i); if (foo_loop(myfoo)) return 0; } return -1; } // To suppress "unused run_foo" warning typedef struct hack { int (*func)(void); } hack; hack myhack = { .func = run_foo, }; ---------------------- %gcc -Warray-bounds -O3 -c test.c test.c: In function ‘run_foo’: test.c:22:23: warning: array subscript is above array bounds [-Warray-bounds] if (myfoo->buf[i] != 1) ----------------------- config: Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-unknown-linux-gnu/5.1.0/lto-wrapper Target: x86_64-unknown-linux-gnu Configured with: /build/gcc/src/gcc-5-20150519/configure --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-shared --enable-threads=posix --enable-libmpx --with-system-zlib --with-isl --enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu --disable-libstdcxx-pch --disable-libssp --enable-gnu-unique-object --enable-linker-build-id --enable-lto --enable-plugin --enable-install-libiberty --with-linker-hash-style=gnu --enable-gnu-indirect-function --disable-multilib --disable-werror --enable-checking=release --with-default-libstdcxx-abi=c++98 Thread model: posix gcc version 5.1.0 (GCC)
I don't think this is a false warning as you are saying the size is at least 4 which means if buf[ does not contain 1, you will access past the loop bounds.
But myfoo->foo_size is not set within the foo_loop function. For instance, myfoo->foo_size could be set outside the function boundary, and then the function could be called (say foo_size = 3 or 4). This is represented by the get_foo() function. Then, as long as the programmer guarantees that when this function is called, myfoo->foo_size is always <= 4, there is certainly no error. Furthermore, note that I put a < 3 condition as opposed to a <= 3.
Created attachment 35699 [details] testv2
Updated attachment that compiles to standalone executable (invoked with similar flags for with/without warning as before: gcc -Warray-bounds -O3 test.c, gcc -Warray-bounds test.c respectively).
Ok, so we peeled the loop like run_foo () { ... <bb 10>: _33 = myfoo_28->buf[3]; if (_33 != 1) goto <bb 13>; else goto <bb 11>; <bb 11>: _34 = (int) _27; if (_34 > 4) goto <bb 12>; else goto <bb 13>; <bb 12>: __builtin_unreachable (); _35 = myfoo_28->buf[4]; <bb 13>: # _36 = PHI <0(2), 1(3), 0(4), 1(5), 0(6), 1(7), 0(8), 1(9), 0(10), 1(11), 0(12)> if (_36 != 0) goto <bb 15>; else goto <bb 14>; <bb 14>: i_37 = 1; <bb 15>: # _2 = PHI <0(13), -1(14)> return _2; and the unreachable () remains in the CFG. Honza - it seems that remove_exits_and_undefined_stmts inserts these unreachable calls but fails to split the BBs. I have a patch.
Author: rguenth Date: Mon Jun 8 14:53:19 2015 New Revision: 224235 URL: https://gcc.gnu.org/viewcvs?rev=224235&root=gcc&view=rev Log: 2015-06-08 Richard Biener <rguenther@suse.de> PR tree-optimization/66422 * tree-ssa-loop-ivcanon.c (remove_exits_and_undefined_stmts): Split block after inserted gcc_unreachable. * gcc.dg/Warray-bounds-16.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/Warray-bounds-16.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-loop-ivcanon.c
Fixed on trunk sofar.
> > run_foo () > { > ... > <bb 10>: > _33 = myfoo_28->buf[3]; > if (_33 != 1) > goto <bb 13>; > else > goto <bb 11>; > > <bb 11>: > _34 = (int) _27; > if (_34 > 4) > goto <bb 12>; > else > goto <bb 13>; > > <bb 12>: > __builtin_unreachable (); > _35 = myfoo_28->buf[4]; > > <bb 13>: > # _36 = PHI <0(2), 1(3), 0(4), 1(5), 0(6), 1(7), 0(8), 1(9), 0(10), 1(11), > 0(12)> > if (_36 != 0) > goto <bb 15>; > else > goto <bb 14>; > > <bb 14>: > i_37 = 1; > > <bb 15>: > # _2 = PHI <0(13), -1(14)> > return _2; > > and the unreachable () remains in the CFG. > > Honza - it seems that remove_exits_and_undefined_stmts inserts these > unreachable calls but fails to split the BBs. > > I have a patch. Hmm, Indeed. I have expected cleanup_cfg to get rid of _35 = myfoo_28->buf[4];. I suppose this changed with your compile time work for GCC 5? Honza
On June 12, 2015 12:01:36 AM GMT+02:00, hubicka at ucw dot cz <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66422 > >--- Comment #8 from Jan Hubicka <hubicka at ucw dot cz> --- >> >> run_foo () >> { >> ... >> <bb 10>: >> _33 = myfoo_28->buf[3]; >> if (_33 != 1) >> goto <bb 13>; >> else >> goto <bb 11>; >> >> <bb 11>: >> _34 = (int) _27; >> if (_34 > 4) >> goto <bb 12>; >> else >> goto <bb 13>; >> >> <bb 12>: >> __builtin_unreachable (); >> _35 = myfoo_28->buf[4]; >> >> <bb 13>: >> # _36 = PHI <0(2), 1(3), 0(4), 1(5), 0(6), 1(7), 0(8), 1(9), 0(10), >1(11), >> 0(12)> >> if (_36 != 0) >> goto <bb 15>; >> else >> goto <bb 14>; >> >> <bb 14>: >> i_37 = 1; >> >> <bb 15>: >> # _2 = PHI <0(13), -1(14)> >> return _2; >> >> and the unreachable () remains in the CFG. >> >> Honza - it seems that remove_exits_and_undefined_stmts inserts these >> unreachable calls but fails to split the BBs. >> >> I have a patch. > >Hmm, Indeed. I have expected cleanup_cfg to get rid of _35 = >myfoo_28->buf[4];. >I suppose this changed with your compile time work for GCC 5? Yes. It no longer scans the whole basic block but just tge last statements. Richard. >Honza
Fixed.
Author: rguenth Date: Mon Jun 22 14:12:24 2015 New Revision: 224732 URL: https://gcc.gnu.org/viewcvs?rev=224732&root=gcc&view=rev Log: 2015-06-22 Richard Biener <rguenther@suse.de> Backport from mainline 2015-06-08 Richard Biener <rguenther@suse.de> PR tree-optimization/66422 * tree-ssa-loop-ivcanon.c (remove_exits_and_undefined_stmts): Split block after inserted gcc_unreachable. * gcc.dg/Warray-bounds-16.c: New testcase. Added: branches/gcc-5-branch/gcc/testsuite/gcc.dg/Warray-bounds-16.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/testsuite/ChangeLog branches/gcc-5-branch/gcc/tree-ssa-loop-ivcanon.c