[dpdk-dev] [PATCH v2] mk: suppress error for reference of packed members

[dpdk-dev] [PATCH v2] mk: suppress error for reference of packed members

[Emmanuel Roullit]

Found with clang build:
error: taking address of packed member 'mlock' of class or structure
'rte_mem_config' may result in an unaligned pointer value
[-Werror,-Waddress-of-packed-member]

Fixes: 29361d4c91ed ("mk: fix build with clang < 3.5")
Fixes: b2bb3a5daaac ("mk: stop on warning only in developer build")

Signed-off-by: Emmanuel Roullit <emmanuel.roullit@gmail.com>
---

v2:
* Only apply compiler flag on clang >= 4.0

 mk/toolchain/clang/rte.toolchain-compat.mk | 4 ++++
 mk/toolchain/clang/rte.vars.mk             | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/mk/toolchain/clang/rte.toolchain-compat.mk b/mk/toolchain/clang/rte.toolchain-compat.mk
index b734413b6..153d2f005 100644
--- a/mk/toolchain/clang/rte.toolchain-compat.mk
+++ b/mk/toolchain/clang/rte.toolchain-compat.mk
@@ -47,3 +47,7 @@ CLANG_MINOR_VERSION := $(shell echo $(CLANG_VERSION) | cut -f2 -d.)
 ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -lt 35 && echo 1), 1)
 	CC_SUPPORTS_Z := false
 endif
+
+ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -ge 40 && echo 1), 1)
+	CC_PACKED_MEMBER_WERROR := true
+endif
diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk
index 7749b991b..a797b7aab 100644
--- a/mk/toolchain/clang/rte.vars.mk
+++ b/mk/toolchain/clang/rte.vars.mk
@@ -76,6 +76,10 @@ endif
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+ifeq ($(CC_PACKED_MEMBER_WERROR),true)
+WERROR_FLAGS += -Wno-address-of-packed-member
+endif
+
 # workaround clang bug with warning "missing field initializer" for "= {0}"
 WERROR_FLAGS += -Wno-missing-field-initializers
 
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2] mk: suppress error for reference of packed members

[Thomas Monjalon]

2017-01-24 22:04, Emmanuel Roullit:
> Found with clang build:
> error: taking address of packed member 'mlock' of class or structure
> 'rte_mem_config' may result in an unaligned pointer value
> [-Werror,-Waddress-of-packed-member]
> 
> Fixes: 29361d4c91ed ("mk: fix build with clang < 3.5")
> Fixes: b2bb3a5daaac ("mk: stop on warning only in developer build")

I think these references are not correct.
The bug is due to a new version of clang, so no Fixes: tag is needed.

You should put "clang 4" in the title.


> Signed-off-by: Emmanuel Roullit <emmanuel.roullit@gmail.com>
> ---
> 
> v2:
> * Only apply compiler flag on clang >= 4.0

Please use --in-reply-to for grouping v2 patches with v1.


> --- a/mk/toolchain/clang/rte.toolchain-compat.mk
> +++ b/mk/toolchain/clang/rte.toolchain-compat.mk
> +ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -ge 40 && echo 1), 1)
> +	CC_PACKED_MEMBER_WERROR := true
> +endif

> --- a/mk/toolchain/clang/rte.vars.mk
> +++ b/mk/toolchain/clang/rte.vars.mk
> +ifeq ($(CC_PACKED_MEMBER_WERROR),true)
> +WERROR_FLAGS += -Wno-address-of-packed-member
> +endif

So you are completely disabling the warning?
Can we assume this warning is of no interest?

Re: [dpdk-dev] [PATCH v2] mk: suppress error for reference of packed members

[Emmanuel Roullit]

Hi Thomas,

Thanks for the hints.
I will improve the patch and resubmit.
Comments are present inline.

Regards,

- Emmanuel

On Wed, Jan 25, 2017 at 11:08 AM, Thomas Monjalon <thomas.monjalon@6wind.com
> wrote:

> 2017-01-24 22:04, Emmanuel Roullit:
> > Found with clang build:
> > error: taking address of packed member 'mlock' of class or structure
> > 'rte_mem_config' may result in an unaligned pointer value
> > [-Werror,-Waddress-of-packed-member]
> >
> > Fixes: 29361d4c91ed ("mk: fix build with clang < 3.5")
> > Fixes: b2bb3a5daaac ("mk: stop on warning only in developer build")
>
> I think these references are not correct.
> The bug is due to a new version of clang, so no Fixes: tag is needed.
>
> You should put "clang 4" in the title.


>
> > Signed-off-by: Emmanuel Roullit <emmanuel.roullit@gmail.com>
> > ---
> >
> > v2:
> > * Only apply compiler flag on clang >= 4.0
>
> Please use --in-reply-to for grouping v2 patches with v1.
>
>
> > --- a/mk/toolchain/clang/rte.toolchain-compat.mk
> > +++ b/mk/toolchain/clang/rte.toolchain-compat.mk
> > +ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -ge 40
> && echo 1), 1)
> > +     CC_PACKED_MEMBER_WERROR := true
> > +endif
>
> > --- a/mk/toolchain/clang/rte.vars.mk
> > +++ b/mk/toolchain/clang/rte.vars.mk
> > +ifeq ($(CC_PACKED_MEMBER_WERROR),true)
> > +WERROR_FLAGS += -Wno-address-of-packed-member
> > +endif
>
> So you are completely disabling the warning?
> Can we assume this warning is of no interest?

Yes, this changeset disables the warning completely.
This warning is interesting as some architectures are sensitive to
unaligned memory access (SPARC and for some instructions on ARM).
ARM64 documentation says "all loads and stores support the use of unaligned
addresses". [1]
On ARMv7, unaligned access are supported except multiple store/load
(STM/LDM) instructions.

Here are the solutions I found to prevent this warning:
- We could remove all packed attributes
- We could add casts to (void*) which would mute the warning on specific
places
- We could disable the warning altogether with -Wno-address-of-packed-member

The first possibility would produce a major changeset with side-effects
which are hard to gauge (for me at least).
The second option would litter the code with casts. It would make a large
changeset and casts should be kept to a minimum.
The last one is the one I picked for its brevity and effectiveness.

More info on this warning here: https://reviews.llvm.org/rL278483

[1]:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch05s01s02.html


>