[v2] kbuild: Enable -Wtautological-compare
diff mbox series

Message ID 20200326194155.29107-1-natechancellor@gmail.com
State New
Headers show
Series
  • [v2] kbuild: Enable -Wtautological-compare
Related show

Commit Message

Nathan Chancellor March 26, 2020, 7:41 p.m. UTC
Currently, we disable -Wtautological-compare, which in turn disables a
bunch of more specific tautological comparison warnings that are useful
for the kernel such as -Wtautological-bitwise-compare. See clang's
documentation below for the other warnings that are suppressed by
-Wtautological-compare. Now that all of the major/noisy warnings have
been fixed, enable -Wtautological-compare so that more issues can be
caught at build time by various continuous integration setups.

-Wtautological-constant-out-of-range-compare is kept disabled under a
normal build but visible at W=1 because there are places in the kernel
where a constant or variable size can change based on the kernel
configuration. These are not fixed in a clean/concise way and the ones
I have audited so far appear to be harmless. It is not a subgroup but
rather just one warning so we do not lose out on much coverage by
default.

Link: https://github.com/ClangBuiltLinux/linux/issues/488
Link: http://releases.llvm.org/10.0.0/tools/clang/docs/DiagnosticsReference.html#wtautological-compare
Link: https://bugs.llvm.org/show_bug.cgi?id=42666
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v2: https://lore.kernel.org/lkml/20200219045423.54190-7-natechancellor@gmail.com/

* Expand commit message a bit by adding more reasoning behind change.
* Disable -Wtautological-constant-out-of-range-compare under a normal
  build but allow it to show up at W=1 for easy auditing.

I hope this can be accepted for 5.7. There are two warnings that I see
still across a bunch of allyesconfig/allmodconfig builds that have
patches sent but not accepted. I will ping them today.

* https://lore.kernel.org/lkml/20191023002014.22571-1-natechancellor@gmail.com/
* https://lore.kernel.org/lkml/20200220051011.26113-1-natechancellor@gmail.com/

 Makefile                   | 2 --
 scripts/Makefile.extrawarn | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

Comments

Masahiro Yamada March 29, 2020, 11:31 a.m. UTC | #1
On Fri, Mar 27, 2020 at 4:42 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Currently, we disable -Wtautological-compare, which in turn disables a
> bunch of more specific tautological comparison warnings that are useful
> for the kernel such as -Wtautological-bitwise-compare. See clang's
> documentation below for the other warnings that are suppressed by
> -Wtautological-compare. Now that all of the major/noisy warnings have
> been fixed, enable -Wtautological-compare so that more issues can be
> caught at build time by various continuous integration setups.
>
> -Wtautological-constant-out-of-range-compare is kept disabled under a
> normal build but visible at W=1 because there are places in the kernel
> where a constant or variable size can change based on the kernel
> configuration. These are not fixed in a clean/concise way and the ones
> I have audited so far appear to be harmless. It is not a subgroup but
> rather just one warning so we do not lose out on much coverage by
> default.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/488
> Link: http://releases.llvm.org/10.0.0/tools/clang/docs/DiagnosticsReference.html#wtautological-compare
> Link: https://bugs.llvm.org/show_bug.cgi?id=42666
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2: https://lore.kernel.org/lkml/20200219045423.54190-7-natechancellor@gmail.com/
>
> * Expand commit message a bit by adding more reasoning behind change.
> * Disable -Wtautological-constant-out-of-range-compare under a normal
>   build but allow it to show up at W=1 for easy auditing.
>
> I hope this can be accepted for 5.7. There are two warnings that I see
> still across a bunch of allyesconfig/allmodconfig builds that have
> patches sent but not accepted. I will ping them today.
>
> * https://lore.kernel.org/lkml/20191023002014.22571-1-natechancellor@gmail.com/
> * https://lore.kernel.org/lkml/20200220051011.26113-1-natechancellor@gmail.com/


OK, I will queue this up and send it to Linus
in the second week of MW.

I hope all warnings will be fixed by that time.
Nathan Chancellor March 31, 2020, 10:11 a.m. UTC | #2
On Sun, Mar 29, 2020 at 08:31:26PM +0900, Masahiro Yamada wrote:
> On Fri, Mar 27, 2020 at 4:42 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Currently, we disable -Wtautological-compare, which in turn disables a
> > bunch of more specific tautological comparison warnings that are useful
> > for the kernel such as -Wtautological-bitwise-compare. See clang's
> > documentation below for the other warnings that are suppressed by
> > -Wtautological-compare. Now that all of the major/noisy warnings have
> > been fixed, enable -Wtautological-compare so that more issues can be
> > caught at build time by various continuous integration setups.
> >
> > -Wtautological-constant-out-of-range-compare is kept disabled under a
> > normal build but visible at W=1 because there are places in the kernel
> > where a constant or variable size can change based on the kernel
> > configuration. These are not fixed in a clean/concise way and the ones
> > I have audited so far appear to be harmless. It is not a subgroup but
> > rather just one warning so we do not lose out on much coverage by
> > default.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/488
> > Link: http://releases.llvm.org/10.0.0/tools/clang/docs/DiagnosticsReference.html#wtautological-compare
> > Link: https://bugs.llvm.org/show_bug.cgi?id=42666
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > v1 -> v2: https://lore.kernel.org/lkml/20200219045423.54190-7-natechancellor@gmail.com/
> >
> > * Expand commit message a bit by adding more reasoning behind change.
> > * Disable -Wtautological-constant-out-of-range-compare under a normal
> >   build but allow it to show up at W=1 for easy auditing.
> >
> > I hope this can be accepted for 5.7. There are two warnings that I see
> > still across a bunch of allyesconfig/allmodconfig builds that have
> > patches sent but not accepted. I will ping them today.
> >
> > * https://lore.kernel.org/lkml/20191023002014.22571-1-natechancellor@gmail.com/
> > * https://lore.kernel.org/lkml/20200220051011.26113-1-natechancellor@gmail.com/
> 
> 
> OK, I will queue this up and send it to Linus
> in the second week of MW.
> 
> I hope all warnings will be fixed by that time.

