diff mbox series

[v2] eal: fix rte_memcpy build on ppc with gcc 9.3

Message ID 20200504210347.24094-1-drc@linux.vnet.ibm.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v2] eal: fix rte_memcpy build on ppc with gcc 9.3 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-nxp-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

David Christensen May 4, 2020, 9:03 p.m. UTC 
  Building DPDK on Ubuntu 20.04 with GCC 9.3.0 results in a "subscript is
outside array bounds" message in rte_memcpy function.  The build error
is caused by an interaction between __builtin_constant_p and
"-Werror=array-bounds" as described in this bugzilla:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90387

Modify the code to disable the array-bounds check for GCC versions 9.0
to 9.3.

Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
---

v2:
* Replace __GNUC__ and __GNUC_MINOR__ with GCC_VERSION
---
 lib/librte_eal/ppc/include/rte_memcpy.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Ferruh Yigit May 5, 2020, 10:33 a.m. UTC | #1 
On 5/4/2020 10:03 PM, David Christensen wrote:
> Building DPDK on Ubuntu 20.04 with GCC 9.3.0 results in a "subscript is
> outside array bounds" message in rte_memcpy function.  The build error
> is caused by an interaction between __builtin_constant_p and
> "-Werror=array-bounds" as described in this bugzilla:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90387
> 
> Modify the code to disable the array-bounds check for GCC versions 9.0
> to 9.3.
> 
> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> ---
> 
> v2:
> * Replace __GNUC__ and __GNUC_MINOR__ with GCC_VERSION
> ---
>  lib/librte_eal/ppc/include/rte_memcpy.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
> index 25311ba1d..de47a5d2e 100644
> --- a/lib/librte_eal/ppc/include/rte_memcpy.h
> +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
> @@ -8,8 +8,8 @@
>  
>  #include <stdint.h>
>  #include <string.h>
> -/*To include altivec.h, GCC version must  >= 4.8 */
> -#include <altivec.h>
> +#include "rte_altivec.h"
> +#include "rte_common.h"

I can't find "rte_altivec.h", am I missing something.

With just ignoring "-Warray-bounds" changes, I confirm ena build issue is fixed
with gcc 9.1

>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -17,6 +17,11 @@ extern "C" {
>  
>  #include "generic/rte_memcpy.h"
>  
> +#if (GCC_VERSION >= 90000 && GCC_VERSION < 90400)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Warray-bounds"
> +#endif
> +
>  static inline void
>  rte_mov16(uint8_t *dst, const uint8_t *src)
>  {
> @@ -192,6 +197,10 @@ rte_memcpy_func(void *dst, const void *src, size_t n)
>  	return ret;
>  }
>  
> +#if (GCC_VERSION >= 90000 && GCC_VERSION < 90400)
> +#pragma GCC diagnostic pop
> +#endif
> +
>  #ifdef __cplusplus
>  }
>  #endif
>
David Christensen May 5, 2020, 4:32 p.m. UTC | #2 
>> diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
>> index 25311ba1d..de47a5d2e 100644
>> --- a/lib/librte_eal/ppc/include/rte_memcpy.h
>> +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
>> @@ -8,8 +8,8 @@
>>   
>>   #include <stdint.h>
>>   #include <string.h>
>> -/*To include altivec.h, GCC version must  >= 4.8 */
>> -#include <altivec.h>
>> +#include "rte_altivec.h"
>> +#include "rte_common.h"
> 
> I can't find "rte_altivec.h", am I missing something.
> 
> With just ignoring "-Warray-bounds" changes, I confirm ena build issue is fixed
> with gcc 9.1

The rte_altivec.h is related to another open patch required to build on 
POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to 
be accepted.  You may not have encountered it if you're not building the 
MLX5 PMD which has additional library requirements.

