From nobody Wed Sep 17 03:32:45 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 54798C4332F for ; Thu, 22 Dec 2022 22:33:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230479AbiLVWd6 (ORCPT ); Thu, 22 Dec 2022 17:33:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38774 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229613AbiLVWdz (ORCPT ); Thu, 22 Dec 2022 17:33:55 -0500 Received: from eu-smtp-delivery-151.mimecast.com (eu-smtp-delivery-151.mimecast.com [185.58.86.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5B64F2126E for ; Thu, 22 Dec 2022 14:33:53 -0800 (PST) Received: from AcuMS.aculab.com (156.67.243.121 [156.67.243.121]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id uk-mta-15-XJ80X7-gOhqXhTMVDmvTZg-1; Thu, 22 Dec 2022 22:33:50 +0000 X-MC-Unique: XJ80X7-gOhqXhTMVDmvTZg-1 Received: from AcuMS.Aculab.com (10.202.163.4) by AcuMS.aculab.com (10.202.163.4) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Thu, 22 Dec 2022 22:33:49 +0000 Received: from AcuMS.Aculab.com ([::1]) by AcuMS.aculab.com ([::1]) with mapi id 15.00.1497.044; Thu, 22 Dec 2022 22:33:49 +0000 From: David Laight To: David Laight , 'Linus Torvalds' CC: LKML , Andy Shevchenko , Andrew Morton , Steven Rostedt , "Joe Perches" Subject: Subject: [RFC PATCH v2 1/1] Slightly relax the type checking done by min() and max(). Thread-Topic: Subject: [RFC PATCH v2 1/1] Slightly relax the type checking done by min() and max(). Thread-Index: AdkWVXECPhRKN4bxSfe9G1rGTLTfYw== Date: Thu, 22 Dec 2022 22:33:49 +0000 Message-ID: References: <58cac72242e54380971cfa842f824470@AcuMS.aculab.com> <433b8b44fe6e43b2b576c311bb55cc8a@AcuMS.aculab.com> <6dc9a68ca3394a3c94164fd7cff9fa47@AcuMS.aculab.com> In-Reply-To: <6dc9a68ca3394a3c94164fd7cff9fa47@AcuMS.aculab.com> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: aculab.com Content-Language: en-US Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Slightly relax the type checking done by min() and max(). Relax the type checking done by min() and max() to: - Allow comparisons of any two signed values. - Allow comparisons of any two unsigned values. - Allow comparisons where an unsigned value is promoted to a signed type. Typically u8 and u16 variables, but also u32 against s64. - Allow comparisons of unsigned values against non-negative signed constant= s. So min(unsigned_var, 5) and min(unsigned_var, 5u) are both allowed. Comparisons of signed values against 31-bit unsigned constants are disallow= ed. In particular min(signed_var, sizeof(...)) is disallowed. Two new pairs of functions are added: min_signed() and max_signed() These are identical to min() and max() except that signed values can be compared against 31-bit unsigned constants. So min_signed(signed_var, sizeof(...)) is allowed and can be negative. min_unsigned() and max_unsigned() These promote their arguments to unsigned types before the comparison. They should be used when a signed expression is known to be positive. Unlike min_t(unsigned_type, x, y) high bits of x and y will not be discarded if the wrong 'unsigned_type' is picked. These reduce the need to use min_t/max_t() and the possibly unwanted side effects if a type that is too small is specified. It isn't hard to find places where 'a =3D min(b, c)' has been replaced with 'a =3D min_t(typeof(a), b, c)' due to a type mismatch between b and = c. But the correct type is an unsigned type not smaller than b or c. Changes for v2: - factor out compile type check using: +#define __typed_null(type, value) (1 ? ((void *)((long)(value))) : (type= *)0) - Disallow min(signed_var, 32u) (Linus didn't like it). - Allow for 32bit unsigned values being promoted to 64bit signed ones. - Use explicit tests rather than a comparison on the pointer types. - Bug fixes Posted as an RFC because it doesn't really make sense to have min_unsigned() since min_signed() will pretty much always have the same effect and the extra function just adds confusion. But I still think that min(signed_var, 16u) should be valid so it could be squashed out the other way. --- include/linux/minmax.h | 127 ++++++++++++++++++++++++++++++++++------- 1 file changed, 106 insertions(+), 21 deletions(-) diff --git a/include/linux/minmax.h b/include/linux/minmax.h index 5433c08fcc68..1351f1b7f9a8 100644 --- a/include/linux/minmax.h +++ b/include/linux/minmax.h @@ -9,47 +9,132 @@ * * - avoid multiple evaluations of the arguments (so side-effects like * "x++" happen only once) when non-constant. - * - perform strict type-checking (to generate warnings instead of - * nasty runtime surprises). See the "unnecessary" pointer comparison - * in __typecheck(). + * - perform type-checking (to generate warnings instead of nasty runtime + * surprises). * - retain result as a constant expressions when called with only * constant expressions (to avoid tripping VLA warnings in stack * allocation usage). + * + * The type-check normally allows: + * - comparison of two signed expressions. + * - comparison of two unsigned expressions. + * - comparison of unsigned char and short variables to a signed expressio= n. + * - comparison of unsigned expresions to 32bit non-negative signed consta= nts. + * The last allows min(unsigned_var, 16) as well as min(unsigned_var, 16u); + * Other comparisons between signed and unsigned values generate a compile + * time error. + * Three additional function pairs futher relax the check: + * - min_signed() allows signed expressions against 31bit unsigned constan= ts. + * So min_signed(foo, sizeof (bar)) is valid - but might be negative. + * - min_unsigned() converts both values to an unsigned type. + * So max_unsigned(foo, sizeof (bar)) is valid but can be very large + * (effectively undefined) if foo is negative. + * - min_t(type, a, b) casts both values to (type) BEFORE the comparison. + * misuse can easily cause truncated values. + */ + +/* + * Compile time test for compile time constant zero. + * The type of the result depends on the value. + * Similar to: (value ? (void *)value : (type *)0) */ -#define __typecheck(x, y) \ - (!!(sizeof((typeof(x) *)1 =3D=3D (typeof(y) *)1))) +#define __typed_null(type, value) (1 ? ((void *)((long)(value))) : (type *= )0) + +/* True if (long)x is zero */ +#define __is_constzero(x) (sizeof(*__typed_null(int, x)) =3D=3D sizeof(int= )) + +/* Compile time 'type error' if cond is zero */ +#define __type_error(cond) \ + ((int)sizeof(__typed_null(int, cond) =3D=3D (long *)0) * 0) + +/* True if x (any type) is constant in the range [0..0x7fffffff] */ +#define __is_positive(x) \ + __is_constzero((x) !=3D (typeof(x))((long)(x) & 0x7fffffffu)) + +/* True if x has a signed type */ +#define __is_signed(x) is_signed_type(typeof(x)) =20 -#define __no_side_effects(x, y) \ - (__is_constexpr(x) && __is_constexpr(y)) +/* True if x will be promoted to a larger signed type */ +#define __is_small(x, y) (sizeof(x) < sizeof((y) + 0u)) =20 -#define __safe_cmp(x, y) \ - (__typecheck(x, y) && __no_side_effects(x, y)) +/* Allow comparisons that can't promote negative signed to unsigned. */ +#define __types_compatible(x, y) \ + (__is_signed(x) ? __is_positive(x) || __is_small(y, x) || __is_signed(y) \ + : __is_positive(y) || __is_small(x, y) || !__is_signed(y)) + +/* Compile error if the types don't match, value is a constant 0. */ +#define __typecheck(__allow_const, x, y) \ + __type_error(__types_compatible(x, y) || (__allow_const && \ + (__is_positive(x) || __is_positive(y)))) + +/* Cast positive constants to int, they may get promoted back to unsigned.= */ +#define __maybe_int_cast(x) \ + __builtin_choose_expr(__is_positive(x), (int)(long)(x), x) =20 #define __cmp(x, y, op) ((x) op (y) ? (x) : (y)) =20 #define __cmp_once(x, y, unique_x, unique_y, op) ({ \ - typeof(x) unique_x =3D (x); \ - typeof(y) unique_y =3D (y); \ - __cmp(unique_x, unique_y, op); }) + typeof(__maybe_int_cast(x)) unique_x =3D (x); \ + typeof(__maybe_int_cast(y)) unique_y =3D (y); \ + __cmp(unique_x, unique_y, op); }) + +#define __careful_cmp(__allow_const, x, y, op) \ + (__typecheck(__allow_const, x, y) + \ + __builtin_choose_expr(__is_constexpr(x) && __is_constexpr(y), \ + __cmp(__maybe_int_cast(x), __maybe_int_cast(y), op), \ + __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))) + +/** + * min - return minimum of two values of the same or compatible types. + * Unsigned values may be compared against positive signed constants. + * min(unsigned_expr, 5) is valid but min(signed_expr, 5u) is not. + * @x: first value + * @y: second value + */ +#define min(x, y) __careful_cmp(0, x, y, <) =20 -#define __careful_cmp(x, y, op) \ - __builtin_choose_expr(__safe_cmp(x, y), \ - __cmp(x, y, op), \ - __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op)) +/** + * min_signed - return minimum of two values of the same or compatible typ= es. + * Values may be compared against positive constants. + * Unlike min(), min_signed(signed_expr, 5u) is valid and may be neg= ative. + * @x: first value + * @y: second value + */ +#define min_signed(x, y) __careful_cmp(1, x, y, <) =20 /** - * min - return minimum of two values of the same or compatible types + * min_unsigned - return minimum of two values converted to an unsigned ty= pe. + * Negative values will be treated as large positive values. * @x: first value * @y: second value */ -#define min(x, y) __careful_cmp(x, y, <) +#define min_unsigned(x, y) min((x) + 0u + 0ull, (y) + 0u + 0ull) =20 /** * max - return maximum of two values of the same or compatible types + * Unsigned values may be compared against positive signed constants. + * max(unsigned_expr, 5) is valid but max(signed_expr, 5u) is not. + * @x: first value + * @y: second value + */ +#define max(x, y) __careful_cmp(0, x, y, >) + +/** + * max_signed - return maximum of two values of the same or compatible typ= es + * Values may be compared against positive constants. + * Unlike max(), max_signed(signed_expr, 5u) is valid. + * @x: first value + * @y: second value + */ +#define max_signed(x, y) __careful_cmp(1, x, y, >) + +/** + * max_unsigned - return maximum of two values converted to an unsigned ty= pe. + * Negative values will be treated as large positive values. * @x: first value * @y: second value */ -#define max(x, y) __careful_cmp(x, y, >) +#define max_unsigned(x, y) max((x) + 0u + 0ull, (y) + 0u + 0ull) =20 /** * min3 - return minimum of three values @@ -101,7 +186,7 @@ * @x: first value * @y: second value */ -#define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) +#define min_t(type, x, y) __careful_cmp(0, (type)(x), (type)(y), <) =20 /** * max_t - return maximum of two values, using the specified type @@ -109,7 +194,7 @@ * @x: first value * @y: second value */ -#define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) +#define max_t(type, x, y) __careful_cmp(0, (type)(x), (type)(y), >) =20 /** * clamp_t - return a value clamped to a given range using a given type --=20 2.17.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1= PT, UK Registration No: 1397386 (Wales)