2019-08-28 05:56:51

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

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 <[email protected]>
---

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


2019-08-28 05:57:21

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

GCC and Clang have different policy for -Wunused-function; GCC does not
warn unused static inline functions at all whereas Clang does if they
are defined in source files instead of included headers although it has
been suppressed since commit abb2ea7dfd82 ("compiler, clang: suppress
warning for unused static inline functions").

We often miss to delete unused functions where 'static inline' is used
in *.c files since there is no tool to detect them. Unused code remains
until somebody notices. For example, commit 075ddd75680f ("regulator:
core: remove unused rdev_get_supply()").

Let's remove __maybe_unused from the inline macro to allow Clang to
start finding unused static inline functions. For now, we do this only
for W=1 build since it is not a good idea to sprinkle warnings for the
normal build.

My initial attempt was to add -Wno-unused-function for no W=1 build
(https://lore.kernel.org/patchwork/patch/1120594/)

Nathan Chancellor pointed out that would weaken Clang's checks since
we would no longer get -Wunused-function without W=1. It is true GCC
would detect unused static non-inline functions, but it would weaken
Clang as a standalone compiler at least.

Here is a counter implementation. The current problem is, W=... only
controls compiler flags, which are globally effective. There is no way
to narrow the scope to only 'static inline' functions.

This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
the 'inline' macro.

This makes the code a bit uglier, so personally I do not want to carry
this forever. If we can manage to fix most of the warnings, we can
drop this entirely, then enable -Wunused-function all the time.

If you contribute to code clean-up, please run "make CC=clang W=1"
and check -Wunused-function warnings. You will find lots of unused
functions.

Some of them are false-positives because the call-sites are disabled
by #ifdef. I do not like to abuse the inline keyword for suppressing
unused-function warnings because it is intended to be a hint for the
compiler optimization. I prefer #ifdef around the definition, or
__maybe_unused if #ifdef would make the code too ugly.

Signed-off-by: Masahiro Yamada <[email protected]>
---

include/linux/compiler_types.h | 20 ++++++++++++++------
scripts/Makefile.extrawarn | 6 ++++++
2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 599c27b56c29..b056a40116da 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -130,10 +130,6 @@ struct ftrace_likely_data {

/*
* Force always-inline if the user requests it so via the .config.
- * GCC does not warn about unused static inline functions for
- * -Wunused-function. This turns out to avoid the need for complex #ifdef
- * directives. Suppress the warning in clang as well by using "unused"
- * function attribute, which is redundant but not harmful for gcc.
* Prefer gnu_inline, so that extern inline functions do not emit an
* externally visible function. This makes extern inline behave as per gnu89
* semantics rather than c99. This prevents multiple symbol definition errors
@@ -144,15 +140,27 @@ struct ftrace_likely_data {
*/
#if !defined(CONFIG_OPTIMIZE_INLINING)
#define inline inline __attribute__((__always_inline__)) __gnu_inline \
- __maybe_unused notrace
+ __inline_maybe_unused notrace
#else
#define inline inline __gnu_inline \
- __maybe_unused notrace
+ __inline_maybe_unused notrace
#endif

#define __inline__ inline
#define __inline inline

+/*
+ * GCC does not warn about unused static inline functions for -Wunused-function.
+ * Suppress the warning in clang as well by using __maybe_unused, but enable it
+ * for W=1 build. This will allow clang to find unused functions. Remove the
+ * __inline_maybe_unused entirely after fixing most of -Wunused-function warnings.
+ */
+#ifdef KBUILD_EXTRA_WARN1
+#define __inline_maybe_unused
+#else
+#define __inline_maybe_unused __maybe_unused
+#endif
+
/*
* Rather then using noinline to prevent stack consumption, use
* noinline_for_stack instead. For documentation reasons.
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 1fa53968e292..3af1770497fd 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -28,6 +28,8 @@ KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
KBUILD_CFLAGS += -Wno-missing-field-initializers
KBUILD_CFLAGS += -Wno-sign-compare

+KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
+
else

# W=1 also stops suppressing some warnings
@@ -56,6 +58,8 @@ KBUILD_CFLAGS += -Wsign-compare
KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)

+KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN2
+
endif

#
@@ -73,4 +77,6 @@ KBUILD_CFLAGS += -Wredundant-decls
KBUILD_CFLAGS += -Wswitch-default
KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)

+KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN3
+
endif
--
2.17.1

2019-08-28 07:22:10

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190828055425.24765-1-yamada.masahiro%40socionext.com.

2019-08-28 14:20:58

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

On Wed, Aug 28, 2019 at 9:20 AM Sedat Dilek <[email protected]> wrote:
>
> On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada
> <[email protected]> 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 -

2019-08-28 14:23:26

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

> build-time checking. For more details see <Documentation/kbuild/kbuild.rst>.

Grrr.

s/ Documentation/kbuild/kbuild.rst / scripts/Makefile.extrawarn

- Sedat -

2019-08-28 16:01:55

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

Something like that...

[PATCH 1/2] kbuild: Improve extrawarn documentation
[PATCH 2/2] kbuild: Rename extrawarn Kconfig to KBUILD_EXTRA_CC_CHECKS

- Sedat -


Attachments:
0001-kbuild-Improve-extrawarn-documentation.patch (1.72 kB)
0002-kbuild-Rename-extrawarn-Kconfig-to-KBUILD_EXTRA_CC_C.patch (4.01 kB)
Download all attachments

2019-08-28 18:20:05

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

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 <[email protected]>

Reviewed-by: Nathan Chancellor <[email protected]>

2019-08-28 18:23:00

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

On Wed, Aug 28, 2019 at 02:54:25PM +0900, Masahiro Yamada wrote:
> GCC and Clang have different policy for -Wunused-function; GCC does not
> warn unused static inline functions at all whereas Clang does if they
> are defined in source files instead of included headers although it has
> been suppressed since commit abb2ea7dfd82 ("compiler, clang: suppress
> warning for unused static inline functions").
>
> We often miss to delete unused functions where 'static inline' is used
> in *.c files since there is no tool to detect them. Unused code remains
> until somebody notices. For example, commit 075ddd75680f ("regulator:
> core: remove unused rdev_get_supply()").
>
> Let's remove __maybe_unused from the inline macro to allow Clang to
> start finding unused static inline functions. For now, we do this only
> for W=1 build since it is not a good idea to sprinkle warnings for the
> normal build.
>
> My initial attempt was to add -Wno-unused-function for no W=1 build
> (https://lore.kernel.org/patchwork/patch/1120594/)
>
> Nathan Chancellor pointed out that would weaken Clang's checks since
> we would no longer get -Wunused-function without W=1. It is true GCC
> would detect unused static non-inline functions, but it would weaken
> Clang as a standalone compiler at least.
>
> Here is a counter implementation. The current problem is, W=... only
> controls compiler flags, which are globally effective. There is no way
> to narrow the scope to only 'static inline' functions.
>
> This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
> When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
> the 'inline' macro.
>
> This makes the code a bit uglier, so personally I do not want to carry
> this forever. If we can manage to fix most of the warnings, we can
> drop this entirely, then enable -Wunused-function all the time.
>
> If you contribute to code clean-up, please run "make CC=clang W=1"
> and check -Wunused-function warnings. You will find lots of unused
> functions.
>
> Some of them are false-positives because the call-sites are disabled
> by #ifdef. I do not like to abuse the inline keyword for suppressing
> unused-function warnings because it is intended to be a hint for the
> compiler optimization. I prefer #ifdef around the definition, or
> __maybe_unused if #ifdef would make the code too ugly.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

I can still see warnings from static unused functions and with W=1, I
see plenty more. I agree that this is uglier because of the
__inline_maybe_unused but I think this is better for regular developers.
I will try to work on these unused-function warnings!

Reviewed-by: Nathan Chancellor <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>

2019-08-28 18:27:29

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

On Wed, Aug 28, 2019 at 12:20 AM Sedat Dilek <[email protected]> wrote:
>
> On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada
> <[email protected]> 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.

--
Thanks,
~Nick Desaulniers

2019-08-28 22:39:52

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

On Tue, Aug 27, 2019 at 10:54 PM Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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

2019-08-28 23:29:53

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

On Wed, Aug 28, 2019 at 11:20 AM Nathan Chancellor
<[email protected]> wrote:
>
> On Wed, Aug 28, 2019 at 02:54:25PM +0900, Masahiro Yamada wrote:
> > GCC and Clang have different policy for -Wunused-function; GCC does not
> > warn unused static inline functions at all whereas Clang does if they
> > are defined in source files instead of included headers although it has
> > been suppressed since commit abb2ea7dfd82 ("compiler, clang: suppress
> > warning for unused static inline functions").
> >
> > We often miss to delete unused functions where 'static inline' is used
> > in *.c files since there is no tool to detect them. Unused code remains
> > until somebody notices. For example, commit 075ddd75680f ("regulator:
> > core: remove unused rdev_get_supply()").
> >
> > Let's remove __maybe_unused from the inline macro to allow Clang to
> > start finding unused static inline functions. For now, we do this only
> > for W=1 build since it is not a good idea to sprinkle warnings for the
> > normal build.
> >
> > My initial attempt was to add -Wno-unused-function for no W=1 build
> > (https://lore.kernel.org/patchwork/patch/1120594/)
> >
> > Nathan Chancellor pointed out that would weaken Clang's checks since
> > we would no longer get -Wunused-function without W=1. It is true GCC
> > would detect unused static non-inline functions, but it would weaken
> > Clang as a standalone compiler at least.

Got it. No problem.

> >
> > Here is a counter implementation. The current problem is, W=... only
> > controls compiler flags, which are globally effective. There is no way
> > to narrow the scope to only 'static inline' functions.
> >
> > This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
> > When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
> > the 'inline' macro.
> >
> > This makes the code a bit uglier, so personally I do not want to carry
> > this forever. If we can manage to fix most of the warnings, we can
> > drop this entirely, then enable -Wunused-function all the time.

How many warnings?

> >
> > If you contribute to code clean-up, please run "make CC=clang W=1"
> > and check -Wunused-function warnings. You will find lots of unused
> > functions.
> >
> > Some of them are false-positives because the call-sites are disabled
> > by #ifdef. I do not like to abuse the inline keyword for suppressing
> > unused-function warnings because it is intended to be a hint for the
> > compiler optimization. I prefer #ifdef around the definition, or
> > __maybe_unused if #ifdef would make the code too ugly.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
>
> I can still see warnings from static unused functions and with W=1, I
> see plenty more. I agree that this is uglier because of the
> __inline_maybe_unused but I think this is better for regular developers.
> I will try to work on these unused-function warnings!

How many are we talking here?

>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]>

This is getting kind of messy. I was more ok when the goal seemed to
be simplifying the definition of `inline`, but this is worse IMO.

--
Thanks,
~Nick Desaulniers

2019-08-29 00:08:28

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

On Wed, Aug 28, 2019 at 04:28:30PM -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 11:20 AM Nathan Chancellor
> <[email protected]> wrote:
> >
> > On Wed, Aug 28, 2019 at 02:54:25PM +0900, Masahiro Yamada wrote:
> > > GCC and Clang have different policy for -Wunused-function; GCC does not
> > > warn unused static inline functions at all whereas Clang does if they
> > > are defined in source files instead of included headers although it has
> > > been suppressed since commit abb2ea7dfd82 ("compiler, clang: suppress
> > > warning for unused static inline functions").
> > >
> > > We often miss to delete unused functions where 'static inline' is used
> > > in *.c files since there is no tool to detect them. Unused code remains
> > > until somebody notices. For example, commit 075ddd75680f ("regulator:
> > > core: remove unused rdev_get_supply()").
> > >
> > > Let's remove __maybe_unused from the inline macro to allow Clang to
> > > start finding unused static inline functions. For now, we do this only
> > > for W=1 build since it is not a good idea to sprinkle warnings for the
> > > normal build.
> > >
> > > My initial attempt was to add -Wno-unused-function for no W=1 build
> > > (https://lore.kernel.org/patchwork/patch/1120594/)
> > >
> > > Nathan Chancellor pointed out that would weaken Clang's checks since
> > > we would no longer get -Wunused-function without W=1. It is true GCC
> > > would detect unused static non-inline functions, but it would weaken
> > > Clang as a standalone compiler at least.
>
> Got it. No problem.
>
> > >
> > > Here is a counter implementation. The current problem is, W=... only
> > > controls compiler flags, which are globally effective. There is no way
> > > to narrow the scope to only 'static inline' functions.
> > >
> > > This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
> > > When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
> > > the 'inline' macro.
> > >
> > > This makes the code a bit uglier, so personally I do not want to carry
> > > this forever. If we can manage to fix most of the warnings, we can
> > > drop this entirely, then enable -Wunused-function all the time.
>
> How many warnings?

In an x86 defconfig build (one of the smallest builds we do), I see an
additional 35 warnings that crop up:

https://gist.github.com/003ba86ba60b4ac7e8109089d6cb1a5a

> > >
> > > If you contribute to code clean-up, please run "make CC=clang W=1"
> > > and check -Wunused-function warnings. You will find lots of unused
> > > functions.
> > >
> > > Some of them are false-positives because the call-sites are disabled
> > > by #ifdef. I do not like to abuse the inline keyword for suppressing
> > > unused-function warnings because it is intended to be a hint for the
> > > compiler optimization. I prefer #ifdef around the definition, or
> > > __maybe_unused if #ifdef would make the code too ugly.
> > >
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> >
> > I can still see warnings from static unused functions and with W=1, I
> > see plenty more. I agree that this is uglier because of the
> > __inline_maybe_unused but I think this is better for regular developers.
> > I will try to work on these unused-function warnings!
>
> How many are we talking here?
>
> >
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > Tested-by: Nathan Chancellor <[email protected]>
>
> This is getting kind of messy. I was more ok when the goal seemed to
> be simplifying the definition of `inline`, but this is worse IMO.

I guess if you want, we can just go back to v1 and have all unused
function warnings hidden by default with clang. Fixing these warnings
will take a significant amount of time given there will probably be a
few hundred so I don't think having this warning hidden behind W=1 for
that long is a good thing.

Cheers,
Nathan

2019-08-29 08:51:11

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

On Thu, Aug 29, 2019 at 12:38 AM 'Nick Desaulniers' via Clang Built
Linux <[email protected]> wrote:
>
> On Tue, Aug 27, 2019 at 10:54 PM Masahiro Yamada
> <[email protected]> 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 <[email protected]>
> > ---
> >
> > 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 -

2019-08-29 09:56:00

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

> 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 <[email protected]>
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

#
--
2.20.1

- Sedat -

2019-08-29 17:58:06

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: refactor scripts/Makefile.extrawarn

On Wed, Aug 28, 2019 at 11:19 PM Sedat Dilek <[email protected]> wrote:
>
> On Wed, Aug 28, 2019 at 9:20 AM Sedat Dilek <[email protected]> wrote:
> >
> > On Wed, Aug 28, 2019 at 7:55 AM Masahiro Yamada
> > <[email protected]> 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

2019-08-30 07:09:39

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

On Thu, Aug 29, 2019 at 1:28 AM 'Nick Desaulniers' via Clang Built
Linux <[email protected]> wrote:
[...]
> > >
> > > Here is a counter implementation. The current problem is, W=... only
> > > controls compiler flags, which are globally effective. There is no way
> > > to narrow the scope to only 'static inline' functions.
> > >
> > > This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
> > > When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
> > > the 'inline' macro.
> > >
> > > This makes the code a bit uglier, so personally I do not want to carry
> > > this forever. If we can manage to fix most of the warnings, we can
> > > drop this entirely, then enable -Wunused-function all the time.
>
> How many warnings?
>

I tried or adapted this 2-2 patch with v2 of 1-2 patch which is in
kbuild.git#kbuild (see below [1]).

$ grep warning: build-log.txt | grep '\[-Wunused-function]' | wc -l
214

$ grep warning: build-log.txt | grep '\[-Wunused-function]' | sort
arch/x86/kernel/apic/io_apic.c:162:19: warning: unused function
'mp_init_irq_at_boot' [-Wunused-function]
arch/x86/kernel/cpu/common.c:298:19: warning: unused function
'flag_is_changeable_p' [-Wunused-function]
arch/x86/kvm/lapic.c:302:19: warning: unused function
'apic_lvt_vector' [-Wunused-function]
arch/x86/xen/p2m.c:137:24: warning: unused function 'p2m_index'
[-Wunused-function]
block/blk-zoned.c:23:24: warning: unused function 'blk_zone_start'
[-Wunused-function]
block/partitions/mac.c:23:20: warning: unused function
'mac_fix_string' [-Wunused-function]
drivers/acpi/ec.c:2047:20: warning: unused function
'acpi_ec_query_exit' [-Wunused-function]
drivers/atm/horizon.c:465:20: warning: unused function 'dump_regs'
[-Wunused-function]
drivers/atm/horizon.c:479:20: warning: unused function 'dump_framer'
[-Wunused-function]
drivers/atm/idt77252.c:1786:1: warning: unused function
'idt77252_fbq_level' [-Wunused-function]
drivers/cpufreq/intel_pstate.c:77:23: warning: unused function
'percent_fp' [-Wunused-function]
drivers/cpufreq/intel_pstate.c:92:23: warning: unused function
'percent_ext_fp' [-Wunused-function]
drivers/crypto/chelsio/chcr_algo.c:129:19: warning: unused function
'is_ofld_imm' [-Wunused-function]
drivers/crypto/qat/qat_common/qat_asym_algs.c:252:34: warning: unused
function 'qat_dh_get_params' [-Wunused-function]
drivers/dma/ioat/dca.c:44:19: warning: unused function
'dca2_tag_map_valid' [-Wunused-function]
drivers/edac/i5100_edac.c:247:19: warning: unused function
'i5100_nrecmema_dm_buf_id' [-Wunused-function]
drivers/gpu/drm/drm_mm.c:155:1: warning: unused function
'drm_mm_interval_tree_insert' [-Wunused-function]
drivers/gpu/drm/drm_mm.c:155:1: warning: unused function
'drm_mm_interval_tree_iter_next' [-Wunused-function]
drivers/gpu/drm/drm_mm.c:294:19: warning: unused function
'rb_hole_size' [-Wunused-function]
drivers/gpu/drm/gma500/psb_drv.c:400:20: warning: unused function
'get_brightness' [-Wunused-function]
drivers/gpu/drm/gma500/psb_irq.c:49:1: warning: unused function
'mid_pipe_vsync' [-Wunused-function]
drivers/gpu/drm/i915/display/intel_hdmi.c:1696:26: warning: unused
function 'intel_hdmi_hdcp2_protocol' [-Wunused-function]
drivers/gpu/drm/i915/i915_sw_fence.c:105:20: warning: unused function
'debug_fence_free' [-Wunused-function]
drivers/gpu/drm/i915/i915_sw_fence.c:84:20: warning: unused function
'debug_fence_init_onstack' [-Wunused-function]
drivers/gpu/drm/i915/intel_guc_submission.c:1117:20: warning: unused
function 'ctx_save_restore_disabled' [-Wunused-function]
drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c:58:35: warning: unused
function 'vmw_overlay' [-Wunused-function]
drivers/hv/ring_buffer.c:89:1: warning: unused function
'hv_set_next_read_location' [-Wunused-function]
drivers/hwmon/nct6683.c:485:19: warning: unused function 'in_to_reg'
[-Wunused-function]
drivers/hwmon/sis5595.c:158:18: warning: unused function 'DIV_TO_REG'
[-Wunused-function]
drivers/hwmon/vt1211.c:198:20: warning: unused function 'superio_outb'
[-Wunused-function]
drivers/infiniband/core/cm.c:1528:19: warning: unused function
'cm_is_active_peer' [-Wunused-function]
drivers/infiniband/core/device.c:2747:1: warning: unused function
'__chk_RDMA_NL_LS' [-Wunused-function]
drivers/infiniband/core/iwcm.c:1208:1: warning: unused function
'__chk_RDMA_NL_IWCM' [-Wunused-function]
drivers/infiniband/core/nldev.c:2105:1: warning: unused function
'__chk_RDMA_NL_NLDEV' [-Wunused-function]
drivers/infiniband/hw/qib/qib_iba7322.c:803:19: warning: unused
function 'qib_read_ureg' [-Wunused-function]
drivers/infiniband/sw/rdmavt/vt.c:287:36: warning: unused function
'to_iucontext' [-Wunused-function]
drivers/isdn/hardware/mISDN/hfcmulti.c:667:1: warning: unused function
'vpm_read_address' [-Wunused-function]
drivers/leds/leds-pca955x.c:140:19: warning: unused function
'pca95xx_num_led_regs' [-Wunused-function]
drivers/md/raid0.c:444:19: warning: unused function
'is_io_in_chunk_boundary' [-Wunused-function]
drivers/media/dvb-frontends/drxk_hard.c:159:19: warning: unused
function 'MulDiv32' [-Wunused-function]
drivers/media/dvb-frontends/stb6100.c:113:20: warning: unused function
'stb6100_normalise_regs' [-Wunused-function]
drivers/media/i2c/cs3308.c:30:19: warning: unused function
'cs3308_read' [-Wunused-function]
drivers/media/i2c/cx25840/cx25840-ir.c:139:19: warning: unused
function 'ns_to_clock_divider' [-Wunused-function]
drivers/media/i2c/cx25840/cx25840-ir.c:145:28: warning: unused
function 'clock_divider_to_ns' [-Wunused-function]
drivers/media/i2c/cx25840/cx25840-ir.c:163:19: warning: unused
function 'freq_to_clock_divider' [-Wunused-function]
drivers/media/mc/mc-entity.c:17:27: warning: unused function
'gobj_type' [-Wunused-function]
drivers/media/pci/cx18/cx18-alsa-main.c:56:23: warning: unused
function 'p_to_snd_cx18_card' [-Wunused-function]
drivers/media/pci/cx23885/cx23888-ir.c:178:19: warning: unused
function 'ns_to_clock_divider' [-Wunused-function]
drivers/media/pci/cx23885/cx23888-ir.c:184:28: warning: unused
function 'clock_divider_to_ns' [-Wunused-function]
drivers/media/pci/cx23885/cx23888-ir.c:202:19: warning: unused
function 'freq_to_clock_divider' [-Wunused-function]
drivers/media/pci/ivtv/ivtv-alsa-main.c:53:23: warning: unused
function 'p_to_snd_ivtv_card' [-Wunused-function]
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c:394:19: warning: unused
function 'vop_interlaced' [-Wunused-function]
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c:399:18: warning: unused
function 'vop_channel' [-Wunused-function]
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c:414:18: warning: unused
function 'vop_hsize' [-Wunused-function]
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c:419:18: warning: unused
function 'vop_vsize' [-Wunused-function]
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c:439:19: warning: unused
function 'vop_sec' [-Wunused-function]
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c:444:19: warning: unused
function 'vop_usec' [-Wunused-function]
drivers/media/rc/fintek-cir.c:55:20: warning: unused function
'fintek_clear_reg_bit' [-Wunused-function]
drivers/media/rc/nuvoton-cir.c:78:20: warning: unused function
'nvt_clear_reg_bit' [-Wunused-function]
drivers/media/usb/au0828/au0828-i2c.c:26:19: warning: unused function
'i2c_slave_did_write_ack' [-Wunused-function]
drivers/media/usb/usbvision/usbvision-video.c:145:37: warning: unused
function 'cd_to_usbvision' [-Wunused-function]
drivers/misc/hpilo.c:396:19: warning: unused function
'is_device_reset' [-Wunused-function]
drivers/mmc/host/sdricoh_cs.c:104:28: warning: unused function
'sdricoh_readw' [-Wunused-function]
drivers/net/ethernet/atheros/atl1c/atl1c_main.c:184:20: warning:
unused function 'atl1c_irq_reset' [-Wunused-function]
drivers/net/ethernet/broadcom/b44.c:201:20: warning: unused function
'__b44_cam_read' [-Wunused-function]
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c:334:20: warning:
unused function 'bnx2x_vf_vlan_credit' [-Wunused-function]
drivers/net/ethernet/cavium/liquidio/request_manager.c:43:19: warning:
unused function 'IQ_INSTR_MODE_64B' [-Wunused-function]
drivers/net/ethernet/chelsio/cxgb3/sge.c:167:32: warning: unused
function 'fl_to_qset' [-Wunused-function]
drivers/net/ethernet/chelsio/cxgb4/sge.c:857:28: warning: unused
function 'calc_tx_descs' [-Wunused-function]
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2952:20: warning: unused
function 'ixgbe_irq_disable_queues' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:1546:20: warning: unused
function 'hw_ena_intr_bit' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2120:19: warning: unused
function 'port_chk_broad_storm' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2278:20: warning: unused
function 'port_cfg_force_flow_ctrl' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2284:19: warning: unused
function 'port_chk_back_pressure' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2290:19: warning: unused
function 'port_chk_force_flow_ctrl' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2298:20: warning: unused
function 'port_cfg_rx' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2304:20: warning: unused
function 'port_cfg_tx' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2315:20: warning: unused
function 'sw_flush_dyn_mac_table' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2326:20: warning: unused
function 'port_cfg_ins_tag' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2332:20: warning: unused
function 'port_cfg_rmv_tag' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2338:19: warning: unused
function 'port_chk_ins_tag' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2344:19: warning: unused
function 'port_chk_rmv_tag' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2350:20: warning: unused
function 'port_cfg_dis_non_vid' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2356:20: warning: unused
function 'port_cfg_in_filter' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2362:19: warning: unused
function 'port_chk_dis_non_vid' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2368:19: warning: unused
function 'port_chk_in_filter' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2411:20: warning: unused
function 'sw_cfg_unk_def_deliver' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2417:19: warning: unused
function 'sw_cfg_chk_unk_def_deliver' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2423:20: warning: unused
function 'sw_cfg_unk_def_port' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2428:19: warning: unused
function 'sw_chk_unk_def_port' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2459:19: warning: unused
function 'port_chk_diffserv' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2465:19: warning: unused
function 'port_chk_802_1p' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2471:19: warning: unused
function 'port_chk_replace_vid' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2477:19: warning: unused
function 'port_chk_prio' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2689:20: warning: unused
function 'sw_get_addr' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2906:20: warning: unused
function 'hw_r_phy_link_stat' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2911:20: warning: unused
function 'hw_r_phy_auto_neg' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2916:20: warning: unused
function 'hw_w_phy_auto_neg' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2921:20: warning: unused
function 'hw_r_phy_rem_cap' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2926:20: warning: unused
function 'hw_r_phy_crossover' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2931:20: warning: unused
function 'hw_w_phy_crossover' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2936:20: warning: unused
function 'hw_r_phy_polarity' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2941:20: warning: unused
function 'hw_w_phy_polarity' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2946:20: warning: unused
function 'hw_r_phy_link_md' [-Wunused-function]
drivers/net/ethernet/micrel/ksz884x.c:2951:20: warning: unused
function 'hw_w_phy_link_md' [-Wunused-function]
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1177:20: warning:
unused function 'myri10ge_vlan_ip_csum' [-Wunused-function]
drivers/net/ethernet/sun/cassini.c:240:20: warning: unused function
'cas_lock_all' [-Wunused-function]
drivers/net/ethernet/sun/cassini.c:269:20: warning: unused function
'cas_unlock_all' [-Wunused-function]
drivers/net/ethernet/tehuti/tehuti.c:1368:19: warning: unused function
'bdx_tx_db_size' [-Wunused-function]
drivers/net/usb/plusb.c:67:1: warning: unused function
'pl_clear_QuickLink_features' [-Wunused-function]
drivers/net/usb/sierra_net.c:357:19: warning: unused function
'sierra_net_is_valid_addrlen' [-Wunused-function]
drivers/net/wireless/ath/ath10k/ce.c:127:1: warning: unused function
'ath10k_get_ring_byte' [-Wunused-function]
drivers/net/wireless/ath/ath10k/ce.c:212:1: warning: unused function
'ath10k_ce_shadow_dest_ring_write_index_set' [-Wunused-function]
drivers/net/wireless/ath/ath10k/ce.c:449:20: warning: unused function
'ath10k_ce_error_intr_enable' [-Wunused-function]
drivers/net/wireless/broadcom/b43legacy/dma.c:130:19: warning: unused
function 'prev_slot' [-Wunused-function]
drivers/net/wireless/broadcom/b43legacy/dma.c:217:19: warning: unused
function 'txring_to_priority' [-Wunused-function]
drivers/net/wireless/broadcom/b43legacy/radio.c:1713:5: warning:
unused function 'freq_r3A_value' [-Wunused-function]
drivers/net/wireless/intel/ipw2x00/ipw2200.c:3010:19: warning: unused
function 'ipw_alive' [-Wunused-function]
drivers/net/wireless/intel/ipw2x00/ipw2200.c:381:19: warning: unused
function '_ipw_read16' [-Wunused-function]
drivers/net/wireless/intel/iwlegacy/3945.c:226:1: warning: unused
function 'il3945_get_tx_fail_reason' [-Wunused-function]
drivers/net/wireless/intel/iwlwifi/mvm/sta.c:1370:18: warning: unused
function 'iwl_mvm_tid_to_ac_queue' [-Wunused-function]
drivers/net/wireless/realtek/rtw88/pci.c:81:21: warning: unused
function 'rtw_pci_get_tx_desc' [-Wunused-function]
drivers/nvme/target/fc.c:151:1: warning: unused function
'nvmet_fc_iodnum' [-Wunused-function]
drivers/nvme/target/fc.c:157:1: warning: unused function
'nvmet_fc_fodnum' [-Wunused-function]
drivers/pci/hotplug/shpchp_hpc.c:177:20: warning: unused function
'shpc_writeb' [-Wunused-function]
drivers/pcmcia/yenta_socket.c:147:18: warning: unused function
'exca_readw' [-Wunused-function]
drivers/scsi/aacraid/aachba.c:1880:19: warning: unused function
'aac_get_safw_phys_device_type' [-Wunused-function]
drivers/scsi/cxgbi/libcxgbi.c:2287:19: warning: unused function
'csk_print_port' [-Wunused-function]
drivers/scsi/cxgbi/libcxgbi.c:2298:19: warning: unused function
'csk_print_ip' [-Wunused-function]
drivers/scsi/myrb.c:2557:20: warning: unused function
'DAC960_LA_gen_intr' [-Wunused-function]
drivers/scsi/myrb.c:2591:20: warning: unused function
'DAC960_LA_ack_mem_mbox_intr' [-Wunused-function]
drivers/scsi/myrb.c:2609:20: warning: unused function
'DAC960_LA_mem_mbox_status_available' [-Wunused-function]
drivers/scsi/myrb.c:2632:20: warning: unused function
'DAC960_LA_intr_enabled' [-Wunused-function]
drivers/scsi/myrb.c:2661:29: warning: unused function
'DAC960_LA_read_status_cmd_ident' [-Wunused-function]
drivers/scsi/myrb.c:2834:20: warning: unused function
'DAC960_PG_gen_intr' [-Wunused-function]
drivers/scsi/myrb.c:2868:20: warning: unused function
'DAC960_PG_ack_mem_mbox_intr' [-Wunused-function]
drivers/scsi/myrb.c:2886:20: warning: unused function
'DAC960_PG_mem_mbox_status_available' [-Wunused-function]
drivers/scsi/myrb.c:2908:20: warning: unused function
'DAC960_PG_intr_enabled' [-Wunused-function]
drivers/scsi/myrb.c:2938:1: warning: unused function
'DAC960_PG_read_status_cmd_ident' [-Wunused-function]
drivers/scsi/myrb.c:3112:20: warning: unused function
'DAC960_PD_gen_intr' [-Wunused-function]
drivers/scsi/myrb.c:3158:20: warning: unused function
'DAC960_PD_intr_enabled' [-Wunused-function]
drivers/scsi/myrs.c:2416:20: warning: unused function
'DAC960_GEM_gen_intr' [-Wunused-function]
drivers/scsi/myrs.c:2460:20: warning: unused function
'DAC960_GEM_ack_mem_mbox_intr' [-Wunused-function]
drivers/scsi/myrs.c:2483:20: warning: unused function
'DAC960_GEM_mem_mbox_status_available' [-Wunused-function]
drivers/scsi/myrs.c:2505:20: warning: unused function
'DAC960_GEM_intr_enabled' [-Wunused-function]
drivers/scsi/myrs.c:2533:30: warning: unused function
'DAC960_GEM_read_cmd_ident' [-Wunused-function]
drivers/scsi/myrs.c:2682:20: warning: unused function
'DAC960_BA_gen_intr' [-Wunused-function]
drivers/scsi/myrs.c:2718:20: warning: unused function
'DAC960_BA_ack_mem_mbox_intr' [-Wunused-function]
drivers/scsi/myrs.c:2737:20: warning: unused function
'DAC960_BA_mem_mbox_status_available' [-Wunused-function]
drivers/scsi/myrs.c:2755:20: warning: unused function
'DAC960_BA_intr_enabled' [-Wunused-function]
drivers/scsi/myrs.c:2782:30: warning: unused function
'DAC960_BA_read_cmd_ident' [-Wunused-function]
drivers/scsi/myrs.c:2932:20: warning: unused function
'DAC960_LP_gen_intr' [-Wunused-function]
drivers/scsi/myrs.c:2968:20: warning: unused function
'DAC960_LP_ack_mem_mbox_intr' [-Wunused-function]
drivers/scsi/myrs.c:2987:20: warning: unused function
'DAC960_LP_mem_mbox_status_available' [-Wunused-function]
drivers/scsi/myrs.c:3005:20: warning: unused function
'DAC960_LP_intr_enabled' [-Wunused-function]
drivers/scsi/myrs.c:3031:30: warning: unused function
'DAC960_LP_read_cmd_ident' [-Wunused-function]
drivers/scsi/qla2xxx/qla_nx.c:384:1: warning: unused function
'qla82xx_pci_set_crbwindow' [-Wunused-function]
drivers/scsi/qla4xxx/ql4_nx.c:3654:1: warning: unused function
'flash_data_addr' [-Wunused-function]
drivers/staging/comedi/drivers/cb_pcidas64.c:232:19: warning: unused
function 'analog_trig_low_threshold_bits' [-Wunused-function]
drivers/staging/comedi/drivers/cb_pcidas64.c:383:28: warning: unused
function 'dma_chain_flag_bits' [-Wunused-function]
drivers/staging/isdn/gigaset/bas-gigaset.c:241:21: warning: unused
function 'usb_pipetype_str' [-Wunused-function]
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c:2072:18: warning:
unused function 'ieee80211_SignalStrengthTranslate'
[-Wunused-function]
drivers/staging/rts5208/xd.c:34:19: warning: unused function
'xd_check_err_code' [-Wunused-function]
drivers/tty/isicom.c:369:19: warning: unused function
'__isicom_paranoia_check' [-Wunused-function]
drivers/usb/host/sl811-hcd.c:588:18: warning: unused function
'checkdone' [-Wunused-function]
drivers/usb/host/u132-hcd.c:220:28: warning: unused function
'udev_to_u132' [-Wunused-function]
drivers/usb/serial/quatech2.c:179:19: warning: unused function
'qt2_setdevice' [-Wunused-function]
drivers/usb/typec/tps6598x.c:158:19: warning: unused function
'tps6598x_write16' [-Wunused-function]
drivers/usb/typec/tps6598x.c:163:19: warning: unused function
'tps6598x_write32' [-Wunused-function]
drivers/vhost/vhost.c:52:1: warning: unused function
'vhost_umem_interval_tree_iter_next' [-Wunused-function]
drivers/video/fbdev/arkfb.c:321:18: warning: unused function
'dac_read_reg' [-Wunused-function]
drivers/video/fbdev/arkfb.c:328:20: warning: unused function
'dac_read_regs' [-Wunused-function]
drivers/video/fbdev/aty/aty128fb.c:548:18: warning: unused function
'_aty_ld_8' [-Wunused-function]
drivers/video/fbdev/neofb.c:145:20: warning: unused function
'write_le32' [-Wunused-function]
drivers/video/fbdev/tridentfb.c:1127:20: warning: unused function
'shadowmode_off' [-Wunused-function]
drivers/watchdog/it87_wdt.c:152:20: warning: unused function
'superio_outw' [-Wunused-function]
fs/btrfs/extent-tree.c:2723:19: warning: unused function
'heads_to_leaves' [-Wunused-function]
fs/cifs/dfs_cache.c:317:20: warning: unused function
'is_sysvol_or_netlogon' [-Wunused-function]
fs/dlm/lock.c:238:19: warning: unused function 'is_granted' [-Wunused-function]
fs/lockd/xdr.c:109:1: warning: unused function 'nlm_encode_oh'
[-Wunused-function]
fs/nfsd/nfs4state.c:6034:1: warning: unused function 'end_offset'
[-Wunused-function]
fs/ocfs2/dlm/dlmrecovery.c:129:20: warning: unused function
'dlm_reset_recovery' [-Wunused-function]
fs/xfs/xfs_sysfs.c:72:1: warning: unused function 'to_mp' [-Wunused-function]
kernel/locking/rwsem.c:219:20: warning: unused function
'is_rwsem_reader_owned' [-Wunused-function]
kernel/locking/rwsem.c:284:35: warning: unused function 'rwsem_owner'
[-Wunused-function]
kernel/power/snapshot.c:1260:21: warning: unused function
'saveable_highmem_page' [-Wunused-function]
kernel/sched/cputime.c:255:19: warning: unused function
'account_other_time' [-Wunused-function]
kernel/sched/fair.c:4383:19: warning: unused function
'cfs_rq_clock_task' [-Wunused-function]
kernel/trace/trace_events_filter.c:1563:1: warning: unused function
'event_set_no_set_filter_flag' [-Wunused-function]
kernel/trace/trace_events_filter.c:1569:1: warning: unused function
'event_clear_no_set_filter_flag' [-Wunused-function]
kernel/trace/trace_events_filter.c:1575:1: warning: unused function
'event_no_set_filter_flag' [-Wunused-function]
kernel/trace/trace_kprobe.c:96:38: warning: unused function
'trace_kprobe_offset' [-Wunused-function]
lib/zlib_inflate/inffast.c:31:1: warning: unused function
'get_unaligned16' [-Wunused-function]
mm/memcontrol.c:4661:20: warning: unused function 'mem_cgroup_id_get'
[-Wunused-function]
mm/slub.c:1401:29: warning: unused function 'slab_free_hook' [-Wunused-function]
mm/slub.c:2007:28: warning: unused function 'tid_to_cpu' [-Wunused-function]
mm/slub.c:2012:29: warning: unused function 'tid_to_event' [-Wunused-function]
mm/zsmalloc.c:479:20: warning: unused function 'set_zspage_inuse'
[-Wunused-function]
net/bluetooth/6lowpan.c:105:35: warning: unused function
'peer_lookup_ba' [-Wunused-function]
net/bluetooth/6lowpan.c:912:20: warning: unused function 'bdaddr_type'
[-Wunused-function]
net/ipv6/exthdrs.c:712:33: warning: unused function 'ipv6_skb_idev'
[-Wunused-function]
net/ipv6/ip6_gre.c:846:20: warning: unused function
'ip6gre_tnl_addr_conflict' [-Wunused-function]
net/sched/sch_choke.c:145:20: warning: unused function
'choke_set_classid' [-Wunused-function]
net/sunrpc/svcauth_unix.c:301:30: warning: unused function
'ip_map_lookup' [-Wunused-function]
net/sunrpc/svcauth_unix.c:330:19: warning: unused function
'ip_map_update' [-Wunused-function]
security/apparmor/file.c:159:20: warning: unused function 'is_deleted'
[-Wunused-function]
security/apparmor/label.c:1230:20: warning: unused function
'label_is_visible' [-Wunused-function]
sound/drivers/portman2x4.c:186:18: warning: unused function
'portman_read_command' [-Wunused-function]
sound/drivers/portman2x4.c:196:18: warning: unused function
'portman_read_data' [-Wunused-function]
sound/pci/asihpi/asihpi.c:261:19: warning: unused function
'hpi_stream_group_get_map' [-Wunused-function]
sound/pci/azt3328.c:368:1: warning: unused function
'snd_azf3328_codec_outl' [-Wunused-function]
sound/pci/rme9652/hdspm.c:6154:19: warning: unused function
'copy_u32_le' [-Wunused-function]
sound/pci/trident/trident_memory.c:109:21: warning: unused function
'offset_ptr' [-Wunused-function]
sound/pci/ymfpci/ymfpci_main.c:34:18: warning: unused function
'snd_ymfpci_readb' [-Wunused-function]

Hope this helps.

- Sedat -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=kbuild&id=025960c034eacc433afd366085077991f8ed6e4e

2019-08-30 09:54:18

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

Just as a sidenote:

From [PATCH v2] kbuild: enable unused-function warnings for W= build with Clang:

"Per the documentation [1], -Wno-unused-function will also disable
-Wunneeded-internal-declaration, which can help find bugs like
commit 8289c4b6f2e5 ("platform/x86: mlx-platform: Properly use
mlxplat_mlxcpld_msn201x_items"). (pointed out by Nathan Chancellor)
I added -Wunneeded-internal-declaration to address it.

If you contribute to code clean-up, please run "make CC=clang W=1"
and check -Wunused-function warnings. You will find lots of unused
functions."

Isn't that missing in your double?

- Sedat -

[1] https://lkml.org/lkml/2019/8/27/729

2019-09-03 15:40:11

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

On Thu, Aug 29, 2019 at 9:05 AM Nathan Chancellor
<[email protected]> wrote:
>
> On Wed, Aug 28, 2019 at 04:28:30PM -0700, Nick Desaulniers wrote:
> > On Wed, Aug 28, 2019 at 11:20 AM Nathan Chancellor
> > <[email protected]> wrote:
> > >
> > > On Wed, Aug 28, 2019 at 02:54:25PM +0900, Masahiro Yamada wrote:
> > > > GCC and Clang have different policy for -Wunused-function; GCC does not
> > > > warn unused static inline functions at all whereas Clang does if they
> > > > are defined in source files instead of included headers although it has
> > > > been suppressed since commit abb2ea7dfd82 ("compiler, clang: suppress
> > > > warning for unused static inline functions").
> > > >
> > > > We often miss to delete unused functions where 'static inline' is used
> > > > in *.c files since there is no tool to detect them. Unused code remains
> > > > until somebody notices. For example, commit 075ddd75680f ("regulator:
> > > > core: remove unused rdev_get_supply()").
> > > >
> > > > Let's remove __maybe_unused from the inline macro to allow Clang to
> > > > start finding unused static inline functions. For now, we do this only
> > > > for W=1 build since it is not a good idea to sprinkle warnings for the
> > > > normal build.
> > > >
> > > > My initial attempt was to add -Wno-unused-function for no W=1 build
> > > > (https://lore.kernel.org/patchwork/patch/1120594/)
> > > >
> > > > Nathan Chancellor pointed out that would weaken Clang's checks since
> > > > we would no longer get -Wunused-function without W=1. It is true GCC
> > > > would detect unused static non-inline functions, but it would weaken
> > > > Clang as a standalone compiler at least.
> >
> > Got it. No problem.
> >
> > > >
> > > > Here is a counter implementation. The current problem is, W=... only
> > > > controls compiler flags, which are globally effective. There is no way
> > > > to narrow the scope to only 'static inline' functions.
> > > >
> > > > This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
> > > > When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
> > > > the 'inline' macro.
> > > >
> > > > This makes the code a bit uglier, so personally I do not want to carry
> > > > this forever. If we can manage to fix most of the warnings, we can
> > > > drop this entirely, then enable -Wunused-function all the time.
> >
> > How many warnings?
>
> In an x86 defconfig build (one of the smallest builds we do), I see an
> additional 35 warnings that crop up:
>
> https://gist.github.com/003ba86ba60b4ac7e8109089d6cb1a5a
>
> > > >
> > > > If you contribute to code clean-up, please run "make CC=clang W=1"
> > > > and check -Wunused-function warnings. You will find lots of unused
> > > > functions.
> > > >
> > > > Some of them are false-positives because the call-sites are disabled
> > > > by #ifdef. I do not like to abuse the inline keyword for suppressing
> > > > unused-function warnings because it is intended to be a hint for the
> > > > compiler optimization. I prefer #ifdef around the definition, or
> > > > __maybe_unused if #ifdef would make the code too ugly.
> > > >
> > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > >
> > > I can still see warnings from static unused functions and with W=1, I
> > > see plenty more. I agree that this is uglier because of the
> > > __inline_maybe_unused but I think this is better for regular developers.
> > > I will try to work on these unused-function warnings!
> >
> > How many are we talking here?
> >
> > >
> > > Reviewed-by: Nathan Chancellor <[email protected]>
> > > Tested-by: Nathan Chancellor <[email protected]>
> >
> > This is getting kind of messy. I was more ok when the goal seemed to
> > be simplifying the definition of `inline`, but this is worse IMO.
>
> I guess if you want, we can just go back to v1 and have all unused
> function warnings hidden by default with clang. Fixing these warnings
> will take a significant amount of time given there will probably be a
> few hundred so I don't think having this warning hidden behind W=1 for
> that long is a good thing.
>
> Cheers,
> Nathan

I slightly prefer this version.

Either way we go, I want to fix -Wunused-function warnings,
then revert this patch as soon as possible.


--
Best Regards

Masahiro Yamada

2019-09-03 15:40:58

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 2/2] kbuild: allow Clang to find unused static inline functions for W=1 build

On Fri, Aug 30, 2019 at 6:52 PM Sedat Dilek <[email protected]> wrote:
>
> Just as a sidenote:
>
> From [PATCH v2] kbuild: enable unused-function warnings for W= build with Clang:
>
> "Per the documentation [1], -Wno-unused-function will also disable
> -Wunneeded-internal-declaration, which can help find bugs like
> commit 8289c4b6f2e5 ("platform/x86: mlx-platform: Properly use
> mlxplat_mlxcpld_msn201x_items"). (pointed out by Nathan Chancellor)
> I added -Wunneeded-internal-declaration to address it.
>
> If you contribute to code clean-up, please run "make CC=clang W=1"
> and check -Wunused-function warnings. You will find lots of unused
> functions."

This information is unrelated to this version,
so I dropped it.


> Isn't that missing in your double?
>
> - Sedat -
>
> [1] https://lkml.org/lkml/2019/8/27/729



--
Best Regards
Masahiro Yamada