| Message ID | 20190828055425.24765-1-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] kbuild: refactor scripts/Makefile.extrawarn | expand |
On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Instead of the warning-[123] magic, let's accumulate compiler options > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > easier to understand what is going on in this file. > > This commit slightly changes the behavior, I think all of which are OK. > > [1] Currently, cc-option calls are needlessly evaluated. For example, > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > needs evaluating only when W=3, but it is actually evaluated for > W=1, W=2 as well. With this commit, only relevant cc-option calls > will be evaluated. This is a slight optimization. > > [2] Currently, unsupported level like W=4 is checked by: > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > This will no longer be checked, but I do not think it is a big > deal. > Hi Masahiro Yamada, thanks for your patch series. If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang, please rename the Kconfig into... KBUILD_ENABLE_EXTRA_CC_CHECKS ...or something similiar (and maybe with some notes in its Kconfig help-text?). Regards, - Sedat - > [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat, > Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2, > and W=3. With this commit, they will be warned only by W=1. I > think this is a more correct behavior since each warning belongs > to only one warning level. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------ > 1 file changed, 53 insertions(+), 51 deletions(-) > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > index a74ce2e3c33e..1fa53968e292 100644 > --- a/scripts/Makefile.extrawarn > +++ b/scripts/Makefile.extrawarn > @@ -1,14 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > # ========================================================================== > -# > # make W=... settings > -# > -# W=1 - warnings that may be relevant and does not occur too often > -# W=2 - warnings that occur quite often but may still be relevant > -# W=3 - the more obscure warnings, can most likely be ignored > -# > -# $(call cc-option, -W...) handles gcc -W.. options which > -# are not supported by all versions of the compiler > # ========================================================================== > > KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned) > @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line") > export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) > endif > > -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS > -warning- := $(empty) > +# > +# W=1 - warnings that may be relevant and does not occur too often > +# > +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > > -warning-1 := -Wextra -Wunused -Wno-unused-parameter > -warning-1 += -Wmissing-declarations > -warning-1 += -Wmissing-format-attribute > -warning-1 += -Wmissing-prototypes > -warning-1 += -Wold-style-definition > -warning-1 += -Wmissing-include-dirs > -warning-1 += $(call cc-option, -Wunused-but-set-variable) > -warning-1 += $(call cc-option, -Wunused-const-variable) > -warning-1 += $(call cc-option, -Wpacked-not-aligned) > -warning-1 += $(call cc-option, -Wstringop-truncation) > +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter > +KBUILD_CFLAGS += -Wmissing-declarations > +KBUILD_CFLAGS += -Wmissing-format-attribute > +KBUILD_CFLAGS += -Wmissing-prototypes > +KBUILD_CFLAGS += -Wold-style-definition > +KBUILD_CFLAGS += -Wmissing-include-dirs > +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) > +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) > # The following turn off the warnings enabled by -Wextra > -warning-1 += -Wno-missing-field-initializers > -warning-1 += -Wno-sign-compare > - > -warning-2 += -Wcast-align > -warning-2 += -Wdisabled-optimization > -warning-2 += -Wnested-externs > -warning-2 += -Wshadow > -warning-2 += $(call cc-option, -Wlogical-op) > -warning-2 += -Wmissing-field-initializers > -warning-2 += -Wsign-compare > -warning-2 += $(call cc-option, -Wmaybe-uninitialized) > -warning-2 += $(call cc-option, -Wunused-macros) > - > -warning-3 := -Wbad-function-cast > -warning-3 += -Wcast-qual > -warning-3 += -Wconversion > -warning-3 += -Wpacked > -warning-3 += -Wpadded > -warning-3 += -Wpointer-arith > -warning-3 += -Wredundant-decls > -warning-3 += -Wswitch-default > -warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > - > -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > - > -ifeq ("$(strip $(warning))","") > - $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > -endif > +KBUILD_CFLAGS += -Wno-missing-field-initializers > +KBUILD_CFLAGS += -Wno-sign-compare > > -KBUILD_CFLAGS += $(warning) > else > > +# W=1 also stops suppressing some warnings > + > ifdef CONFIG_CC_IS_CLANG > KBUILD_CFLAGS += -Wno-initializer-overrides > KBUILD_CFLAGS += -Wno-format > KBUILD_CFLAGS += -Wno-sign-compare > KBUILD_CFLAGS += -Wno-format-zero-length > endif > + > +endif > + > +# > +# W=2 - warnings that occur quite often but may still be relevant > +# > +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > + > +KBUILD_CFLAGS += -Wcast-align > +KBUILD_CFLAGS += -Wdisabled-optimization > +KBUILD_CFLAGS += -Wnested-externs > +KBUILD_CFLAGS += -Wshadow > +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op) > +KBUILD_CFLAGS += -Wmissing-field-initializers > +KBUILD_CFLAGS += -Wsign-compare > +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized) > +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros) > + > +endif > + > +# > +# W=3 - the more obscure warnings, can most likely be ignored > +# > +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > + > +KBUILD_CFLAGS += -Wbad-function-cast > +KBUILD_CFLAGS += -Wcast-qual > +KBUILD_CFLAGS += -Wconversion > +KBUILD_CFLAGS += -Wpacked > +KBUILD_CFLAGS += -Wpadded > +KBUILD_CFLAGS += -Wpointer-arith > +KBUILD_CFLAGS += -Wredundant-decls > +KBUILD_CFLAGS += -Wswitch-default > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat) > + > endif > -- > 2.17.1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190828055425.24765-1-yamada.masahiro%40socionext.com.
On Wed, Aug 28, 2019 at 9:20 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > Instead of the warning-[123] magic, let's accumulate compiler options > > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > > easier to understand what is going on in this file. > > > > This commit slightly changes the behavior, I think all of which are OK. > > > > [1] Currently, cc-option calls are needlessly evaluated. For example, > > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > > needs evaluating only when W=3, but it is actually evaluated for > > W=1, W=2 as well. With this commit, only relevant cc-option calls > > will be evaluated. This is a slight optimization. > > > > [2] Currently, unsupported level like W=4 is checked by: > > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > > This will no longer be checked, but I do not think it is a big > > deal. > > > > Hi Masahiro Yamada, > > thanks for your patch series. > > If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang, > please rename the Kconfig into... > > KBUILD_ENABLE_EXTRA_CC_CHECKS > > ...or something similiar (and maybe with some notes in its Kconfig help-text?). > I have tested both patches against recent kbuild-next and can boot on bare metal with clang. I have *not* passed any W= to my make, but I see that clang's W=1 kbuild-cflags are active. [ scripts/Makefile.extrawarn ] ifeq ("$(origin W)", "command line") export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) endif # # W=1 - warnings that may be relevant and does not occur too often # ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) [ ... ] KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1 else # W=1 also stops suppressing some warnings ifdef CONFIG_CC_IS_CLANG KBUILD_CFLAGS += -Wno-initializer-overrides KBUILD_CFLAGS += -Wno-format KBUILD_CFLAGS += -Wno-sign-compare KBUILD_CFLAGS += -Wno-format-zero-length endif # CONFIG_CC_IS_CLANG endif # KBUILD_ENABLE_EXTRA_GCC_CHECKS These clang KBUILD_CFLAGS are active independently of passing W=1. $ grep '\-Wno-initializer-overrides' build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt | wc -l 27195 So the above comment is misleading? Is W=1 activated by default? Or do I miss something? [ Documentation/kbuild/kbuild.rst ] KBUILD_ENABLE_EXTRA_GCC_CHECKS ------------------------------ If enabled over the make command line with "W=1", it turns on additional gcc -W... options for more extensive build-time checking. What about? KBUILD_CC_EXTRA_CHECKS (or KBUILD_EXTRA_CC_CHECKS) ------------------------------ If enabled over the make command line with "W=...", it turns on additional compiler warning options like -Wmissing-declarations for more extensive build-time checking. For more details see <Documentation/kbuild/kbuild.rst>. W=1 - warnings that may be relevant and does not occur too often W=1 - also stops suppressing some warnings W=2 - warnings that occur quite often but may still be relevant W=3 - the more obscure warnings, can most likely be ignored - Sedat -
> build-time checking. For more details see <Documentation/kbuild/kbuild.rst>.
Grrr.
s/ Documentation/kbuild/kbuild.rst / scripts/Makefile.extrawarn
- Sedat -
Something like that... [PATCH 1/2] kbuild: Improve extrawarn documentation [PATCH 2/2] kbuild: Rename extrawarn Kconfig to KBUILD_EXTRA_CC_CHECKS - Sedat -
On Wed, Aug 28, 2019 at 02:54:24PM +0900, Masahiro Yamada wrote: > Instead of the warning-[123] magic, let's accumulate compiler options > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > easier to understand what is going on in this file. > > This commit slightly changes the behavior, I think all of which are OK. > > [1] Currently, cc-option calls are needlessly evaluated. For example, > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > needs evaluating only when W=3, but it is actually evaluated for > W=1, W=2 as well. With this commit, only relevant cc-option calls > will be evaluated. This is a slight optimization. > > [2] Currently, unsupported level like W=4 is checked by: > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > This will no longer be checked, but I do not think it is a big > deal. > > [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat, > Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2, > and W=3. With this commit, they will be warned only by W=1. I > think this is a more correct behavior since each warning belongs > to only one warning level. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
On Wed, Aug 28, 2019 at 12:20 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > Instead of the warning-[123] magic, let's accumulate compiler options > > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > > easier to understand what is going on in this file. > > > > This commit slightly changes the behavior, I think all of which are OK. > > > > [1] Currently, cc-option calls are needlessly evaluated. For example, > > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > > needs evaluating only when W=3, but it is actually evaluated for > > W=1, W=2 as well. With this commit, only relevant cc-option calls > > will be evaluated. This is a slight optimization. > > > > [2] Currently, unsupported level like W=4 is checked by: > > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > > This will no longer be checked, but I do not think it is a big > > deal. > > > > Hi Masahiro Yamada, > > thanks for your patch series. > > If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang, > please rename the Kconfig into... > > KBUILD_ENABLE_EXTRA_CC_CHECKS > > ...or something similiar (and maybe with some notes in its Kconfig help-text?). I too would like to see that changed.
On Tue, Aug 27, 2019 at 10:54 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Instead of the warning-[123] magic, let's accumulate compiler options > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > easier to understand what is going on in this file. > > This commit slightly changes the behavior, I think all of which are OK. > > [1] Currently, cc-option calls are needlessly evaluated. For example, > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > needs evaluating only when W=3, but it is actually evaluated for > W=1, W=2 as well. With this commit, only relevant cc-option calls > will be evaluated. This is a slight optimization. > > [2] Currently, unsupported level like W=4 is checked by: > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > This will no longer be checked, but I do not think it is a big > deal. > > [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat, > Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2, > and W=3. With this commit, they will be warned only by W=1. I > think this is a more correct behavior since each warning belongs > to only one warning level. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------ > 1 file changed, 53 insertions(+), 51 deletions(-) > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > index a74ce2e3c33e..1fa53968e292 100644 > --- a/scripts/Makefile.extrawarn > +++ b/scripts/Makefile.extrawarn > @@ -1,14 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > # ========================================================================== > -# > # make W=... settings > -# > -# W=1 - warnings that may be relevant and does not occur too often > -# W=2 - warnings that occur quite often but may still be relevant > -# W=3 - the more obscure warnings, can most likely be ignored > -# > -# $(call cc-option, -W...) handles gcc -W.. options which > -# are not supported by all versions of the compiler > # ========================================================================== > > KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned) > @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line") > export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) > endif > > -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS > -warning- := $(empty) > +# > +# W=1 - warnings that may be relevant and does not occur too often > +# > +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > > -warning-1 := -Wextra -Wunused -Wno-unused-parameter > -warning-1 += -Wmissing-declarations > -warning-1 += -Wmissing-format-attribute > -warning-1 += -Wmissing-prototypes > -warning-1 += -Wold-style-definition > -warning-1 += -Wmissing-include-dirs > -warning-1 += $(call cc-option, -Wunused-but-set-variable) > -warning-1 += $(call cc-option, -Wunused-const-variable) > -warning-1 += $(call cc-option, -Wpacked-not-aligned) > -warning-1 += $(call cc-option, -Wstringop-truncation) > +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter > +KBUILD_CFLAGS += -Wmissing-declarations > +KBUILD_CFLAGS += -Wmissing-format-attribute > +KBUILD_CFLAGS += -Wmissing-prototypes > +KBUILD_CFLAGS += -Wold-style-definition > +KBUILD_CFLAGS += -Wmissing-include-dirs > +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) > +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) > # The following turn off the warnings enabled by -Wextra > -warning-1 += -Wno-missing-field-initializers > -warning-1 += -Wno-sign-compare > - > -warning-2 += -Wcast-align > -warning-2 += -Wdisabled-optimization > -warning-2 += -Wnested-externs > -warning-2 += -Wshadow > -warning-2 += $(call cc-option, -Wlogical-op) > -warning-2 += -Wmissing-field-initializers > -warning-2 += -Wsign-compare > -warning-2 += $(call cc-option, -Wmaybe-uninitialized) > -warning-2 += $(call cc-option, -Wunused-macros) > - > -warning-3 := -Wbad-function-cast > -warning-3 += -Wcast-qual > -warning-3 += -Wconversion > -warning-3 += -Wpacked > -warning-3 += -Wpadded > -warning-3 += -Wpointer-arith > -warning-3 += -Wredundant-decls > -warning-3 += -Wswitch-default > -warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > - > -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > - > -ifeq ("$(strip $(warning))","") > - $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > -endif > +KBUILD_CFLAGS += -Wno-missing-field-initializers > +KBUILD_CFLAGS += -Wno-sign-compare > > -KBUILD_CFLAGS += $(warning) > else > > +# W=1 also stops suppressing some warnings > + > ifdef CONFIG_CC_IS_CLANG > KBUILD_CFLAGS += -Wno-initializer-overrides > KBUILD_CFLAGS += -Wno-format > KBUILD_CFLAGS += -Wno-sign-compare > KBUILD_CFLAGS += -Wno-format-zero-length > endif I find this part of the patch exceedingly confusing, and I think it mistakenly changes the behavior of W=2, W=3, and W=4. If W != 1 && CC == clang, then disable some flags? What? So W=2,3,4 those are disabled, but at W=1 are not? Didn't the previous version set these unless any W= was set? > + > +endif > + > +# > +# W=2 - warnings that occur quite often but may still be relevant > +# > +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > + > +KBUILD_CFLAGS += -Wcast-align > +KBUILD_CFLAGS += -Wdisabled-optimization > +KBUILD_CFLAGS += -Wnested-externs > +KBUILD_CFLAGS += -Wshadow > +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op) > +KBUILD_CFLAGS += -Wmissing-field-initializers > +KBUILD_CFLAGS += -Wsign-compare > +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized) > +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros) > + > +endif > + > +# > +# W=3 - the more obscure warnings, can most likely be ignored > +# > +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > + > +KBUILD_CFLAGS += -Wbad-function-cast > +KBUILD_CFLAGS += -Wcast-qual > +KBUILD_CFLAGS += -Wconversion > +KBUILD_CFLAGS += -Wpacked > +KBUILD_CFLAGS += -Wpadded > +KBUILD_CFLAGS += -Wpointer-arith > +KBUILD_CFLAGS += -Wredundant-decls > +KBUILD_CFLAGS += -Wswitch-default > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat) > + > endif > -- > 2.17.1 >
On Thu, Aug 29, 2019 at 12:38 AM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > On Tue, Aug 27, 2019 at 10:54 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > Instead of the warning-[123] magic, let's accumulate compiler options > > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > > easier to understand what is going on in this file. > > > > This commit slightly changes the behavior, I think all of which are OK. > > > > [1] Currently, cc-option calls are needlessly evaluated. For example, > > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > > needs evaluating only when W=3, but it is actually evaluated for > > W=1, W=2 as well. With this commit, only relevant cc-option calls > > will be evaluated. This is a slight optimization. > > > > [2] Currently, unsupported level like W=4 is checked by: > > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > > This will no longer be checked, but I do not think it is a big > > deal. > > > > [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat, > > Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2, > > and W=3. With this commit, they will be warned only by W=1. I > > think this is a more correct behavior since each warning belongs > > to only one warning level. > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > --- > > > > scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------ > > 1 file changed, 53 insertions(+), 51 deletions(-) > > > > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn > > index a74ce2e3c33e..1fa53968e292 100644 > > --- a/scripts/Makefile.extrawarn > > +++ b/scripts/Makefile.extrawarn > > @@ -1,14 +1,6 @@ > > # SPDX-License-Identifier: GPL-2.0 > > # ========================================================================== > > -# > > # make W=... settings > > -# > > -# W=1 - warnings that may be relevant and does not occur too often > > -# W=2 - warnings that occur quite often but may still be relevant > > -# W=3 - the more obscure warnings, can most likely be ignored > > -# > > -# $(call cc-option, -W...) handles gcc -W.. options which > > -# are not supported by all versions of the compiler > > # ========================================================================== > > > > KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned) > > @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line") > > export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) > > endif > > > > -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS > > -warning- := $(empty) > > +# > > +# W=1 - warnings that may be relevant and does not occur too often > > +# > > +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > > > > -warning-1 := -Wextra -Wunused -Wno-unused-parameter > > -warning-1 += -Wmissing-declarations > > -warning-1 += -Wmissing-format-attribute > > -warning-1 += -Wmissing-prototypes > > -warning-1 += -Wold-style-definition > > -warning-1 += -Wmissing-include-dirs > > -warning-1 += $(call cc-option, -Wunused-but-set-variable) > > -warning-1 += $(call cc-option, -Wunused-const-variable) > > -warning-1 += $(call cc-option, -Wpacked-not-aligned) > > -warning-1 += $(call cc-option, -Wstringop-truncation) > > +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter > > +KBUILD_CFLAGS += -Wmissing-declarations > > +KBUILD_CFLAGS += -Wmissing-format-attribute > > +KBUILD_CFLAGS += -Wmissing-prototypes > > +KBUILD_CFLAGS += -Wold-style-definition > > +KBUILD_CFLAGS += -Wmissing-include-dirs > > +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) > > +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) > > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) > > +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) > > # The following turn off the warnings enabled by -Wextra > > -warning-1 += -Wno-missing-field-initializers > > -warning-1 += -Wno-sign-compare > > - > > -warning-2 += -Wcast-align > > -warning-2 += -Wdisabled-optimization > > -warning-2 += -Wnested-externs > > -warning-2 += -Wshadow > > -warning-2 += $(call cc-option, -Wlogical-op) > > -warning-2 += -Wmissing-field-initializers > > -warning-2 += -Wsign-compare > > -warning-2 += $(call cc-option, -Wmaybe-uninitialized) > > -warning-2 += $(call cc-option, -Wunused-macros) > > - > > -warning-3 := -Wbad-function-cast > > -warning-3 += -Wcast-qual > > -warning-3 += -Wconversion > > -warning-3 += -Wpacked > > -warning-3 += -Wpadded > > -warning-3 += -Wpointer-arith > > -warning-3 += -Wredundant-decls > > -warning-3 += -Wswitch-default > > -warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > > - > > -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > > -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > > -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) > > - > > -ifeq ("$(strip $(warning))","") > > - $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > > -endif > > +KBUILD_CFLAGS += -Wno-missing-field-initializers > > +KBUILD_CFLAGS += -Wno-sign-compare > > > > -KBUILD_CFLAGS += $(warning) > > else > > > > +# W=1 also stops suppressing some warnings > > + > > ifdef CONFIG_CC_IS_CLANG > > KBUILD_CFLAGS += -Wno-initializer-overrides > > KBUILD_CFLAGS += -Wno-format > > KBUILD_CFLAGS += -Wno-sign-compare > > KBUILD_CFLAGS += -Wno-format-zero-length > > endif > > I find this part of the patch exceedingly confusing, and I think it > mistakenly changes the behavior of W=2, W=3, and W=4. If W != 1 && CC > == clang, then disable some flags? What? So W=2,3,4 those are > disabled, but at W=1 are not? Didn't the previous version set these > unless any W= was set? > > > + > > +endif > > + > > +# > > +# W=2 - warnings that occur quite often but may still be relevant > > +# > > +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > > + > > +KBUILD_CFLAGS += -Wcast-align > > +KBUILD_CFLAGS += -Wdisabled-optimization > > +KBUILD_CFLAGS += -Wnested-externs > > +KBUILD_CFLAGS += -Wshadow > > +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op) > > +KBUILD_CFLAGS += -Wmissing-field-initializers > > +KBUILD_CFLAGS += -Wsign-compare > > +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized) > > +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros) > > + > > +endif > > + > > +# > > +# W=3 - the more obscure warnings, can most likely be ignored > > +# > > +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > > + > > +KBUILD_CFLAGS += -Wbad-function-cast > > +KBUILD_CFLAGS += -Wcast-qual > > +KBUILD_CFLAGS += -Wconversion > > +KBUILD_CFLAGS += -Wpacked > > +KBUILD_CFLAGS += -Wpadded > > +KBUILD_CFLAGS += -Wpointer-arith > > +KBUILD_CFLAGS += -Wredundant-decls > > +KBUILD_CFLAGS += -Wswitch-default > > +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat) > > + > > endif > > -- > > 2.17.1 > > > > > -- > Thanks, > ~Nick Desaulniers Just a quick test... ...with clang-9 (built with tc-build from llvm-project.git#release/9.x) $ mycompiler --version ClangBuiltLinux clang version 9.0.0 (git://github.com/llvm/llvm-project e82a53603ae3fed2215a44b5ac603db00a780c02) (based on LLVM 9.0.0) Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /home/sdi/src/llvm-toolchain/install/bin $ mylinker --version LLD 9.0.0 (git://github.com/llvm/llvm-project e82a53603ae3fed2215a44b5ac603db00a780c02) (compatible with GNU linkers) With each run (changing W=...), I stopped my build-script manually, that's why the numbers differ. [ NO W-N ] sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ; grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_NO-W-N | wc -l ; done [ Wno-initializer-overrides ] 178 [ Wno-format ] 178 [ Wno-sign-compare ] 178 [ Wno-format-zero-length ] 178 [ W=1 ] sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ; grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-1 | wc -l ; done [ Wno-initializer-overrides ] 0 [ Wno-format ] 169 [ Wno-sign-compare ] 169 [ Wno-format-zero-length ] 0 [ W=2 ] sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ; grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-2 | wc -l ; done [ Wno-initializer-overrides ] 129 [ Wno-format ] 129 [ Wno-sign-compare ] 129 [ Wno-format-zero-length ] 129 [ W=3 ] sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ; grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-3 | wc -l ; done [ Wno-initializer-overrides ] 114 [ Wno-format ] 114 [ Wno-sign-compare ] 114 [ Wno-format-zero-length ] 114 [ W=4 ] sdi@iniza:~/src/linux-kernel$ for i in Wno-initializer-overrides Wno-format Wno-sign-compare Wno-format-zero-length ; do echo [ $i ] ; grep $i build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt_W-4 | wc -l ; done [ Wno-initializer-overrides ] 495 [ Wno-format ] 495 [ Wno-sign-compare ] 495 [ Wno-format-zero-length ] 495 W=1 is not passing -Wno-initializer-overrides and -Wno-format-zero-length. Unsure if other KBUILD_CFLAGS in W=1 disables them. So, if it is desired to pass the CLANG extrawarn compiler-options to all W=... then I ask myself why the CLANG block is in the W=1 block only? So if CLANG extrawarn options are independent of any W=... make-option then I prefer to put it in a seperate block with an appropriate comment. According to the commit message W=4 is unsupported, but I can do a 'make V=1 W=4'. If I pass an unsupported value to W=... then I expect an warning or info or exit or whatever. My €0,02. - Sedat -
> So, if it is desired to pass the CLANG extrawarn compiler-options to > all W=... then I ask myself why the CLANG block is in the W=1 block > only? > So if CLANG extrawarn options are independent of any W=... make-option > then I prefer to put it in a seperate block with an appropriate > comment. > Maybe something like that (on top of the two patches I had sent). From: Sedat Dilek <sedat.dilek@credativ.de> Date: Thu, 29 Aug 2019 11:35:21 +0200 Subject: [PATCH 3/3] kbuild: Move extra warnings for Clang --- Documentation/kbuild/kbuild.rst | 5 +++-- scripts/Makefile.extrawarn | 21 ++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst index 3e65d32af875..fa9772ae2367 100644 --- a/Documentation/kbuild/kbuild.rst +++ b/Documentation/kbuild/kbuild.rst @@ -245,10 +245,11 @@ KBUILD_EXTRA_CC_CHECKS ------------------------------ If enabled over the make command line with "W=...", it turns on additional compiler warning options like -Wmissing-declarations for more extensive -build-time checking. For more details see <scripts/Makefile.extrawarn>. +build-time checking. +Some extra warning options are set for all W=... settings when using Clang. +For more details see <scripts/Makefile.extrawarn>. W=1 - warnings that may be relevant and does not occur too often -W=1 - also stops suppressing some warnings W=2 - warnings that occur quite often but may still be relevant W=3 - the more obscure warnings, can most likely be ignored diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 72677ee9f202..86c0f8ae7e35 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -5,6 +5,16 @@ KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned) +# +# W=... - stops suppressing some warnings when using Clang +# +ifdef CONFIG_CC_IS_CLANG +KBUILD_CFLAGS += -Wno-initializer-overrides +KBUILD_CFLAGS += -Wno-format +KBUILD_CFLAGS += -Wno-sign-compare +KBUILD_CFLAGS += -Wno-format-zero-length +endif + ifeq ("$(origin W)", "command line") export KBUILD_EXTRA_CC_CHECKS := $(W) endif @@ -30,17 +40,6 @@ KBUILD_CFLAGS += -Wno-sign-compare KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1 -else - -# W=1 - also stops suppressing some warnings - -ifdef CONFIG_CC_IS_CLANG -KBUILD_CFLAGS += -Wno-initializer-overrides -KBUILD_CFLAGS += -Wno-format -KBUILD_CFLAGS += -Wno-sign-compare -KBUILD_CFLAGS += -Wno-format-zero-length -endif - endif #
On Wed, Aug 28, 2019 at 11:19 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Wed, Aug 28, 2019 at 9:20 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > > > > Instead of the warning-[123] magic, let's accumulate compiler options > > > to KBUILD_CFLAGS directly as the top Makefile does. I think this makes > > > easier to understand what is going on in this file. > > > > > > This commit slightly changes the behavior, I think all of which are OK. > > > > > > [1] Currently, cc-option calls are needlessly evaluated. For example, > > > warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > > > needs evaluating only when W=3, but it is actually evaluated for > > > W=1, W=2 as well. With this commit, only relevant cc-option calls > > > will be evaluated. This is a slight optimization. > > > > > > [2] Currently, unsupported level like W=4 is checked by: > > > $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > > > This will no longer be checked, but I do not think it is a big > > > deal. > > > > > > > Hi Masahiro Yamada, > > > > thanks for your patch series. > > > > If KBUILD_ENABLE_EXTRA_GCC_CHECKS does extra(-warning)-checks for GCC and Clang, > > please rename the Kconfig into... You repeatedly mentioned "Kconfig" in your posts, where there is nothing related to Kconfig. > > > > KBUILD_ENABLE_EXTRA_CC_CHECKS You missed the fact this is already used not only for C compilers, but also for Device Tree compiler. (see scripts/Makefile.lib) One more thing, this is the environment variable that Kbuild officially supports. Keeping the backward compatibility is must. When I mentioned to rename this before, Arnd suggested to keep it as is. https://patchwork.kernel.org/patch/10172331/#21385013 I do not know whether he is still planning that rework, though. > > ...or something similiar (and maybe with some notes in its Kconfig help-text?). What did you mean by "Kconfig help-text" ? > > > > I have tested both patches against recent kbuild-next and can boot on > bare metal with clang. > > I have *not* passed any W= to my make, but I see that clang's W=1 > kbuild-cflags are active. > > [ scripts/Makefile.extrawarn ] > > ifeq ("$(origin W)", "command line") > export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) > endif > > # > # W=1 - warnings that may be relevant and does not occur too often > # > ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) > [ ... ] > KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1 > > else > > # W=1 also stops suppressing some warnings > > ifdef CONFIG_CC_IS_CLANG > KBUILD_CFLAGS += -Wno-initializer-overrides > KBUILD_CFLAGS += -Wno-format > KBUILD_CFLAGS += -Wno-sign-compare > KBUILD_CFLAGS += -Wno-format-zero-length > endif # CONFIG_CC_IS_CLANG > > endif # KBUILD_ENABLE_EXTRA_GCC_CHECKS > > These clang KBUILD_CFLAGS are active independently of passing W=1. > > $ grep '\-Wno-initializer-overrides' > build-log_5.3.0-rc6-2-amd64-cbl-asmgoto.txt | wc -l > 27195 > > So the above comment is misleading? > > Is W=1 activated by default? > > Or do I miss something? I won't comment back to your long analysis. Instead, I will post v2. I hope you will notice something. > [ Documentation/kbuild/kbuild.rst ] > > KBUILD_ENABLE_EXTRA_GCC_CHECKS > ------------------------------ > If enabled over the make command line with "W=1", it turns on additional > gcc -W... options for more extensive build-time checking. > > What about? > > KBUILD_CC_EXTRA_CHECKS (or KBUILD_EXTRA_CC_CHECKS) > ------------------------------ > If enabled over the make command line with "W=...", it turns on additional > compiler warning options like -Wmissing-declarations for more extensive > build-time checking. For more details see <Documentation/kbuild/kbuild.rst>. > > W=1 - warnings that may be relevant and does not occur too often > W=1 - also stops suppressing some warnings > W=2 - warnings that occur quite often but may still be relevant > W=3 - the more obscure warnings, can most likely be ignored > > - Sedat - -- Best Regards Masahiro Yamada
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index a74ce2e3c33e..1fa53968e292 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -1,14 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 # ========================================================================== -# # make W=... settings -# -# W=1 - warnings that may be relevant and does not occur too often -# W=2 - warnings that occur quite often but may still be relevant -# W=3 - the more obscure warnings, can most likely be ignored -# -# $(call cc-option, -W...) handles gcc -W.. options which -# are not supported by all versions of the compiler # ========================================================================== KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned) @@ -17,58 +9,68 @@ ifeq ("$(origin W)", "command line") export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) endif -ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS -warning- := $(empty) +# +# W=1 - warnings that may be relevant and does not occur too often +# +ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) -warning-1 := -Wextra -Wunused -Wno-unused-parameter -warning-1 += -Wmissing-declarations -warning-1 += -Wmissing-format-attribute -warning-1 += -Wmissing-prototypes -warning-1 += -Wold-style-definition -warning-1 += -Wmissing-include-dirs -warning-1 += $(call cc-option, -Wunused-but-set-variable) -warning-1 += $(call cc-option, -Wunused-const-variable) -warning-1 += $(call cc-option, -Wpacked-not-aligned) -warning-1 += $(call cc-option, -Wstringop-truncation) +KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter +KBUILD_CFLAGS += -Wmissing-declarations +KBUILD_CFLAGS += -Wmissing-format-attribute +KBUILD_CFLAGS += -Wmissing-prototypes +KBUILD_CFLAGS += -Wold-style-definition +KBUILD_CFLAGS += -Wmissing-include-dirs +KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) +KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) +KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) +KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) # The following turn off the warnings enabled by -Wextra -warning-1 += -Wno-missing-field-initializers -warning-1 += -Wno-sign-compare - -warning-2 += -Wcast-align -warning-2 += -Wdisabled-optimization -warning-2 += -Wnested-externs -warning-2 += -Wshadow -warning-2 += $(call cc-option, -Wlogical-op) -warning-2 += -Wmissing-field-initializers -warning-2 += -Wsign-compare -warning-2 += $(call cc-option, -Wmaybe-uninitialized) -warning-2 += $(call cc-option, -Wunused-macros) - -warning-3 := -Wbad-function-cast -warning-3 += -Wcast-qual -warning-3 += -Wconversion -warning-3 += -Wpacked -warning-3 += -Wpadded -warning-3 += -Wpointer-arith -warning-3 += -Wredundant-decls -warning-3 += -Wswitch-default -warning-3 += $(call cc-option, -Wpacked-bitfield-compat) - -warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) -warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) -warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS))) - -ifeq ("$(strip $(warning))","") - $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) -endif +KBUILD_CFLAGS += -Wno-missing-field-initializers +KBUILD_CFLAGS += -Wno-sign-compare -KBUILD_CFLAGS += $(warning) else +# W=1 also stops suppressing some warnings + ifdef CONFIG_CC_IS_CLANG KBUILD_CFLAGS += -Wno-initializer-overrides KBUILD_CFLAGS += -Wno-format KBUILD_CFLAGS += -Wno-sign-compare KBUILD_CFLAGS += -Wno-format-zero-length endif + +endif + +# +# W=2 - warnings that occur quite often but may still be relevant +# +ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) + +KBUILD_CFLAGS += -Wcast-align +KBUILD_CFLAGS += -Wdisabled-optimization +KBUILD_CFLAGS += -Wnested-externs +KBUILD_CFLAGS += -Wshadow +KBUILD_CFLAGS += $(call cc-option, -Wlogical-op) +KBUILD_CFLAGS += -Wmissing-field-initializers +KBUILD_CFLAGS += -Wsign-compare +KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized) +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros) + +endif + +# +# W=3 - the more obscure warnings, can most likely be ignored +# +ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),) + +KBUILD_CFLAGS += -Wbad-function-cast +KBUILD_CFLAGS += -Wcast-qual +KBUILD_CFLAGS += -Wconversion +KBUILD_CFLAGS += -Wpacked +KBUILD_CFLAGS += -Wpadded +KBUILD_CFLAGS += -Wpointer-arith +KBUILD_CFLAGS += -Wredundant-decls +KBUILD_CFLAGS += -Wswitch-default +KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat) + endif
Instead of the warning-[123] magic, let's accumulate compiler options to KBUILD_CFLAGS directly as the top Makefile does. I think this makes easier to understand what is going on in this file. This commit slightly changes the behavior, I think all of which are OK. [1] Currently, cc-option calls are needlessly evaluated. For example, warning-3 += $(call cc-option, -Wpacked-bitfield-compat) needs evaluating only when W=3, but it is actually evaluated for W=1, W=2 as well. With this commit, only relevant cc-option calls will be evaluated. This is a slight optimization. [2] Currently, unsupported level like W=4 is checked by: $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) This will no longer be checked, but I do not think it is a big deal. [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat, Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2, and W=3. With this commit, they will be warned only by W=1. I think this is a more correct behavior since each warning belongs to only one warning level. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- scripts/Makefile.extrawarn | 104 +++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 51 deletions(-)