Hi Dan,
On 18/12/2025 at 20:36, Dan Carpenter wrote:
> On Thu, Dec 18, 2025 at 07:50:01PM +0100, Vincent Mailhol wrote:
(...)
>> With this, remove gcc's -Wtype-limits. People who still want to catch
>> incorrect comparisons between unsigned integers and zero can now use
>> sparse instead.
>>
>> On a side note, clang also has a -Wtype-limits warning but:
>>
>> * it is not enabled in the kernel at the moment because, contrary to
>> gcc, clang did not include it under -Wextra.
>>
>> * it does not warn if the code results from a macro expansion. So,
>> if activated, it would not cause as much spam as gcc does.
>>
>> * -Wtype-limits is split into four sub-warnings [3] meaning that if
>> it were to be activated, we could select which one to keep.
>>
>
> Sounds good. I like your Sparse check.
Does it mean I have your Reviewed-by?
> Maybe we should enable the Sparse checking as well because it sounds
> like they are doing a lot of things right.
I am not sure to understand what do you mean by "enable the Sparse checking"?
The new sparse check I introduced is on by default.
> I think Smatch catches the
> same bugs that Clang would but it would be good to have multiple
> implementations. The -Wtautological-unsigned-enum-zero-compare trips
> people up because they aren't necessarily expecting enums to be
> unsigned.
I do not know enough about Smatch, I will let you judge on that one.
Concerning clang, here are the statistics:
$ make -s LLVM=1 CFLAGS_KERNEL="-Wtype-limits" 2>&1 | grep -o
'\[-W\S*\]' | sort | uniq -c
2 [-Wtautological-type-limit-compare]
15 [-Wtautological-unsigned-enum-zero-compare]$ make -s LLVM=1
CFLAGS_KERNEL="-Wtype-limits"
(done on a linux v6.19-rc1 defconfig with clang v20.1.8)
Not so many warnings, at least, less than what I would have thought!
-Wtautological-unsigned-char-zero-compare and
-Wtautological-unsigned-zero-compare gave zero findings. So those two
can be enabled, I guess? I am still surprised that
-Wtautological-unsigned-zero-compare gives nothing. I would have
expected some kind of false positives on that one. No sure if I missed
something here.
The two -Wtautological-type-limit-compare are:
fs/libfs.c:1640:20: warning: result of comparison 'u64' (aka 'unsigned
long long') > 18446744073709551615 is always false
[-Wtautological-type-limit-compare]
1640 | (last_fs_page > (pgoff_t)(~0ULL))) {
| ~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~
1 warning generated.
block/ioctl.c:765:29: warning: result of comparison 'sector_t' (aka
'unsigned long long') > 18446744073709551615 is always false
[-Wtautological-type-limit-compare]
765 | if (bdev_nr_sectors(bdev) > ~0UL)
| ~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~
1 warning generated.
If I got it correctly, those checks are just meant for the case where
unsigned long are 32 bits.
Because clang does not warn when the code comes from a macro
expansion, a way to silent these would be to use:
(last_fs_page > type_max(pgoff_t))
in fs/libfs.c and:
if (bdev_nr_sectors(bdev) > ULONG_MAX)
in block/ioctl.c.
Well, none of those findings were incorrect to begin with, but
arguably, the code readability can be improved.
So, I would say why not for -Wtautological-type-limit-compare.
Concerning the -Wtautological-unsigned-enum-zero-compare, here is a
representative finding:
drivers/video/hdmi.c:1099:20: warning: result of comparison of unsigned
enum expression < 0 is always false [-Wtautological-unsigned-enum-zero-compare]
1099 | if (active_aspect < 0 || active_aspect > 0xf)
| ~~~~~~~~~~~~~ ^ ~
(all the other 14 findings follow the same pattern).
Here, the code just want to check that a value is in range. This is
the same logic as gcc's -Wtype-limits: something we do *not* want.
So -Wtautological-unsigned-enum-zero-compare will stay disabled.
In conclusion, I agree that we could enable three of clang's
-Wtype-limits sub-warning. But this is not the scope of that series. I
would rather prefer to have this as a separate series.
Yours sincerely,
Vincent Mailhol
