I posted a patch to fix Bug 2789 but it's orthogonal to this issue. We should still see if we can come up with a compile-time-only circumvention.
On Wed, Dec 21, 2016 at 12:30 PM, Maxim Uvarov <maxim.uva...@linaro.org> wrote: > On 12/21/16 19:07, Bill Fischofer wrote: >> On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes <mike.hol...@linaro.org> wrote: >>> On 21 December 2016 at 10:49, Maxim Uvarov <maxim.uva...@linaro.org> wrote: >>> >>>> On 12/21/16 18:31, Mike Holmes wrote: >>>>> >>>>> >>>>> On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uva...@linaro.org >>>>> <mailto:maxim.uva...@linaro.org>> wrote: >>>>> >>>>> On debian jessie gcc unable to compile current code >>>>> with error: >>>>> odp_packet.c: In function 'odp_packet_trunc_head': >>>>> odp_packet.c:314:46: error: array subscript is above array bounds >>>>> [-Werror=array-bounds] >>>>> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len; >>>>> >>>>> The problem is that it breaks compilation only on .len line: >>>>> for (i = 0; i < num; i++) { >>>>> to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr; >>>>> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data; >>>>> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len; >>>>> } >>>>> >>>>> If that line is commented out compilation passes. If lines are >>>> reordered >>>>> than compilation also passes. Because there is no warning on .hdr >>>>> and .data >>>>> it looks like compiler error. Additional check for preconfigured >>>> value >>>>> is workaround to that situation. >>>>> >>>>> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org >>>>> <mailto:maxim.uva...@linaro.org>> >>>>> --- >>>>> platform/linux-generic/odp_packet.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/platform/linux-generic/odp_packet.c >>>>> b/platform/linux-generic/odp_packet.c >>>>> index 0d3fd05..f3e0a12 100644 >>>>> --- a/platform/linux-generic/odp_packet.c >>>>> +++ b/platform/linux-generic/odp_packet.c >>>>> @@ -309,6 +309,8 @@ static inline void >>>>> copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from, >>>>> int i; >>>>> >>>>> for (i = 0; i < num; i++) { >>>>> + if (odp_unlikely((num + i) >= >>>> CONFIG_PACKET_MAX_SEGS)) >>>>> + ODP_ABORT("packet segmenation error\n"); >>>>> >>>>> >>>>> Does this impact performance, if so should it be optionally compiled in >>>>> for that GCC version ? >>>>> >>>> >>>> >>>> CONFIG_PACKET_MAX_SEG is currently 2. And according to our tests it will >>>> add one if check for odp_packet_trunc_head() and odp_packet_trunc_tail() >>>> calls. So overhead is one if instruction. >> >> I agree that this is a bit of a kludge. I'm currently debugging Bug >> 2789 where things fail if CONFIG_PACKET_MAX_SEG > 2. I'd like to see >> if the fix for that doesn't result in a cleaner implementation in this >> area that may not expose this issue. >> > > btw, compilation fails only with -O2 or -O3 and -O0 compiles. > > Maxim. > > >>>> >>>> >>>> It fails on Jessie, which is: >>>> # gcc -v >>>> Using built-in specs. >>>> COLLECT_GCC=gcc >>>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper >>>> Target: x86_64-linux-gnu >>>> Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10' >>>> --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs >>>> --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr >>>> --program-suffix=-4.9 --enable-shared --enable-linker-build-id >>>> --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix >>>> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib >>>> --enable-nls --with-sysroot=/ --enable-clocale=gnu >>>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes >>>> --enable-gnu-unique-object --disable-vtable-verify --enable-plugin >>>> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk >>>> --enable-gtk-cairo >>>> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre >>>> --enable-java-home >>>> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64 >>>> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64 >>>> --with-arch-directory=amd64 >>>> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc >>>> --enable-multiarch --with-arch-32=i586 --with-abi=m64 >>>> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic >>>> --enable-checking=release --build=x86_64-linux-gnu >>>> --host=x86_64-linux-gnu --target=x86_64-linux-gnu >>>> Thread model: posix >>>> gcc version 4.9.2 (Debian 4.9.2-10) >>>> >>>> >>>> And passes on my Ubuntu which is (4.8 or 4.9): >>>> gcc-4.9 -v >>>> Using built-in specs. >>>> COLLECT_GCC=gcc-4.9 >>>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper >>>> Target: x86_64-linux-gnu >>>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu >>>> 4.9.3-0ubuntu4' >>>> --with-bugurl=file:///usr/share/doc/gccgo-4.9/README.Bugs >>>> --enable-languages=c,c++,go --prefix=/usr --program-suffix=-4.9 >>>> --enable-shared --enable-linker-build-id --libexecdir=/usr/lib >>>> --without-included-gettext --enable-threads=posix >>>> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib >>>> --enable-nls --with-sysroot=/ --enable-clocale=gnu >>>> --enable-libstdcxx-time=yes --enable-gnu-unique-object >>>> --disable-vtable-verify --disable-libquadmath --enable-plugin >>>> --with-system-zlib --enable-multiarch --disable-werror >>>> --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 >>>> --enable-multilib --with-tune=generic --enable-checking=release >>>> --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu >>>> Thread model: posix >>>> gcc version 4.9.3 (Ubuntu 4.9.3-0ubuntu4) >>>> >>>> >>>> And for arm it looks like it failed with some 5.x gcc version. So it >>>> might be not easy to define exactly which version is it. >>>> >>>> >>>> >>> Perhaps we need to just add a comment then - possibly a doxygen one maybe >>> a @note ? >>> So that anyone editing in future will not re break it by mistake, they >>> might not test on a legacy compiler >>> >>> >>>> >>>> >>>>> >>>>> >>>>> to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + >>>>> i].hdr; >>>>> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + >>>>> i].data; >>>>> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + >>>>> i].len; >>>>> -- >>>>> 2.7.1.250.gff4ea60 >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Mike Holmes >>>>> Program Manager - Linaro Networking Group >>>>> Linaro.org <http://www.linaro.org/>* **│ *Open source software for ARM >>>> SoCs >>>>> "Work should be fun and collaborative, the rest follows" >>>>> >>>>> __ >>>>> >>>>> >>>> >>>> >>> >>> >>> -- >>> Mike Holmes >>> Program Manager - Linaro Networking Group >>> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs >>> "Work should be fun and collaborative, the rest follows" >
