diff mbox series

[v2] mk: report address of packed member as warning

Message ID 20190502141316.25907-1-reshma.pattan@intel.com (mailing list archive)
State Not Applicable, archived
Headers
Series [v2] mk: report address of packed member as warning |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Pattan, Reshma May 2, 2019, 2:13 p.m. UTC 
  gcc 9 on Fedora 30 gives an error
"taking address of packed member may result in an
unaligned pointer value" for -Waddress-of-packed-member.

Report it as warning instead of error to fix the build.

Snippet of build before fix
...lib/librte_eal/linux/eal/eal_memalloc.c: In function ‘alloc_seg_walk’:
...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking address
of packed member of ‘struct rte_mem_config’ may result in an unaligned
pointer value [-Werror=address-of-packed-member]
  768 |  cur_msl = &mcfg->memsegs[msl_idx];
      |            ^~~~~~~~~~~~~~~~~~~~~~~

Snippet of build after fix
..lib/librte_eal/linux/eal/eal_memory.c: In function ‘remap_segment’:
..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking address
of packed member of ‘struct rte_mem_config’ may result in an unaligned
pointer value [-Waddress-of-packed-member]
  685 |   msl = &mcfg->memsegs[msl_idx];

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 mk/toolchain/gcc/rte.vars.mk | 4 ++++
 1 file changed, 4 insertions(+)

Comments

David Marchand May 2, 2019, 2:24 p.m. UTC | #1 
On Thu, May 2, 2019 at 4:13 PM Reshma Pattan <reshma.pattan@intel.com>
wrote:

> gcc 9 on Fedora 30 gives an error
> "taking address of packed member may result in an
> unaligned pointer value" for -Waddress-of-packed-member.
>
> Report it as warning instead of error to fix the build.
>
> Snippet of build before fix
> ...lib/librte_eal/linux/eal/eal_memalloc.c: In function ‘alloc_seg_walk’:
> ...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking address
> of packed member of ‘struct rte_mem_config’ may result in an unaligned
> pointer value [-Werror=address-of-packed-member]
>   768 |  cur_msl = &mcfg->memsegs[msl_idx];
>       |            ^~~~~~~~~~~~~~~~~~~~~~~
>
> Snippet of build after fix
> ..lib/librte_eal/linux/eal/eal_memory.c: In function ‘remap_segment’:
> ..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking address
> of packed member of ‘struct rte_mem_config’ may result in an unaligned
> pointer value [-Waddress-of-packed-member]
>   685 |   msl = &mcfg->memsegs[msl_idx];
>
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  mk/toolchain/gcc/rte.vars.mk | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> index d8b99faf6..61032bbbc 100644
> --- a/mk/toolchain/gcc/rte.vars.mk
> +++ b/mk/toolchain/gcc/rte.vars.mk
> @@ -87,5 +87,9 @@ WERROR_FLAGS += -Wimplicit-fallthrough=2
>  WERROR_FLAGS += -Wno-format-truncation
>  endif
>
> +#Report "taking address of packed member may result
> +#in an unaligned pointer value" issues as warnings.
> +WERROR_FLAGS += -Wno-error=address-of-packed-member
> +
>  export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>  export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
> --
> 2.21.0
>
>
No, you can't do this unconditionally.

Looked at clang (in make build system) and meson (globally) and those
completely disable the warning.
$ git grep address-of-packed-member origin/master --
origin/master:config/meson.build:       '-Wno-address-of-packed-member'
origin/master:mk/toolchain/clang/rte.vars.mk:WERROR_FLAGS +=
-Wno-address-of-packed-member

It should be something like (untested):

@@ -87,5 +87,10 @@ WERROR_FLAGS += -Wimplicit-fallthrough=2
 WERROR_FLAGS += -Wno-format-truncation
 endif

