[PATCHv2] linux-generic: packet: fix buggy compiler error

Message ID 1482414591-10661-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Dec. 22, 2016, 1:49 p.m.
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 fails on .len line. 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>

---
 v2: - add comment (Mike)
     - in description .len line reorder does not fix compilation (Bill)

 platform/linux-generic/odp_packet.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.7.1.250.gff4ea60

Comments

Mike Holmes Dec. 22, 2016, 2:08 p.m. | #1
On 22 December 2016 at 08:49, 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 fails on .len line. 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>

>


Reviewed-by: Mike Holmes <mike.holmes@linaro.org>


-O3 now passes for me on x86 gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005



> ---

>  v2: - add comment (Mike)

>      - in description .len line reorder does not fix compilation (Bill)

>

>  platform/linux-generic/odp_packet.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/platform/linux-generic/odp_packet.c

> b/platform/linux-generic/odp_packet.c

> index 0d3fd05..599d839 100644

> --- a/platform/linux-generic/odp_packet.c

> +++ b/platform/linux-generic/odp_packet.c

> @@ -309,6 +309,10 @@ static inline void copy_num_segs(odp_packet_hdr_t

> *to, odp_packet_hdr_t *from,

>         int i;

>

>         for (i = 0; i < num; i++) {

> +               /* check bellow also fixes gcc bug, refer to corresponding

> +                * git commit */

> +               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

>

>



-- 
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"
Savolainen, Petri (Nokia - FI/Espoo) Dec. 22, 2016, 2:13 p.m. | #2
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Mike

> Holmes

> Sent: Thursday, December 22, 2016 4:08 PM

> To: Maxim Uvarov <maxim.uvarov@linaro.org>

> Cc: lng-odp <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy compiler

> error

> 

> On 22 December 2016 at 08:49, 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 fails on .len line. 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>

> >

> 

> Reviewed-by: Mike Holmes <mike.holmes@linaro.org>

> 

> -O3 now passes for me on x86 gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005

> 


I'll send shortly a patch set which modifies also this function. I think it should remove the issue also.

-Petri
Maxim Uvarov Dec. 22, 2016, 2:15 p.m. | #3
On 22 December 2016 at 17:13, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia-bell-labs.com> wrote:

>

>

> > -----Original Message-----

> > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Mike

> > Holmes

> > Sent: Thursday, December 22, 2016 4:08 PM

> > To: Maxim Uvarov <maxim.uvarov@linaro.org>

> > Cc: lng-odp <lng-odp@lists.linaro.org>

> > Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy

> compiler

> > error

> >

> > On 22 December 2016 at 08:49, 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 fails on .len line. 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>

> > >

> >

> > Reviewed-by: Mike Holmes <mike.holmes@linaro.org>

> >

> > -O3 now passes for me on x86 gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005

> >

>

> I'll send shortly a patch set which modifies also this function. I think

> it should remove the issue also.

>

> -Petri

>

> Ok, waiting.
Bill Fischofer Dec. 22, 2016, 2:20 p.m. | #4
On Thu, Dec 22, 2016 at 8:13 AM, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia-bell-labs.com> wrote:
>

>

>> -----Original Message-----

>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Mike

>> Holmes

>> Sent: Thursday, December 22, 2016 4:08 PM

>> To: Maxim Uvarov <maxim.uvarov@linaro.org>

>> Cc: lng-odp <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy compiler

>> error

>>

>> On 22 December 2016 at 08:49, 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 fails on .len line. 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>

>> >

>>

>> Reviewed-by: Mike Holmes <mike.holmes@linaro.org>

>>

>> -O3 now passes for me on x86 gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005

>>

>

> I'll send shortly a patch set which modifies also this function. I think it should remove the issue also.


Thanks, Petri. Can you also please take a look at my patch for Bug
2789, http://patches.opendataplane.org/patch/7696/ as this is also
working in the new pool code area.

>

> -Petri

>
Savolainen, Petri (Nokia - FI/Espoo) Dec. 22, 2016, 2:39 p.m. | #5
> -----Original Message-----

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> Sent: Thursday, December 22, 2016 4:20 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-

> labs.com>

> Cc: Mike Holmes <mike.holmes@linaro.org>; Maxim Uvarov

> <maxim.uvarov@linaro.org>; lng-odp <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy compiler

> error

> 

> On Thu, Dec 22, 2016 at 8:13 AM, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia-bell-labs.com> wrote:

> >

> >

> >> -----Original Message-----

> >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Mike

> >> Holmes

> >> Sent: Thursday, December 22, 2016 4:08 PM

> >> To: Maxim Uvarov <maxim.uvarov@linaro.org>

> >> Cc: lng-odp <lng-odp@lists.linaro.org>

> >> Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy

> compiler

> >> error

> >>

> >> On 22 December 2016 at 08:49, 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 fails on .len line. 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>

> >> >

> >>

> >> Reviewed-by: Mike Holmes <mike.holmes@linaro.org>

> >>

> >> -O3 now passes for me on x86 gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0

> 20161005

> >>

> >

> > I'll send shortly a patch set which modifies also this function. I think

> it should remove the issue also.

> 

> Thanks, Petri. Can you also please take a look at my patch for Bug

> 2789, http://patches.opendataplane.org/patch/7696/ as this is also

> working in the new pool code area.

> 


The free_segments() issues is corrected. The trunc head/tail might not be ... but on the other hand I'm not sure if zero length packets must be supported. Trunc documentation allows that, but e.g. alloc does not. Zero length packets are not very useful, so we could change API to get rid of the corner case.

-Petri
Maxim Uvarov Dec. 22, 2016, 2:48 p.m. | #6
Something updated in CI and we are again green:

https://ci.linaro.org/job/odp-api-check/


Maxim.

On 12/22/16 17:39, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 

> 

>> -----Original Message-----

>> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

>> Sent: Thursday, December 22, 2016 4:20 PM

>> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-

>> labs.com>

>> Cc: Mike Holmes <mike.holmes@linaro.org>; Maxim Uvarov

>> <maxim.uvarov@linaro.org>; lng-odp <lng-odp@lists.linaro.org>

>> Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy compiler

>> error

>>

>> On Thu, Dec 22, 2016 at 8:13 AM, Savolainen, Petri (Nokia - FI/Espoo)

>> <petri.savolainen@nokia-bell-labs.com> wrote:

>>>

>>>

>>>> -----Original Message-----

>>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>> Mike

>>>> Holmes

>>>> Sent: Thursday, December 22, 2016 4:08 PM

>>>> To: Maxim Uvarov <maxim.uvarov@linaro.org>

>>>> Cc: lng-odp <lng-odp@lists.linaro.org>

>>>> Subject: Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy

>> compiler

>>>> error

>>>>

>>>> On 22 December 2016 at 08:49, 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 fails on .len line. 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>

>>>>>

>>>>

>>>> Reviewed-by: Mike Holmes <mike.holmes@linaro.org>

>>>>

>>>> -O3 now passes for me on x86 gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0

>> 20161005

>>>>

>>>

>>> I'll send shortly a patch set which modifies also this function. I think

>> it should remove the issue also.

>>

>> Thanks, Petri. Can you also please take a look at my patch for Bug

>> 2789, http://patches.opendataplane.org/patch/7696/ as this is also

>> working in the new pool code area.

>>

> 

> The free_segments() issues is corrected. The trunc head/tail might not be ... but on the other hand I'm not sure if zero length packets must be supported. Trunc documentation allows that, but e.g. alloc does not. Zero length packets are not very useful, so we could change API to get rid of the corner case.

> 

> -Petri

>
Nicolas Morey-Chaisemartin Dec. 22, 2016, 2:58 p.m. | #7
Le 12/22/2016 à 02:49 PM, Maxim Uvarov a écrit :
> 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 fails on .len line. 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>

> ---

>  v2: - add comment (Mike)

>      - in description .len line reorder does not fix compilation (Bill)

>

>  platform/linux-generic/odp_packet.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c

> index 0d3fd05..599d839 100644

> --- a/platform/linux-generic/odp_packet.c

> +++ b/platform/linux-generic/odp_packet.c

> @@ -309,6 +309,10 @@ static inline void copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,

>  	int i;

>  

>  	for (i = 0; i < num; i++) {

> +		/* check bellow also fixes gcc bug, refer to corresponding

> +		 * git commit */

> +		if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))

> +			ODP_ABORT("packet segmenation error\n");

There's a typo in segmentation

>  		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;

Patch

diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 0d3fd05..599d839 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -309,6 +309,10 @@  static inline void copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,
 	int i;
 
 	for (i = 0; i < num; i++) {
+		/* check bellow also fixes gcc bug, refer to corresponding
+		 * git commit */
+		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;