[PATCH next 11/14] bit: Strengthen compile-time tests in GENMASK() and BIT()

[david.laight.linux@gmail.com]

From: David Laight <david.laight.linux@gmail.com>

The current checks in GENMASK/BIT (eg reversed high/low) only work
for 'integer constant expressions' not 'compile-time constants'.
This is true for const_true() and -Wshift-count-overflow/negative.
While compile-time constants may be unusual, they can happen through
function inlining.

This isn't too bad with gcc, but if clang detects a negative/over-large
shift it treats it as 'undefined behaviour' and silently discards all
code that would use the result, so:
int f(u32 x) {int n = 32; return x >> n; }
generates a function that just contains a 'return' instruction.
If 'n' was a variable that happened to be 32, most modern cpu mask
the count - so would return 'x', some might return 0.

Add extra checks for arguments that pass __builtin_constant_p()
but are not 'integer constant expressions.
__builtin_choose_expr() isn't strong enough to allow
_Static_assert() or ({ ... }) in the other branch so non-standard
schemes are used to report the errors.

To reduce pre-processor bloat the checks are only enabled for W=c
(implied by W=1) builds (where they are errors).

Update the unit tests to match.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bits.h  | 45 +++++++++++++++++++++++++++++++++----------
 lib/tests/test_bits.c | 34 +++++++++++++++++++-------------
 2 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 43631a334314..0f559038981d 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -23,20 +23,35 @@
 #include <linux/compiler.h>
 #include <linux/overflow.h>
 
-#define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
+#ifndef KBUILD_EXTRA_WARNc
+#define GENMASK_INPUT_CHECK(h, l, width) 0
+#else
+int GENMASK_INPUT_CHECK_FAIL(void) __compiletime_error("Invalid bit numbers");
+#define GENMASK_INPUT_CHECK(h, l, width)				\
+	(__builtin_choose_expr(__is_constexpr((l) > (h)),		\
+		sizeof(struct { char low_bit_greater_than_high[-((l) > (h))];}), \
+		__builtin_constant_p((l) | (h)) &&			\
+			((l) < 0 || (l) > (h) || (h) >= width) &&	\
+			GENMASK_INPUT_CHECK_FAIL()))
+#endif
 
 /*
- * Generate a mask for the specified type @t. Additional checks are made to
- * guarantee the value returned fits in that type, relying on
- * -Wshift-count-overflow compiler check to detect incompatible arguments.
+ * Generate a mask for the specified type @t.
+ * Checks are made to guarantee the value returned fits in that type.
+ * The compiler's -Wshift-count-overflow/negative check detects invalid values
+ * from 'constant integer expressions' but not other compile time constants.
+ * Clang treats out of value constants as 'undefined behaviour' and stops
+ * generating code - so explicit checks are needed.
+ * Neither BUILD_BUG() nor BUILD_BUG_ON_ZERO() can be used.
+ *
  * For example, all these create build errors or warnings:
  *
  * - GENMASK(15, 20): wrong argument order
  * - GENMASK(72, 15): doesn't fit unsigned long
  * - GENMASK_U32(33, 15): doesn't fit in a u32
  */
-#define GENMASK_TYPE(t, h, l)					\
-	((unsigned int)GENMASK_INPUT_CHECK(h, l) +		\
+#define GENMASK_TYPE(t, h, l)						\
+	((unsigned int)GENMASK_INPUT_CHECK(h, l, BITS_PER_TYPE(t)) +	\
 	 ((t)-1 << (l) & (t)-1 >> (BITS_PER_TYPE(t) - 1 - (h))))
 #endif
 
@@ -52,16 +67,26 @@
 #if !defined(__ASSEMBLY__)
 /*
  * Fixed-type variants of BIT(), with additional checks like GENMASK_TYPE().
- * The following examples generate compiler warnings from BIT_INPUT_CHECK().
+ * The following examples generate compiler errors from BIT_INPUT_CHECK().
  *
  * - BIT_U8(8)
  * - BIT_U32(-1)
  * - BIT_U32(40)
  */
-#define BIT_INPUT_CHECK(type, nr) \
-	BUILD_BUG_ON_ZERO(const_true((nr) >= BITS_PER_TYPE(type)))
 
-#define BIT_TYPE(type, nr) ((unsigned int)BIT_INPUT_CHECK(type, nr) + ((type)1 << (nr)))
+#ifndef KBUILD_EXTRA_WARNc
+#define BIT_INPUT_CHECK(nr, width) 0
+#else
+int BIT_INPUT_CHECK_FAIL(void) __compiletime_error("Bit number out of range");
+#define BIT_INPUT_CHECK(nr, width)						\
+	(__builtin_choose_expr(__is_constexpr(nr),				\
+		sizeof(struct { char bit_number_too_big[-((nr) >= (width))];}),	\
+		__builtin_constant_p(nr) && ((nr) < 0 || (nr) >= width) &&	\
+			BIT_INPUT_CHECK_FAIL()))
+#endif
+
+#define BIT_TYPE(type, nr) \
+	((unsigned int)BIT_INPUT_CHECK(+(nr), BITS_PER_TYPE(type)) + ((type)1 << (nr)))
 #endif /* defined(__ASSEMBLY__) */
 
 #define BIT_U8(nr)	BIT_TYPE(u8, nr)
diff --git a/lib/tests/test_bits.c b/lib/tests/test_bits.c
index 55be8230f9e7..36eb4661e78b 100644
--- a/lib/tests/test_bits.c
+++ b/lib/tests/test_bits.c
@@ -3,6 +3,8 @@
  * Test cases for functions and macros in bits.h
  */
 
+#define KBUILD_EXTRA_WARNc 1
+
 #include <kunit/test.h>
 #include <linux/bits.h>
 #include <linux/types.h>
@@ -118,24 +120,30 @@ static void genmask_u128_test(struct kunit *test)
 
 static void genmask_input_check_test(struct kunit *test)
 {
-	unsigned int x, y;
-	int z, w;
+	unsigned int x = 1, y = 2;
+	int z = 1, w = 2;
+
+	OPTIMIZER_HIDE_VAR(x);
+	OPTIMIZER_HIDE_VAR(y);
+	OPTIMIZER_HIDE_VAR(z);
+	OPTIMIZER_HIDE_VAR(w);
 
 	/* Unknown input */
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(x, 0));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(0, x));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(x, y));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(x, 0, 32));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(0, x, 32));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(x, y, 32));
 
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(z, 0));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(0, z));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(z, w));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(z, 0, 32));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(0, z, 32));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(z, w, 32));
 
 	/* Valid input */
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(1, 1));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(39, 21));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(100, 80));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(110, 65));
-	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(127, 0));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(1, 1, 32));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(39, 21, 64));
+
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(100, 80, 128));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(110, 65, 128));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(127, 0, 128));
 }
 
 
-- 
2.39.5