+# disable packed member unalign warnings
+ifeq ($(shell test $(GCC_VERSION) -ge 90 && echo 1), 1)
+WERROR_FLAGS += -Wno-address-of-packed-member
+endif
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
Bruce Richardson May 2, 2019, 2:41 p.m. UTC | #2 
On Thu, May 02, 2019 at 04:24:37PM +0200, David Marchand wrote:
>    On Thu, May 2, 2019 at 4:13 PM Reshma Pattan
>    <[1]reshma.pattan@intel.com> wrote:
> 
>      gcc 9 on Fedora 30 gives an error
>      "taking address of packed member may result in an
>      unaligned pointer value" for -Waddress-of-packed-member.
>      Report it as warning instead of error to fix the build.
>      Snippet of build before fix
>      ...lib/librte_eal/linux/eal/eal_memalloc.c: In function
>      ‘alloc_seg_walk’:
>      ...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking
>      address
>      of packed member of ‘struct rte_mem_config’ may result in an
>      unaligned
>      pointer value [-Werror=address-of-packed-member]
>        768 |  cur_msl = &mcfg->memsegs[msl_idx];
>            |            ^~~~~~~~~~~~~~~~~~~~~~~
>      Snippet of build after fix
>      ..lib/librte_eal/linux/eal/eal_memory.c: In function
>      ‘remap_segment’:
>      ..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking
>      address
>      of packed member of ‘struct rte_mem_config’ may result in an
>      unaligned
>      pointer value [-Waddress-of-packed-member]
>        685 |   msl = &mcfg->memsegs[msl_idx];
>      Signed-off-by: Reshma Pattan <[2]reshma.pattan@intel.com>
>      ---
>       mk/toolchain/gcc/[3]rte.vars.mk | 4 ++++
>       1 file changed, 4 insertions(+)
>      diff --git a/mk/toolchain/gcc/[4]rte.vars.mk
>      b/mk/toolchain/gcc/[5]rte.vars.mk
>      index d8b99faf6..61032bbbc 100644
>      --- a/mk/toolchain/gcc/[6]rte.vars.mk
>      +++ b/mk/toolchain/gcc/[7]rte.vars.mk
>      @@ -87,5 +87,9 @@ WERROR_FLAGS += -Wimplicit-fallthrough=2
>       WERROR_FLAGS += -Wno-format-truncation
>       endif
>      +#Report "taking address of packed member may result
>      +#in an unaligned pointer value" issues as warnings.
>      +WERROR_FLAGS += -Wno-error=address-of-packed-member
>      +
>       export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>       export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
>      --
>      2.21.0
> 
>    No, you can't do this unconditionally.
>    Looked at clang (in make build system) and meson (globally) and those
>    completely disable the warning.
>    $ git grep address-of-packed-member origin/master --
>    origin/master:config/meson.build:       '-Wno-address-of-packed-member'
>    origin/master:mk/toolchain/clang/rte.vars.mk:WERROR_FLAGS +=
>    -Wno-address-of-packed-member
>    It should be something like (untested):
>    @@ -87,5 +87,10 @@ WERROR_FLAGS += -Wimplicit-fallthrough=2
>     WERROR_FLAGS += -Wno-format-truncation
>     endif
> 
>    +# disable packed member unalign warnings
>    +ifeq ($(shell test $(GCC_VERSION) -ge 90 && echo 1), 1)
>    +WERROR_FLAGS += -Wno-address-of-packed-member
>    +endif
>    +
Actually, you should not need to check the GCC version for this.
GCC always ignores silently any warning disable flags it doesn't recognise,
only printing information about them if there is another error to report.
[This is why meson, when you ask it if the compiler supports "-Wno-foo", will
check the flag and also the -Wfoo flag, as gcc won't report an error with
just the "no" form]

