Hi everyone,
This patch series aims to silence some instances of clang's
-Wtautological-compare that are not problematic and enable it globally
for the kernel because it has a bunch of subwarnings that can find real
bugs in the kernel such as
https://lore.kernel.org/lkml/[email protected]/
and https://bugs.llvm.org/show_bug.cgi?id=42666, which was specifically
requested by Dmitry.
The first patch adds a macro that casts the section variables to
unsigned long (uintptr_t), which silences the warning and adds
documentation.
Patches two through four silence the warning in the places I have
noticed it across all of my builds with -Werror, including arm, arm64,
and x86_64 defconfig/allmodconfig/allyesconfig. There might still be
more lurking but those will have to be teased out over time.
Patch six finally enables the warning, while leaving one of the
subwarnings disabled because it is rather noisy and somewhat pointless
for the kernel, where core kernel code is expected to build and run with
many different configurations where variable types can be different
sizes.
A slight meta comment: This is the first treewide patchset that I have
sent. I pray I did everything right but please let me know if I did not.
I assume someone like Andrew will pick this up with acks from everyone
but let me know if there is someone better.
Cheers,
Nathan
Nathan Chancellor (6):
asm/sections: Add COMPARE_SECTIONS macro
kernel/extable: Wrap section comparison in sort_main_extable with
COMPARE_SECTIONS
tracing: Wrap section comparison in tracer_alloc_buffers with
COMPARE_SECTIONS
dynamic_debug: Wrap section comparison in dynamic_debug_init with
COMPARE_SECTIONS
mm: kmemleak: Wrap section comparison in kmemleak_init with
COMPARE_SECTIONS
kbuild: Enable -Wtautological-compare
Makefile | 3 +--
include/asm-generic/sections.h | 7 +++++++
kernel/extable.c | 3 ++-
kernel/trace/trace.c | 2 +-
lib/dynamic_debug.c | 2 +-
mm/kmemleak.c | 3 ++-
6 files changed, 14 insertions(+), 6 deletions(-)
base-commit: 02815e777db630e3c183718cab73752b48a5053e
--
2.25.1
Clang warns:
../kernel/extable.c:37:52: warning: array comparison always evaluates to
a constant [-Wtautological-compare]
if (main_extable_sort_needed && __stop___ex_table > __start___ex_table) {
^
1 warning generated.
These are not true arrays, they are linker defined symbols, which are
just addresses so there is not a real issue here. Use the
COMPARE_SECTIONS macro to silence this warning by casting the linker
defined symbols to unsigned long, which keeps the logic the same.
Link: https://github.com/ClangBuiltLinux/linux/issues/765
Signed-off-by: Nathan Chancellor <[email protected]>
---
kernel/extable.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/extable.c b/kernel/extable.c
index a0024f27d3a1..17bf4ccb9de9 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -34,7 +34,8 @@ u32 __initdata __visible main_extable_sort_needed = 1;
/* Sort the kernel's built-in exception table */
void __init sort_main_extable(void)
{
- if (main_extable_sort_needed && __stop___ex_table > __start___ex_table) {
+ if (main_extable_sort_needed &&
+ COMPARE_SECTIONS(__stop___ex_table, >, __start___ex_table)) {
pr_notice("Sorting __ex_table...\n");
sort_extable(__start___ex_table, __stop___ex_table);
}
--
2.25.1
On Tue, 18 Feb 2020 21:54:17 -0700 Nathan Chancellor <[email protected]> wrote:
> Hi everyone,
>
> This patch series aims to silence some instances of clang's
> -Wtautological-compare that are not problematic and enable it globally
> for the kernel because it has a bunch of subwarnings that can find real
> bugs in the kernel such as
> https://lore.kernel.org/lkml/[email protected]/
> and https://bugs.llvm.org/show_bug.cgi?id=42666, which was specifically
> requested by Dmitry.
>
> The first patch adds a macro that casts the section variables to
> unsigned long (uintptr_t), which silences the warning and adds
> documentation.
>
> Patches two through four silence the warning in the places I have
> noticed it across all of my builds with -Werror, including arm, arm64,
> and x86_64 defconfig/allmodconfig/allyesconfig. There might still be
> more lurking but those will have to be teased out over time.
>
> Patch six finally enables the warning, while leaving one of the
> subwarnings disabled because it is rather noisy and somewhat pointless
> for the kernel, where core kernel code is expected to build and run with
> many different configurations where variable types can be different
> sizes.
>
For some reason none of these patches apply. Not sure why - prehaps
something in the diff headers.
Anyway, the kmemleak.c code has recently changed in ways which impact
these patches. Please take a look at that, redo, retest and resend?
Hi Andrew,
On Mon, Apr 20, 2020 at 09:03:32PM -0700, Andrew Morton wrote:
> On Tue, 18 Feb 2020 21:54:17 -0700 Nathan Chancellor <[email protected]> wrote:
>
> > Hi everyone,
> >
> > This patch series aims to silence some instances of clang's
> > -Wtautological-compare that are not problematic and enable it globally
> > for the kernel because it has a bunch of subwarnings that can find real
> > bugs in the kernel such as
> > https://lore.kernel.org/lkml/[email protected]/
> > and https://bugs.llvm.org/show_bug.cgi?id=42666, which was specifically
> > requested by Dmitry.
> >
> > The first patch adds a macro that casts the section variables to
> > unsigned long (uintptr_t), which silences the warning and adds
> > documentation.
> >
> > Patches two through four silence the warning in the places I have
> > noticed it across all of my builds with -Werror, including arm, arm64,
> > and x86_64 defconfig/allmodconfig/allyesconfig. There might still be
> > more lurking but those will have to be teased out over time.
> >
> > Patch six finally enables the warning, while leaving one of the
> > subwarnings disabled because it is rather noisy and somewhat pointless
> > for the kernel, where core kernel code is expected to build and run with
> > many different configurations where variable types can be different
> > sizes.
> >
>
> For some reason none of these patches apply. Not sure why - prehaps
> something in the diff headers.
>
> Anyway, the kmemleak.c code has recently changed in ways which impact
> these patches. Please take a look at that, redo, retest and resend?
>
>
Thank you for doubling back around to this but we are good here. These
warnings have all been fixed in Linus' tree without the need for the
first patch in the series.
For those curious:
63174f61dfaef ("kernel/extable.c: use address-of operator on section symbols")
bf2cbe044da27 ("tracing: Use address-of operator on section symbols")
8306b057a85ec ("lib/dynamic_debug.c: use address-of operator on section symbols")
b0d14fc43d392 ("mm/kmemleak.c: use address-of operator on section symbols")
afe956c577b2d ("kbuild: Enable -Wtautological-compare")
Cheers,
Nathan