Re: [PATCH 1/2] kbuild: remove gcc's -Wtype-limits

[Vincent Mailhol <mailhol@kernel.org>]

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