/Bruce
David Marchand May 2, 2019, 2:48 p.m. UTC | #3 
On Thu, May 2, 2019 at 4:42 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, May 02, 2019 at 04:24:37PM +0200, David Marchand wrote:
> >    On Thu, May 2, 2019 at 4:13 PM Reshma Pattan
> >    <[1]reshma.pattan@intel.com> wrote:
> >
> >      gcc 9 on Fedora 30 gives an error
> >      "taking address of packed member may result in an
> >      unaligned pointer value" for -Waddress-of-packed-member.
> >      Report it as warning instead of error to fix the build.
> >      Snippet of build before fix
> >      ...lib/librte_eal/linux/eal/eal_memalloc.c: In function
> >      ‘alloc_seg_walk’:
> >      ...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking
> >      address
> >      of packed member of ‘struct rte_mem_config’ may result in an
> >      unaligned
> >      pointer value [-Werror=address-of-packed-member]
> >        768 |  cur_msl = &mcfg->memsegs[msl_idx];
> >            |            ^~~~~~~~~~~~~~~~~~~~~~~
> >      Snippet of build after fix
> >      ..lib/librte_eal/linux/eal/eal_memory.c: In function
> >      ‘remap_segment’:
> >      ..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking
> >      address
> >      of packed member of ‘struct rte_mem_config’ may result in an
> >      unaligned
> >      pointer value [-Waddress-of-packed-member]
> >        685 |   msl = &mcfg->memsegs[msl_idx];
> >      Signed-off-by: Reshma Pattan <[2]reshma.pattan@intel.com>
> >      ---
> >       mk/toolchain/gcc/[3]rte.vars.mk | 4 ++++
> >       1 file changed, 4 insertions(+)
> >      diff --git a/mk/toolchain/gcc/[4]rte.vars.mk
> >      b/mk/toolchain/gcc/[5]rte.vars.mk
> >      index d8b99faf6..61032bbbc 100644
> >      --- a/mk/toolchain/gcc/[6]rte.vars.mk
> >      +++ b/mk/toolchain/gcc/[7]rte.vars.mk
> >      @@ -87,5 +87,9 @@ WERROR_FLAGS += -Wimplicit-fallthrough=2
> >       WERROR_FLAGS += -Wno-format-truncation
> >       endif
> >      +#Report "taking address of packed member may result
> >      +#in an unaligned pointer value" issues as warnings.
> >      +WERROR_FLAGS += -Wno-error=address-of-packed-member
> >      +
> >       export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
> >       export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
> >      --
> >      2.21.0
> >
> >    No, you can't do this unconditionally.
> >    Looked at clang (in make build system) and meson (globally) and those
> >    completely disable the warning.
> >    $ git grep address-of-packed-member origin/master --
> >    origin/master:config/meson.build:
>  '-Wno-address-of-packed-member'
> >    origin/master:mk/toolchain/clang/rte.vars.mk:WERROR_FLAGS +=
> >    -Wno-address-of-packed-member
> >    It should be something like (untested):
> >    @@ -87,5 +87,10 @@ WERROR_FLAGS += -Wimplicit-fallthrough=2
> >     WERROR_FLAGS += -Wno-format-truncation
> >     endif
> >
> >    +# disable packed member unalign warnings
> >    +ifeq ($(shell test $(GCC_VERSION) -ge 90 && echo 1), 1)
> >    +WERROR_FLAGS += -Wno-address-of-packed-member
> >    +endif
> >    +
> Actually, you should not need to check the GCC version for this.
> GCC always ignores silently any warning disable flags it doesn't recognise,
> only printing information about them if there is another error to report.
> [This is why meson, when you ask it if the compiler supports "-Wno-foo",
> will
> check the flag and also the -Wfoo flag, as gcc won't report an error with
> just the "no" form]
>

WERROR_FLAGS += -Wno-error=address-of-packed-member triggers an error.

[dmarchan@dmarchan dpdk]$ make EXTRA_CFLAGS="-g -O0" O=master
cc1: error: -Werror=address-of-packed-member: no option
-Waddress-of-packed-member
== Build lib
cc1: error: -Werror=address-of-packed-member: no option
-Waddress-of-packed-member
== Build lib/librte_kvargs
cc1: error: -Werror=address-of-packed-member: no option
-Waddress-of-packed-member
  CC rte_kvargs.o
cc1: error: -Werror=address-of-packed-member: no option
-Waddress-of-packed-member
make[3]: *** [rte_kvargs.o] Error 1
make[2]: *** [librte_kvargs] Error 2
make[1]: *** [lib] Error 2
make: *** [all] Error 2


WERROR_FLAGS += -Wno-address-of-packed-member is ignored.

Marvellous :-)
Sorry for the noise, let's go with your initial patch.
Anatoly Burakov May 2, 2019, 3:53 p.m. UTC | #4 
On 02-May-19 3:13 PM, Reshma Pattan wrote:
> gcc 9 on Fedora 30 gives an error
> "taking address of packed member may result in an
> unaligned pointer value" for -Waddress-of-packed-member.
> 
> Report it as warning instead of error to fix the build.
> 
> Snippet of build before fix
> ...lib/librte_eal/linux/eal/eal_memalloc.c: In function ‘alloc_seg_walk’:
> ...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking address
> of packed member of ‘struct rte_mem_config’ may result in an unaligned
> pointer value [-Werror=address-of-packed-member]
>    768 |  cur_msl = &mcfg->memsegs[msl_idx];
>        |            ^~~~~~~~~~~~~~~~~~~~~~~
> 
> Snippet of build after fix
> ..lib/librte_eal/linux/eal/eal_memory.c: In function ‘remap_segment’:
> ..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking address
> of packed member of ‘struct rte_mem_config’ may result in an unaligned
> pointer value [-Waddress-of-packed-member]
>    685 |   msl = &mcfg->memsegs[msl_idx];
> 