Just a follow up, those two patches have been picked up and should be in
this coming release:

https://git.kernel.org/balbi/usb/c/58582220d2d34228e5a1e1585e41b735713988bb
https://git.kernel.org/rostedt/linux-trace/c/bf2cbe044da275021b2de5917240411a19e5c50d

As of next-20200331, with the former applied (because it is not there
yet) along with this patch, I see no warnings on arm, arm64, x86_64
all{mod,yes}config.

Cheers,
Nathan
Nick Desaulniers March 31, 2020, 4:02 p.m. UTC | #3
On Tue, Mar 31, 2020 at 3:11 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
> Just a follow up, those two patches have been picked up and should be in
> this coming release:
>
> https://git.kernel.org/balbi/usb/c/58582220d2d34228e5a1e1585e41b735713988bb
> https://git.kernel.org/rostedt/linux-trace/c/bf2cbe044da275021b2de5917240411a19e5c50d
>
> As of next-20200331, with the former applied (because it is not there
> yet) along with this patch, I see no warnings on arm, arm64, x86_64
> all{mod,yes}config.

kbuild test robot is testing more arch's than that with Clang so it
may report if it finds more instances of that warning in those.
Nathan Chancellor March 31, 2020, 7:26 p.m. UTC | #4
On Tue, Mar 31, 2020 at 09:02:19AM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> On Tue, Mar 31, 2020 at 3:11 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> > Just a follow up, those two patches have been picked up and should be in
> > this coming release:
> >
> > https://git.kernel.org/balbi/usb/c/58582220d2d34228e5a1e1585e41b735713988bb
> > https://git.kernel.org/rostedt/linux-trace/c/bf2cbe044da275021b2de5917240411a19e5c50d
> >
> > As of next-20200331, with the former applied (because it is not there
> > yet) along with this patch, I see no warnings on arm, arm64, x86_64
> > all{mod,yes}config.
> 
> kbuild test robot is testing more arch's than that with Clang so it
> may report if it finds more instances of that warning in those.
> 
> -- 
> Thanks,
> ~Nick Desaulniers
> 

I'll keep an eye out. Hopefully not too many more are lurking but we
have definitely caught some bad behavior with this warning already so
getting it turned on so that all CI systems can benefit from it is
important.

Cheers,
Nathan
Masahiro Yamada April 2, 2020, 5:05 p.m. UTC | #5
On Wed, Apr 1, 2020 at 4:26 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Tue, Mar 31, 2020 at 09:02:19AM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> > On Tue, Mar 31, 2020 at 3:11 AM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > > Just a follow up, those two patches have been picked up and should be in
> > > this coming release:
> > >
> > > https://git.kernel.org/balbi/usb/c/58582220d2d34228e5a1e1585e41b735713988bb
> > > https://git.kernel.org/rostedt/linux-trace/c/bf2cbe044da275021b2de5917240411a19e5c50d
> > >
> > > As of next-20200331, with the former applied (because it is not there
> > > yet) along with this patch, I see no warnings on arm, arm64, x86_64
> > > all{mod,yes}config.
> >
> > kbuild test robot is testing more arch's than that with Clang so it
> > may report if it finds more instances of that warning in those.
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> >
>
> I'll keep an eye out. Hopefully not too many more are lurking but we
> have definitely caught some bad behavior with this warning already so
> getting it turned on so that all CI systems can benefit from it is
> important.
>
> Cheers,
> Nathan



Applied to linux-kbuild.

I will rebase my branch during this MW,
so the commit ID will be unstable.
Please do not record it until it lands in Linus' tree.

Patch
diff mbox series

diff --git a/Makefile b/Makefile
index db442a9ee6b2..05f9f50dda3e 100644
--- a/Makefile
+++ b/Makefile
@@ -746,8 +746,6 @@  ifdef CONFIG_CC_IS_CLANG
 KBUILD_CPPFLAGS += -Qunused-arguments
 KBUILD_CFLAGS += -Wno-format-invalid-specifier
 KBUILD_CFLAGS += -Wno-gnu
-# Quiet clang warning: comparison of unsigned expression < 0 is always false
-KBUILD_CFLAGS += -Wno-tautological-compare
 # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the
 # source of a reference will be _MergedGlobals and not on of the whitelisted names.
 # See modpost pattern 2
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index ca08f2fe7c34..4aea7cf71d11 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -49,6 +49,7 @@  KBUILD_CFLAGS += -Wno-format
 KBUILD_CFLAGS += -Wno-sign-compare
 KBUILD_CFLAGS += -Wno-format-zero-length
 KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
+KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
 endif
 
 endif