Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

minmax: improve macro expansion and type checking

This clarifies the rules for min()/max()/clamp() type checking and makes
them a much more efficient macro expansion.

In particular, we now look at the type and range of the inputs to see
whether they work together, generating a mask of acceptable comparisons,
and then just verifying that the inputs have a shared case:

- an expression with a signed type can be used for
(1) signed comparisons
(2) unsigned comparisons if it is statically known to have a
non-negative value

- an expression with an unsigned type can be used for
(3) unsigned comparison
(4) signed comparisons if the type is smaller than 'int' and thus
the C integer promotion rules will make it signed anyway

Here rule (1) and (3) are obvious, and rule (2) is important in order to
allow obvious trivial constants to be used together with unsigned
values.

Rule (4) is not necessarily a good idea, but matches what we used to do,
and we have extant cases of this situation in the kernel. Notably with
bcachefs having an expression like

min(bch2_bucket_sectors_dirty(a), ca->mi.bucket_size)

where bch2_bucket_sectors_dirty() returns an 's64', and
'ca->mi.bucket_size' is of type 'u16'.

Technically that bcachefs comparison is clearly sensible on a C type
level, because the 'u16' will go through the normal C integer promotion,
and become 'int', and then we're comparing two signed values and
everything looks sane.

However, it's not entirely clear that a 'min(s64,u16)' operation makes a
lot of conceptual sense, and it's possible that we will remove rule (4).
After all, the _reason_ we have these complicated type checks is exactly
that the C type promotion rules are not very intuitive.

But at least for now the rule is in place for backwards compatibility.

Also note that rule (2) existed before, but is hugely relaxed by this
commit. It used to be true only for the simplest compile-time
non-negative integer constants. The new macro model will allow cases
where the compiler can trivially see that an expression is non-negative
even if it isn't necessarily a constant.

For example, the amdgpu driver does

min_t(size_t, sizeof(fru_info->serial), pia[addr] & 0x3F));

because our old 'min()' macro would see that 'pia[addr] & 0x3F' is of
type 'int' and clearly not a C constant expression, so doing a 'min()'
with a 'size_t' is a signedness violation.

Our new 'min()' macro still sees that 'pia[addr] & 0x3F' is of type
'int', but is smart enough to also see that it is clearly non-negative,
and thus would allow that case without any complaints.

Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Laight <David.Laight@aculab.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