Dave
David Marchand May 5, 2020, 4:41 p.m. UTC | #3 
On Tue, May 5, 2020 at 6:32 PM David Christensen <drc@linux.vnet.ibm.com> wrote:
> >> diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
> >> index 25311ba1d..de47a5d2e 100644
> >> --- a/lib/librte_eal/ppc/include/rte_memcpy.h
> >> +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
> >> @@ -8,8 +8,8 @@
> >>
> >>   #include <stdint.h>
> >>   #include <string.h>
> >> -/*To include altivec.h, GCC version must  >= 4.8 */
> >> -#include <altivec.h>
> >> +#include "rte_altivec.h"
> >> +#include "rte_common.h"
> >
> > I can't find "rte_altivec.h", am I missing something.
> >
> > With just ignoring "-Warray-bounds" changes, I confirm ena build issue is fixed
> > with gcc 9.1
>
> The rte_altivec.h is related to another open patch required to build on
> POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to
> be accepted.  You may not have encountered it if you're not building the
> MLX5 PMD which has additional library requirements.

Is there a point in having two patches?
Ferruh Yigit May 5, 2020, 6:42 p.m. UTC | #4 
On 5/5/2020 5:32 PM, David Christensen wrote:
> 
> 
>>> diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
>>> index 25311ba1d..de47a5d2e 100644
>>> --- a/lib/librte_eal/ppc/include/rte_memcpy.h
>>> +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
>>> @@ -8,8 +8,8 @@
>>>   
>>>   #include <stdint.h>
>>>   #include <string.h>
>>> -/*To include altivec.h, GCC version must  >= 4.8 */
>>> -#include <altivec.h>
>>> +#include "rte_altivec.h"
>>> +#include "rte_common.h"
>>
>> I can't find "rte_altivec.h", am I missing something.
>>
>> With just ignoring "-Warray-bounds" changes, I confirm ena build issue is fixed
>> with gcc 9.1
> 
> The rte_altivec.h is related to another open patch required to build on 
> POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to 
> be accepted.  You may not have encountered it if you're not building the 
> MLX5 PMD which has additional library requirements.
> 

I see, I missed it. Looks good on top of that patch, although it still doesn't
apply cleanly.

It helps to put a comment to the patch about the dependent patch if it is not
merged yet.
David Christensen May 5, 2020, 8:28 p.m. UTC | #5 
>> The rte_altivec.h is related to another open patch required to build on
>> POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to
>> be accepted.  You may not have encountered it if you're not building the
>> MLX5 PMD which has additional library requirements.
> 
> Is there a point in having two patches?

The address different problems.  This patch addresses a build issue with 
gcc in Ubuntu 20.04, the other patch is related to the new trace library 
that uses boolean values from stdbool.h (or the use of the --std=c11 
build option) that causes a build error on POWER systems because of 
altivec definitions.

Dave
David Christensen May 5, 2020, 8:37 p.m. UTC | #6 
>>> I can't find "rte_altivec.h", am I missing something.
>>>
>>> With just ignoring "-Warray-bounds" changes, I confirm ena build issue is fixed
>>> with gcc 9.1
>>
>> The rte_altivec.h is related to another open patch required to build on
>> POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to
>> be accepted.  You may not have encountered it if you're not building the
>> MLX5 PMD which has additional library requirements.
>>
> 
> I see, I missed it. Looks good on top of that patch, although it still doesn't
> apply cleanly.
> 
> It helps to put a comment to the patch about the dependent patch if it is not
> merged yet.

It was an oversight on my part, forgetting that I had another patch 
installed.  I can submit a corrected version if you think it's necessary 
but then that that patch will have to be adjusted when it's eventually 
merged.  Sorry for the confusion.

