Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vincent Mailhol <mailhol.vincent@wanadoo.fr>
To:     Rikard Falkeborn <rikard.falkeborn@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org
Cc:     Arnd Bergmann <arnd@arndb.de>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        Kees Cook <keescook@chromium.org>,
        Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Subject: [PATCH] linux/bits.h: fix -Wtype-limits warnings in GENMASK_INPUT_CHECK()
Date:   Fri,  4 Mar 2022 21:44:16 +0900
Message-Id: <20220304124416.1181029-1-mailhol.vincent@wanadoo.fr>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

When compiling with -Wtype-limits (activated for example with make
W=2), GENMASK_INPUT_CHECK() will generate some warnings if invoked
with an unsigned integer and zero.

For example, this:

| #include <linux/bits.h>
| u32 foo(u32 bar)
| { return GENMASK(bar, 0); }

would yield:

| In file included from ./include/linux/bits.h:22,
|                  from ./foo.c:1:
| foo.c: In function 'foo':
| ./include/linux/bits.h:25:36: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
|    25 |                 __is_constexpr((l) > (h)), (l) > (h), 0)))
|       |                                    ^
| ./include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
|    16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
|       |                                                              ^
| ./include/linux/bits.h:25:17: note: in expansion of macro '__is_constexpr'
|    25 |                 __is_constexpr((l) > (h)), (l) > (h), 0)))
|       |                 ^~~~~~~~~~~~~~
| ./include/linux/bits.h:38:10: note: in expansion of macro 'GENMASK_INPUT_CHECK'
|    38 |         (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|       |          ^~~~~~~~~~~~~~~~~~~
| foo.c:16:10: note: in expansion of macro 'GENMASK'
|    16 | { return GENMASK(bar, 0); }
|       |          ^~~~~~~
| ./include/linux/bits.h:25:48: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
|    25 |                 __is_constexpr((l) > (h)), (l) > (h), 0)))
|       |                                                ^
| ./include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
|    16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
|       |                                                              ^
| ./include/linux/bits.h:38:10: note: in expansion of macro 'GENMASK_INPUT_CHECK'
|    38 |         (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|       |          ^~~~~~~~~~~~~~~~~~~
| foo.c:16:10: note: in expansion of macro 'GENMASK'
|    16 | { return GENMASK(bar, 0); }
|       |          ^~~~~~~

This pattern is harmless but because it occurs in header files
(example find_first_bit() from linux/find.h [1]) and because of the
include hell, the macro GENMASK_INPUT_CHECK() is accountable for 31%
(164714/532484) of all warnings when compiling all modules at W=2
level.

Reference (using gcc 11.2 and linux v5.17-rc6):

| $ make allyesconfig
| $ sed -i '/CONFIG_WERROR/d' .config
| $ make W=2 -j8 2> kernel_w2.log > /dev/null
| $ grep "\./include/linux/bits\.h:.*: warning" kernel_w2\.log | wc -l
| 164714
| $ grep ": warning: " kernel_w2.log | wc -l
| 532484

This heavily pollutes the output of make W=2 and make it painful to
find other relevant issues.

In this patch, we silent this warning by:

  * replacing the comparison > by and logical and && in the first
    argument of __builtin_choose_expr().

  * casting the high bit of the mask to a signed integer in the second
    argument of __builtin_choose_expr().

By doing so, 31% of W=2 warnings get removed.

[1] https://elixir.bootlin.com/linux/v5.17-rc6/source/include/linux/find.h#L119

Fixes: 295bcca84916 ("linux/bits.h: add compile time sanity check of
GENMASK inputs")
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 include/linux/bits.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 87d112650dfb..542e9a8985b1 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -22,7 +22,7 @@
 #include <linux/build_bug.h>
 #define GENMASK_INPUT_CHECK(h, l) \
 	(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
-		__is_constexpr((l) > (h)), (l) > (h), 0)))
+		__is_constexpr((h)) && __is_constexpr((l)), (l) > (int)(h), 0)))
 #else
 /*
  * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
-- 
2.34.1