From: Nathan Chancellor <natechancellor@gmail.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: linux-kbuild@vger.kernel.org,
Nick Desaulniers <ndesaulniers@google.com>,
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
Borislav Petkov <bp@suse.de>, Kees Cook <keescook@chromium.org>,
Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
Michal Marek <michal.lkml@markovi.net>,
Paul Burton <paul.burton@mips.com>,
Xiaozhou Liu <liuxiaozhou@bytedance.com>,
clang-built-linux@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kbuild: enable unused-function warnings for W= build with Clang
Date: Mon, 19 Aug 2019 09:09:20 -0700 [thread overview]
Message-ID: <20190819160920.GA108942@archlinux-threadripper> (raw)
In-Reply-To: <20190819105138.5053-1-yamada.masahiro@socionext.com>
On Mon, Aug 19, 2019 at 07:51:38PM +0900, Masahiro Yamada wrote:
> GCC and Clang have different policy for -Wunused-function; GCC does
> not report unused-function warnings at all for the functions marked
> as 'static inline'. Clang does report unused-function warnings if they
> are defined in source files instead of headers.
>
> We could use Clang for detecting unused functions, but it has been
> suppressed since commit abb2ea7dfd82 ("compiler, clang: suppress
> warning for unused static inline functions").
>
> So, we never notice left-over code if functions in .c files are
> marked as inline.
>
> Let's remove __maybe_unused from the inline macro. As always, it is
> not a good idea to sprinkle warnings for the normal build. So, these
> warnings will be shown for the W= build.
>
> 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 might affect the compiler's
> optimization. When I need to fix unused-functions warnings, I prefer
> adding #ifdef or __maybe_unused to function definitions.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
So if I understand everything correctly, this change allows us to start
finding unused static inline functions with clang at W=1 but disables
-Wunused-function by default... I am not sure that is a good tradeoff
as I am pretty sure that W=1 is fairly noisy for clang although I
haven't checked lately. I'd argue most regular developers do not build
with W=1 meaning -Wunused-function generally will not be run with clang
at all, missing stuff like this:
https://lore.kernel.org/lkml/20190523010235.GA105588@archlinux-epyc/
https://lore.kernel.org/lkml/1558574945-19275-1-git-send-email-skomatineni@nvidia.com/
Furthermore, per the documemtation [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").
[1]: https://clang.llvm.org/docs/DiagnosticsReference.html#wunused-function
Is there a way to conditionally remove __maybe_unused from the inline
defintion so that we keep the current behavior but we can still
selectively find potentially unused functions?
Cheers,
Nathan
next prev parent reply other threads:[~2019-08-19 16:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-19 10:51 [PATCH] kbuild: enable unused-function warnings for W= build with Clang Masahiro Yamada
2019-08-19 16:09 ` Nathan Chancellor [this message]
2019-08-19 16:58 ` Masahiro Yamada
2019-08-20 2:25 ` Nathan Chancellor
2019-08-19 16:10 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190819160920.GA108942@archlinux-threadripper \
--to=natechancellor@gmail.com \
--cc=bp@suse.de \
--cc=clang-built-linux@googlegroups.com \
--cc=keescook@chromium.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=liuxiaozhou@bytedance.com \
--cc=luc.vanoostenryck@gmail.com \
--cc=michal.lkml@markovi.net \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=ndesaulniers@google.com \
--cc=paul.burton@mips.com \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.