Dave
Ferruh Yigit May 6, 2020, 9:23 a.m. UTC | #7 
On 5/5/2020 9:37 PM, David Christensen wrote:
>>>> I can't find "rte_altivec.h", am I missing something.
>>>>
>>>> With just ignoring "-Warray-bounds" changes, I confirm ena build issue is fixed
>>>> with gcc 9.1
>>>
>>> The rte_altivec.h is related to another open patch required to build on
>>> POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to
>>> be accepted.  You may not have encountered it if you're not building the
>>> MLX5 PMD which has additional library requirements.
>>>
>>
>> I see, I missed it. Looks good on top of that patch, although it still doesn't
>> apply cleanly.
>>
>> It helps to put a comment to the patch about the dependent patch if it is not
>> merged yet.
> 
> It was an oversight on my part, forgetting that I had another patch 
> installed.  I can submit a corrected version if you think it's necessary 
> but then that that patch will have to be adjusted when it's eventually 
> merged.  Sorry for the confusion.
> 

No worries, but I won't be merging the patches, it looks easy to manage the
conflict but eventually it is up to David to have a new version or not.
David Marchand May 6, 2020, 9:35 a.m. UTC | #8 
On Tue, May 5, 2020 at 10:28 PM David Christensen
<drc@linux.vnet.ibm.com> wrote:
>
>
> >> The rte_altivec.h is related to another open patch required to build on
> >> POWER systems (http://patches.dpdk.org/patch/69605/) that's waiting to
> >> be accepted.  You may not have encountered it if you're not building the
> >> MLX5 PMD which has additional library requirements.
> >
> > Is there a point in having two patches?
>
> The address different problems.  This patch addresses a build issue with
> gcc in Ubuntu 20.04, the other patch is related to the new trace library
> that uses boolean values from stdbool.h (or the use of the --std=c11
> build option) that causes a build error on POWER systems because of
> altivec definitions.

Ok, thanks.
WRT the conflict on the other ppc patch, no need for a new version.

Though it is probably worth backporting if it is a problem with gcc 9.x.
WDYT?
I can add a Cc: stable@dpdk.org when applying.
David Christensen May 6, 2020, 3:59 p.m. UTC | #9 
> Though it is probably worth backporting if it is a problem with gcc 9.x.
> WDYT?
> I can add a Cc: stable@dpdk.org when applying.

Agree, should be backported.

Dave
David Marchand May 6, 2020, 4:13 p.m. UTC | #10 
On Mon, May 4, 2020 at 11:04 PM David Christensen
<drc@linux.vnet.ibm.com> wrote:
>
> Building DPDK on Ubuntu 20.04 with GCC 9.3.0 results in a "subscript is
> outside array bounds" message in rte_memcpy function.  The build error
> is caused by an interaction between __builtin_constant_p and
> "-Werror=array-bounds" as described in this bugzilla:

Good to know, thanks.


> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90387
>
> Modify the code to disable the array-bounds check for GCC versions 9.0
> to 9.3.
>

Cc: stable@dpdk.org

> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>

Applied, thanks.
diff mbox series

Patch

diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
index 25311ba1d..de47a5d2e 100644
--- a/lib/librte_eal/ppc/include/rte_memcpy.h
+++ b/lib/librte_eal/ppc/include/rte_memcpy.h
@@ -8,8 +8,8 @@ 
 
 #include <stdint.h>
 #include <string.h>
-/*To include altivec.h, GCC version must  >= 4.8 */
-#include <altivec.h>
+#include "rte_altivec.h"
+#include "rte_common.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -17,6 +17,11 @@  extern "C" {
 
 #include "generic/rte_memcpy.h"
 
+#if (GCC_VERSION >= 90000 && GCC_VERSION < 90400)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
+
 static inline void
 rte_mov16(uint8_t *dst, const uint8_t *src)
 {
@@ -192,6 +197,10 @@  rte_memcpy_func(void *dst, const void *src, size_t n)
 	return ret;
 }
 
+#if (GCC_VERSION >= 90000 && GCC_VERSION < 90400)
+#pragma GCC diagnostic pop
+#endif
+
 #ifdef __cplusplus
 }
 #endif