+68 -15
+9
include/linux/compiler.h
··· 297 297 #define is_unsigned_type(type) (!is_signed_type(type)) 298 298 299 299 /* 300 + * Useful shorthand for "is this condition known at compile-time?" 301 + * 302 + * Note that the condition may involve non-constant values, 303 + * but the compiler may know enough about the details of the 304 + * values to determine that the condition is statically true. 305 + */ 306 + #define statically_true(x) (__builtin_constant_p(x) && (x)) 307 + 308 + /* 300 309 * This is needed in functions which generate the stack canary, see 301 310 * arch/x86/kernel/smpboot.c::start_secondary() for an example. 302 311 */
+59 -15
include/linux/minmax.h
··· 26 26 #define __typecheck(x, y) \ 27 27 (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) 28 28 29 - /* is_signed_type() isn't a constexpr for pointer types */ 30 - #define __is_signed(x) \ 31 - __builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \ 32 - is_signed_type(typeof(x)), 0) 29 + /* 30 + * __sign_use for integer expressions: 31 + * bit #0 set if ok for unsigned comparisons 32 + * bit #1 set if ok for signed comparisons 33 + * 34 + * In particular, statically non-negative signed integer 35 + * expressions are ok for both. 36 + * 37 + * NOTE! Unsigned types smaller than 'int' are implicitly 38 + * converted to 'int' in expressions, and are accepted for 39 + * signed conversions for now. This is debatable. 40 + * 41 + * Note that 'x' is the original expression, and 'ux' is 42 + * the unique variable that contains the value. 43 + * 44 + * We use 'ux' for pure type checking, and 'x' for when 45 + * we need to look at the value (but without evaluating 46 + * it for side effects! Careful to only ever evaluate it 47 + * with sizeof() or __builtin_constant_p() etc). 48 + * 49 + * Pointers end up being checked by the normal C type 50 + * rules at the actual comparison, and these expressions 51 + * only need to be careful to not cause warnings for 52 + * pointer use. 53 + */ 54 + #define __signed_type_use(x,ux) (2+__is_nonneg(x,ux)) 55 + #define __unsigned_type_use(x,ux) (1+2*(sizeof(ux)<4)) 56 + #define __sign_use(x,ux) (is_signed_type(typeof(ux))? \ 57 + __signed_type_use(x,ux):__unsigned_type_use(x,ux)) 33 58 34 - /* True for a non-negative signed int constant */ 35 - #define __is_noneg_int(x) \ 36 - (__builtin_choose_expr(__is_constexpr(x) && __is_signed(x), x, -1) >= 0) 59 + /* 60 + * To avoid warnings about casting pointers to integers 61 + * of different sizes, we need that special sign type. 62 + * 63 + * On 64-bit we can just always use 'long', since any 64 + * integer or pointer type can just be cast to that. 65 + * 66 + * This does not work for 128-bit signed integers since 67 + * the cast would truncate them, but we do not use s128 68 + * types in the kernel (we do use 'u128', but they will 69 + * be handled by the !is_signed_type() case). 70 + * 71 + * NOTE! The cast is there only to avoid any warnings 72 + * from when values that aren't signed integer types. 73 + */ 74 + #ifdef CONFIG_64BIT 75 + #define __signed_type(ux) long 76 + #else 77 + #define __signed_type(ux) typeof(__builtin_choose_expr(sizeof(ux)>4,1LL,1L)) 78 + #endif 79 + #define __is_nonneg(x,ux) statically_true((__signed_type(ux))(x)>=0) 37 80 38 - #define __types_ok(x, y, ux, uy) \ 39 - (__is_signed(ux) == __is_signed(uy) || \ 40 - __is_signed((ux) + 0) == __is_signed((uy) + 0) || \ 41 - __is_noneg_int(x) || __is_noneg_int(y)) 81 + #define __types_ok(x,y,ux,uy) \ 82 + (__sign_use(x,ux) & __sign_use(y,uy)) 83 + 84 + #define __types_ok3(x,y,z,ux,uy,uz) \ 85 + (__sign_use(x,ux) & __sign_use(y,uy) & __sign_use(z,uz)) 42 86 43 87 #define __cmp_op_min < 44 88 #define __cmp_op_max > ··· 97 53 98 54 #define __careful_cmp_once(op, x, y, ux, uy) ({ \ 99 55 __auto_type ux = (x); __auto_type uy = (y); \ 100 - static_assert(__types_ok(x, y, ux, uy), \ 101 - #op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \ 56 + BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \ 57 + #op"("#x", "#y") signedness error"); \ 102 58 __cmp(op, ux, uy); }) 103 59 104 60 #define __careful_cmp(op, x, y) \ ··· 114 70 static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \ 115 71 (lo) <= (hi), true), \ 116 72 "clamp() low limit " #lo " greater than high limit " #hi); \ 117 - static_assert(__types_ok(uval, lo, uval, ulo), "clamp() 'lo' signedness error"); \ 118 - static_assert(__types_ok(uval, hi, uval, uhi), "clamp() 'hi' signedness error"); \ 73 + BUILD_BUG_ON_MSG(!__types_ok3(val,lo,hi,uval,ulo,uhi), \ 74 + "clamp("#val", "#lo", "#hi") signedness error"); \ 119 75 __clamp(uval, ulo, uhi); }) 120 76 121 77 #define __careful_clamp(val, lo, hi) \