Fixing these would require an ABI break, because these are exposed 
externally. Should we submit a deprecation notice for EAL?
Stephen Hemminger May 2, 2019, 6:54 p.m. UTC | #5 
On Thu, 2 May 2019 16:53:50 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 02-May-19 3:13 PM, Reshma Pattan wrote:
> > gcc 9 on Fedora 30 gives an error
> > "taking address of packed member may result in an
> > unaligned pointer value" for -Waddress-of-packed-member.
> > 
> > Report it as warning instead of error to fix the build.
> > 
> > Snippet of build before fix
> > ...lib/librte_eal/linux/eal/eal_memalloc.c: In function ‘alloc_seg_walk’:
> > ...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking address
> > of packed member of ‘struct rte_mem_config’ may result in an unaligned
> > pointer value [-Werror=address-of-packed-member]
> >    768 |  cur_msl = &mcfg->memsegs[msl_idx];
> >        |            ^~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Snippet of build after fix
> > ..lib/librte_eal/linux/eal/eal_memory.c: In function ‘remap_segment’:
> > ..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking address
> > of packed member of ‘struct rte_mem_config’ may result in an unaligned
> > pointer value [-Waddress-of-packed-member]
> >    685 |   msl = &mcfg->memsegs[msl_idx];
> >   
> 
> Fixing these would require an ABI break, because these are exposed 
> externally. Should we submit a deprecation notice for EAL?


Ideally mem config and related structures would not be exposed in the
API. Like lcore_config and eal_config it should be eal_private
Anatoly Burakov May 3, 2019, 7:56 a.m. UTC | #6 
On 02-May-19 7:54 PM, Stephen Hemminger wrote:
> On Thu, 2 May 2019 16:53:50 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> 
>> On 02-May-19 3:13 PM, Reshma Pattan wrote:
>>> gcc 9 on Fedora 30 gives an error
>>> "taking address of packed member may result in an
>>> unaligned pointer value" for -Waddress-of-packed-member.
>>>
>>> Report it as warning instead of error to fix the build.
>>>
>>> Snippet of build before fix
>>> ...lib/librte_eal/linux/eal/eal_memalloc.c: In function ‘alloc_seg_walk’:
>>> ...lib/librte_eal/linux/eal/eal_memalloc.c:768:12: error: taking address
>>> of packed member of ‘struct rte_mem_config’ may result in an unaligned
>>> pointer value [-Werror=address-of-packed-member]
>>>     768 |  cur_msl = &mcfg->memsegs[msl_idx];
>>>         |            ^~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Snippet of build after fix
>>> ..lib/librte_eal/linux/eal/eal_memory.c: In function ‘remap_segment’:
>>> ..lib/librte_eal/linux/eal/eal_memory.c:685:9: warning: taking address
>>> of packed member of ‘struct rte_mem_config’ may result in an unaligned
>>> pointer value [-Waddress-of-packed-member]
>>>     685 |   msl = &mcfg->memsegs[msl_idx];
>>>    
>>
>> Fixing these would require an ABI break, because these are exposed
>> externally. Should we submit a deprecation notice for EAL?
> 
> 
> Ideally mem config and related structures would not be exposed in the
> API. Like lcore_config and eal_config it should be eal_private
> 

In a perfect world, that would've been the case. However, these are in 
shared memory structures that are used for multiprocess synchronization.
diff mbox series

Patch

diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index d8b99faf6..61032bbbc 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -87,5 +87,9 @@  WERROR_FLAGS += -Wimplicit-fallthrough=2
 WERROR_FLAGS += -Wno-format-truncation
 endif
 
+#Report "taking address of packed member may result
+#in an unaligned pointer value" issues as warnings.
+WERROR_FLAGS += -Wno-error=address-of-packed-member
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS