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

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

[Masahiro Yamada]

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
+
+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

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

[Masahiro Yamada]

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 <yamada.masahiro@socionext.com>
---

 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

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

[Sedat Dilek]

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.

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

[Sedat Dilek]

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 -

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

[Sedat Dilek]

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

Grrr.

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

- Sedat -

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

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 158 bytes --]

Something like that...

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

- Sedat -

[-- Attachment #2: 0001-kbuild-Improve-extrawarn-documentation.patch --]
[-- Type: text/x-patch, Size: 1763 bytes --]

From 1275ec0f1d31c4ac57b73b318bdc45151d99e8dc Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@credativ.de>
Date: Wed, 28 Aug 2019 16:27:13 +0200
Subject: [PATCH 1/2] kbuild: Improve extrawarn documentation

---
 Documentation/kbuild/kbuild.rst | 10 ++++++++--
 scripts/Makefile.extrawarn      |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 62f9d86c082c..f0f1c475d7fa 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -243,8 +243,14 @@ To get all available archs you can also specify all. E.g.::
 
 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.
+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>.
+
+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
 
 KBUILD_BUILD_TIMESTAMP
 ----------------------
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 3af1770497fd..6770f8da4e6d 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -32,7 +32,7 @@ KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
 
 else
 
-# W=1 also stops suppressing some warnings
+# W=1 - also stops suppressing some warnings
 
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += -Wno-initializer-overrides
-- 
2.20.1


[-- Attachment #3: 0002-kbuild-Rename-extrawarn-Kconfig-to-KBUILD_EXTRA_CC_C.patch --]
[-- Type: text/x-patch, Size: 4106 bytes --]

From b364881f8b2a7aa258dcdbaba8d6bc57b41def0d Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@credativ.de>
Date: Wed, 28 Aug 2019 16:30:03 +0200
Subject: [PATCH 2/2] kbuild: Rename extrawarn Kconfig to
 KBUILD_EXTRA_CC_CHECKS

---
 Documentation/kbuild/kbuild.rst | 2 +-
 scripts/Makefile.build          | 2 +-
 scripts/Makefile.extrawarn      | 8 ++++----
 scripts/Makefile.lib            | 4 ++--
 scripts/genksyms/Makefile       | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index f0f1c475d7fa..dcc83d993459 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -241,7 +241,7 @@ To get all available archs you can also specify all. E.g.::
 
     $ make ALLSOURCE_ARCHS=all tags
 
-KBUILD_ENABLE_EXTRA_GCC_CHECKS
+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
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2a21ca86b720..1de9b9ddddaa 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -85,7 +85,7 @@ else ifeq ($(KBUILD_CHECKSRC),2)
         cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
 endif
 
-ifneq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
+ifneq ($(KBUILD_EXTRA_CC_CHECKS),)
   cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $<
 endif
 
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 6770f8da4e6d..0ee4a5a88d2c 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -6,13 +6,13 @@
 KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
 
 ifeq ("$(origin W)", "command line")
-  export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W)
+  export KBUILD_EXTRA_CC_CHECKS := $(W)
 endif
 
 #
 # W=1 - warnings that may be relevant and does not occur too often
 #
-ifneq ($(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 1, $(KBUILD_EXTRA_CC_CHECKS)),)
 
 KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter
 KBUILD_CFLAGS += -Wmissing-declarations
@@ -46,7 +46,7 @@ endif
 #
 # W=2 - warnings that occur quite often but may still be relevant
 #
-ifneq ($(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 2, $(KBUILD_EXTRA_CC_CHECKS)),)
 
 KBUILD_CFLAGS += -Wcast-align
 KBUILD_CFLAGS += -Wdisabled-optimization
@@ -65,7 +65,7 @@ endif
 #
 # W=3 - the more obscure warnings, can most likely be ignored
 #
-ifneq ($(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 3, $(KBUILD_EXTRA_CC_CHECKS)),)
 
 KBUILD_CFLAGS += -Wbad-function-cast
 KBUILD_CFLAGS += -Wcast-qual
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 888e5c830646..1f9e38550ce4 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -248,7 +248,7 @@ quiet_cmd_gzip = GZIP    $@
 DTC ?= $(objtree)/scripts/dtc/dtc
 
 # Disable noisy checks by default
-ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifeq ($(findstring 1,$(KBUILD_EXTRA_CC_CHECKS)),)
 DTC_FLAGS += -Wno-unit_address_vs_reg \
 	-Wno-unit_address_format \
 	-Wno-avoid_unnecessary_addr_size \
@@ -259,7 +259,7 @@ DTC_FLAGS += -Wno-unit_address_vs_reg \
 	-Wno-pci_device_reg
 endif
 
-ifneq ($(findstring 2,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifneq ($(findstring 2,$(KBUILD_EXTRA_CC_CHECKS)),)
 DTC_FLAGS += -Wnode_name_chars_strict \
 	-Wproperty_name_chars_strict
 endif
diff --git a/scripts/genksyms/Makefile b/scripts/genksyms/Makefile
index baf44ed0a93a..e08fc3a6f7e2 100644
--- a/scripts/genksyms/Makefile
+++ b/scripts/genksyms/Makefile
@@ -12,7 +12,7 @@ genksyms-objs	:= genksyms.o parse.tab.o lex.lex.o
 #
 # Just in case, run "$(YACC) --version" without suppressing stderr
 # so that 'bison: not found' will be displayed if it is missing.
-ifeq ($(findstring 1,$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)),)
+ifeq ($(findstring 1,$(KBUILD_EXTRA_CC_CHECKS)),)
 
 quiet_cmd_bison_no_warn = $(quiet_cmd_bison)
       cmd_bison_no_warn = $(YACC) --version >/dev/null; \
-- 
2.20.1

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

[Nathan Chancellor]

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>

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

[Nathan Chancellor]

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 <yamada.masahiro@socionext.com>

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 <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

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

[Nick Desaulniers]

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.

-- 
Thanks,
~Nick Desaulniers

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

[Nick Desaulniers]

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

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

[Nick Desaulniers]

On Wed, Aug 28, 2019 at 11:20 AM Nathan Chancellor
<natechancellor@gmail.com> 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 <yamada.masahiro@socionext.com>
>
> 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 <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>

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

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

[Nathan Chancellor]

On Wed, Aug 28, 2019 at 04:28:30PM -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 11:20 AM Nathan Chancellor
> <natechancellor@gmail.com> 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 <yamada.masahiro@socionext.com>
> >
> > 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 <natechancellor@gmail.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> 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

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

[Sedat Dilek]

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 -

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

[Sedat Dilek]

> 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

 #
-- 
2.20.1

- Sedat -

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

[Masahiro Yamada]

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

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

[Sedat Dilek]

On Thu, Aug 29, 2019 at 1:28 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> 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

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

[Sedat Dilek]

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

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

[Masahiro Yamada]

On Thu, Aug 29, 2019 at 9:05 AM Nathan Chancellor
<natechancellor@gmail.com> 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
> > <natechancellor@gmail.com> 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 <yamada.masahiro@socionext.com>
> > >
> > > 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 <natechancellor@gmail.com>
> > > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> >
> > 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

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

[Masahiro Yamada]

On Fri, Aug 30, 2019 at 6:52 PM Sedat Dilek <sedat.dilek@gmail.com> 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