| Message ID | 1482333963-20494-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uvarov@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.uvarov@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 ? > 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"
On 12/21/16 18:31, Mike Holmes wrote: > > > On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@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.uvarov@linaro.org > <mailto:maxim.uvarov@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. 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. > > > 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" > > __ > >
On 21 December 2016 at 10:49, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/21/16 18:31, Mike Holmes wrote: > > > > > > On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uvarov@linaro.org > > <mailto:maxim.uvarov@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.uvarov@linaro.org > > <mailto:maxim.uvarov@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. > > > 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"
On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > On 21 December 2016 at 10:49, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > >> On 12/21/16 18:31, Mike Holmes wrote: >> > >> > >> > On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uvarov@linaro.org >> > <mailto:maxim.uvarov@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.uvarov@linaro.org >> > <mailto:maxim.uvarov@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. >> >> >> 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"
On 12/21/16 19:07, Bill Fischofer wrote: > On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes <mike.holmes@linaro.org> wrote: >> On 21 December 2016 at 10:49, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> >>> On 12/21/16 18:31, Mike Holmes wrote: >>>> >>>> >>>> On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uvarov@linaro.org >>>> <mailto:maxim.uvarov@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.uvarov@linaro.org >>>> <mailto:maxim.uvarov@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"
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.uvarov@linaro.org> wrote: > On 12/21/16 19:07, Bill Fischofer wrote: >> On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes <mike.holmes@linaro.org> wrote: >>> On 21 December 2016 at 10:49, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >>> >>>> On 12/21/16 18:31, Mike Holmes wrote: >>>>> >>>>> >>>>> On 21 December 2016 at 10:26, Maxim Uvarov <maxim.uvarov@linaro.org >>>>> <mailto:maxim.uvarov@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.uvarov@linaro.org >>>>> <mailto:maxim.uvarov@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" >
On Wed, Dec 21, 2016 at 9:26 AM, Maxim Uvarov <maxim.uvarov@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. If reordering solves the problem then why is that not the preferred solution since that doesn't add any pathlength? These statements are independent so it shouldn't matter. I would put a comment in referencing the bug to give future maintainers a caution about this. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@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"); > 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 >
On 21 December 2016 at 23:45, Bill Fischofer <bill.fischofer@linaro.org> wrote: > On Wed, Dec 21, 2016 at 9:26 AM, Maxim Uvarov <maxim.uvarov@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. > > If reordering solves the problem then why is that not the preferred > solution since that doesn't add any pathlength? These statements are > independent so it shouldn't matter. I would put a comment in > referencing the bug to give future maintainers a caution about this. > > Sorry, I wanted to write that reorder also does not solve the problems. If that line is commented out compilation passes. If lines are reordered than compilation also fails. Because there is no warning on .hdr and .data it looks like compiler error. Additional check for preconfigured value is workaround to that situation. Maxim. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@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"); > > 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 > > >
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"); 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;
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.uvarov@linaro.org> --- platform/linux-generic/odp_packet.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.7.1.250.gff4ea60