linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
@ 2025-12-18 18:50 Vincent Mailhol
  2025-12-18 18:50 ` [PATCH 1/2] " Vincent Mailhol
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Vincent Mailhol @ 2025-12-18 18:50 UTC (permalink / raw)
  To: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba
  Cc: Linus Torvalds, linux-kbuild, linux-sparse, linux-kernel, llvm,
	dri-devel, linux-btrfs, Vincent Mailhol

I often read on the mailing list people saying "who cares about W=2
builds anyway?". At least I do. Not that I want to fix all of them,
but on some occasions, such as new driver submissions, I have often
found a couple valid diagnostics in the W=2 output.

That said, the annoying thing is that W=2 is heavily polluted by one
warning: -Wtype-limits. Try a gcc W=2 build on any file and see the
results for yourself. I suspect this to be the reason why so few
people are using W=2.

This series removes gcc's -Wtype-limits in an attempt to make W=2 more
useful. Those who do not use W=2 can continue to not use it if they
want. Those who, like me, use it form time to time will get an
improved experience from the reduced spam.

Extra details on statistics, past attempts and -Wtype-limits
alternatives are given in the first patch description.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Vincent Mailhol (2):
      kbuild: remove gcc's -Wtype-limits
      kbuild: cleanup local -Wno-type-limits exceptions

 drivers/gpu/drm/Makefile | 1 -
 fs/btrfs/Makefile        | 1 -
 scripts/Makefile.warn    | 4 +++-
 3 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: 3e7f562e20ee87a25e104ef4fce557d39d62fa85
change-id: 20251205-remove_wtype-limits-c77eb46d09c2

Best regards,
-- 
Vincent Mailhol <mailhol@kernel.org>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 18:50 [PATCH 0/2] kbuild: remove gcc's -Wtype-limits Vincent Mailhol
@ 2025-12-18 18:50 ` Vincent Mailhol
  2025-12-18 19:36   ` Dan Carpenter
  2025-12-18 18:50 ` [PATCH 2/2] kbuild: cleanup local -Wno-type-limits exceptions Vincent Mailhol
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Vincent Mailhol @ 2025-12-18 18:50 UTC (permalink / raw)
  To: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba
  Cc: Linus Torvalds, linux-kbuild, linux-sparse, linux-kernel, llvm,
	dri-devel, linux-btrfs, Vincent Mailhol

W=2 builds are heavily polluted by the -Wtype-limits warning.

Here are some W=12 statistics on Linux v6.19-rc1 for an x86_64
defconfig (with just CONFIG_WERROR set to "n") using gcc 14.3.1:

	 Warning name			count	percent
	-------------------------------------------------
	 -Wlogical-op			    2	  0.00 %
	 -Wmaybe-uninitialized		  138	  0.20 %
	 -Wunused-macros		  869	  1.24 %
	 -Wmissing-field-initializers	 1418	  2.02 %
	 -Wshadow			 2234	  3.19 %
	 -Wtype-limits			65378	 93.35 %
	-------------------------------------------------
	 Total				70039	100.00 %

As we can see, -Wtype-limits represents the vast majority of all
warnings. The reason behind this is that these warnings appear in
some common header files, meaning that some unique warnings are
repeated tens of thousands of times (once per header inclusion).

Add to this the fact that each warning is coupled with a dozen lines
detailing some macro expansion. The end result is that the W=2 output
is just too bloated and painful to use.

Three years ago, I proposed in [1] modifying one such header to
silence that noise. Because the code was not faulty, Linus rejected
the idea and instead suggested simply removing that warning.

At that time, I could not bring myself to send such a patch because,
despite its problems, -Wtype-limits would still catch the below bug:

	unsigned int ret;

	ret = check();
	if (ret < 0)
		error();

Meanwhile, based on another suggestion from Linus, I added a new check
to sparse [2] that would catch the above bug without the useless spam.

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.

So there is no present need to explicitly disable -Wtype-limits in
clang.

[1] linux/bits.h: GENMASK_INPUT_CHECK: reduce W=2 noise by 31% treewide
Link: https://lore.kernel.org/all/20220308141201.2343757-1-mailhol.vincent@wanadoo.fr/

[2] Warn about "unsigned value that used to be signed against zero"
Link: https://lore.kernel.org/all/20250921061337.3047616-1-mailhol@kernel.org/

[3] clang's -Wtype-limits
Link: https://clang.llvm.org/docs/DiagnosticsReference.html#wtype-limits

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 scripts/Makefile.warn | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.warn b/scripts/Makefile.warn
index 68e6fafcb80c..c593ab1257de 100644
--- a/scripts/Makefile.warn
+++ b/scripts/Makefile.warn
@@ -55,6 +55,9 @@ else
 KBUILD_CFLAGS += -Wno-main
 endif
 
+# Too noisy on range checks and in macros handling both signed and unsigned.
+KBUILD_CFLAGS += -Wno-type-limits
+
 # These result in bogus false positives
 KBUILD_CFLAGS += $(call cc-option, -Wno-dangling-pointer)
 
@@ -174,7 +177,6 @@ else
 
 # The following turn off the warnings enabled by -Wextra
 KBUILD_CFLAGS += -Wno-missing-field-initializers
-KBUILD_CFLAGS += -Wno-type-limits
 KBUILD_CFLAGS += -Wno-shift-negative-value
 
 ifdef CONFIG_CC_IS_CLANG

-- 
2.51.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] kbuild: cleanup local -Wno-type-limits exceptions
  2025-12-18 18:50 [PATCH 0/2] kbuild: remove gcc's -Wtype-limits Vincent Mailhol
  2025-12-18 18:50 ` [PATCH 1/2] " Vincent Mailhol
@ 2025-12-18 18:50 ` Vincent Mailhol
  2025-12-18 20:24   ` David Sterba
  2025-12-18 20:26 ` [PATCH 0/2] kbuild: remove gcc's -Wtype-limits David Laight
  2025-12-19  7:33 ` Nicolas Schier
  3 siblings, 1 reply; 15+ messages in thread
From: Vincent Mailhol @ 2025-12-18 18:50 UTC (permalink / raw)
  To: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba
  Cc: Linus Torvalds, linux-kbuild, linux-sparse, linux-kernel, llvm,
	dri-devel, linux-btrfs, Vincent Mailhol

Now that -Wno-type-limits is globally deactivated, there is no need
for local exceptions anymore.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/gpu/drm/Makefile | 1 -
 fs/btrfs/Makefile        | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0e1c668b46d2..b879a60ca79a 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -22,7 +22,6 @@ subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
 # The following turn off the warnings enabled by -Wextra
 ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
 subdir-ccflags-y += -Wno-missing-field-initializers
-subdir-ccflags-y += -Wno-type-limits
 subdir-ccflags-y += -Wno-shift-negative-value
 endif
 ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 743d7677b175..40bc2f7e6f6b 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -17,7 +17,6 @@ subdir-ccflags-y += $(condflags)
 # The following turn off the warnings enabled by -Wextra
 subdir-ccflags-y += -Wno-missing-field-initializers
 subdir-ccflags-y += -Wno-sign-compare
-subdir-ccflags-y += -Wno-type-limits
 subdir-ccflags-y += -Wno-shift-negative-value
 
 obj-$(CONFIG_BTRFS_FS) := btrfs.o

-- 
2.51.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 18:50 ` [PATCH 1/2] " Vincent Mailhol
@ 2025-12-18 19:36   ` Dan Carpenter
  2025-12-18 22:31     ` Vincent Mailhol
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2025-12-18 19:36 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba, Linus Torvalds, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Thu, Dec 18, 2025 at 07:50:01PM +0100, Vincent Mailhol wrote:
> W=2 builds are heavily polluted by the -Wtype-limits warning.
> 
> Here are some W=12 statistics on Linux v6.19-rc1 for an x86_64
> defconfig (with just CONFIG_WERROR set to "n") using gcc 14.3.1:
> 
> 	 Warning name			count	percent
> 	-------------------------------------------------
> 	 -Wlogical-op			    2	  0.00 %
> 	 -Wmaybe-uninitialized		  138	  0.20 %
> 	 -Wunused-macros		  869	  1.24 %
> 	 -Wmissing-field-initializers	 1418	  2.02 %
> 	 -Wshadow			 2234	  3.19 %
> 	 -Wtype-limits			65378	 93.35 %
> 	-------------------------------------------------
> 	 Total				70039	100.00 %
> 
> As we can see, -Wtype-limits represents the vast majority of all
> warnings. The reason behind this is that these warnings appear in
> some common header files, meaning that some unique warnings are
> repeated tens of thousands of times (once per header inclusion).
> 
> Add to this the fact that each warning is coupled with a dozen lines
> detailing some macro expansion. The end result is that the W=2 output
> is just too bloated and painful to use.
> 
> Three years ago, I proposed in [1] modifying one such header to
> silence that noise. Because the code was not faulty, Linus rejected
> the idea and instead suggested simply removing that warning.
> 
> At that time, I could not bring myself to send such a patch because,
> despite its problems, -Wtype-limits would still catch the below bug:
> 
> 	unsigned int ret;
> 
> 	ret = check();
> 	if (ret < 0)
> 		error();
> 
> Meanwhile, based on another suggestion from Linus, I added a new check
> to sparse [2] that would catch the above bug without the useless spam.
> 
> 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.

Maybe we should enable the Sparse checking as well because it sounds
like they are doing a lot of things right.  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.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] kbuild: cleanup local -Wno-type-limits exceptions
  2025-12-18 18:50 ` [PATCH 2/2] kbuild: cleanup local -Wno-type-limits exceptions Vincent Mailhol
@ 2025-12-18 20:24   ` David Sterba
  0 siblings, 0 replies; 15+ messages in thread
From: David Sterba @ 2025-12-18 20:24 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba, Linus Torvalds, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Thu, Dec 18, 2025 at 07:50:02PM +0100, Vincent Mailhol wrote:
> Now that -Wno-type-limits is globally deactivated, there is no need
> for local exceptions anymore.
> 
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---

>  fs/btrfs/Makefile        | 1 -

Acked-by: David Sterba <dsterba@suse.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 18:50 [PATCH 0/2] kbuild: remove gcc's -Wtype-limits Vincent Mailhol
  2025-12-18 18:50 ` [PATCH 1/2] " Vincent Mailhol
  2025-12-18 18:50 ` [PATCH 2/2] kbuild: cleanup local -Wno-type-limits exceptions Vincent Mailhol
@ 2025-12-18 20:26 ` David Laight
  2025-12-18 20:34   ` Linus Torvalds
  2025-12-19  7:33 ` Nicolas Schier
  3 siblings, 1 reply; 15+ messages in thread
From: David Laight @ 2025-12-18 20:26 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba, Linus Torvalds, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Thu, 18 Dec 2025 19:50:00 +0100
Vincent Mailhol <mailhol@kernel.org> wrote:

> I often read on the mailing list people saying "who cares about W=2
> builds anyway?". At least I do. Not that I want to fix all of them,
> but on some occasions, such as new driver submissions, I have often
> found a couple valid diagnostics in the W=2 output.
> 
> That said, the annoying thing is that W=2 is heavily polluted by one
> warning: -Wtype-limits. Try a gcc W=2 build on any file and see the
> results for yourself. I suspect this to be the reason why so few
> people are using W=2.

One possibility is to conditionally add _Pragma() inside #defines to
turn off the warning for the main false positives (I guess all the
BUILD_BUG_xxxx and statically_true are the main ones).
But don't 'bloat' the #define expansions for normal builds.

	David

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 20:26 ` [PATCH 0/2] kbuild: remove gcc's -Wtype-limits David Laight
@ 2025-12-18 20:34   ` Linus Torvalds
  2025-12-18 22:06     ` David Laight
  0 siblings, 1 reply; 15+ messages in thread
From: Linus Torvalds @ 2025-12-18 20:34 UTC (permalink / raw)
  To: David Laight
  Cc: Vincent Mailhol, Nathan Chancellor, Nicolas Schier,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Chris Mason, David Sterba, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Fri, 19 Dec 2025 at 08:26, David Laight <david.laight.linux@gmail.com> wrote:
>
> One possibility is to conditionally add _Pragma()

No. That compiler warning is pure and utter garbage. I have pointed it
out fopr *years*, and compiler people don't get it.

So that warning just needs to die. It's shit. It's wrong.

The sparse patch points out that this *can* be done correctly if you a
compiler person doesn't have their head up their arse.

(And no, I'm not claiming the sparse patch is perfect. I'm only
claiming the sparse patch is _much_ better. Bit tt could be better
still, and there could be other valid cases that could be warned for).

The "warn on type limits" is idiotic. It expects programmers to have
to always track what the exact type limits are, instead of just
writing safe and obvious code, and it warns about *good* code and.

It's exactly the *wrong* kind of thing to warn about.

               Linus

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 20:34   ` Linus Torvalds
@ 2025-12-18 22:06     ` David Laight
  2025-12-18 22:19       ` Linus Torvalds
  2025-12-19  7:08       ` Dan Carpenter
  0 siblings, 2 replies; 15+ messages in thread
From: David Laight @ 2025-12-18 22:06 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Vincent Mailhol, Nathan Chancellor, Nicolas Schier,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Chris Mason, David Sterba, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Fri, 19 Dec 2025 08:34:05 +1200
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, 19 Dec 2025 at 08:26, David Laight <david.laight.linux@gmail.com> wrote:
> >
> > One possibility is to conditionally add _Pragma()  
> 
> No. That compiler warning is pure and utter garbage. I have pointed it
> out fopr *years*, and compiler people don't get it.
> 
> So that warning just needs to die. It's shit. It's wrong.

True - especially for code like:
	if (x < 0 || x > limit)
		return ...
where the code is correct even with 'accidental' conversion of a
negative signed value to a large unsigned one.

clang seems to have a dozen similar warnings, all of which are a PITA
for kernel code - like rejecting !(4 << 16).

_Pragma() might be usable for -Wshadow, which is generally useful for
local variables (but not global functions like log() and j0()).
(I usually enable it and fix up the consequences.)
Things like the masked userspace access define which carefully
creates a readonly variable that shadows a user local would need
to disable that one.

	David


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 22:06     ` David Laight
@ 2025-12-18 22:19       ` Linus Torvalds
  2025-12-19  7:08       ` Dan Carpenter
  1 sibling, 0 replies; 15+ messages in thread
From: Linus Torvalds @ 2025-12-18 22:19 UTC (permalink / raw)
  To: David Laight
  Cc: Vincent Mailhol, Nathan Chancellor, Nicolas Schier,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Chris Mason, David Sterba, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Fri, 19 Dec 2025 at 10:06, David Laight <david.laight.linux@gmail.com> wrote:
>
> True - especially for code like:
>         if (x < 0 || x > limit)
>                 return ...

Exactly.

And yes, sometimes the type of 'x' is obvious, and having the range
check for zero can be seen as redundant for unsigned types, but even
in that "obviously redundant" case the code is *clearer* with both the
lower and upper range checked.

And apart from being clearer, it's also then safe when somebody does
change the type for whatever reason.

And lots of types do *not* have obvious signedness. They might be
typedefs, or have other much subtler issues. Something as simple as
"char" has subtle sign behavior, and when it comes to things like
enums the signedness can also be very non-obvious.

So having both sides of a range check is *always* a good idea, even if
one side _may_ be redundant for some type-range reasons.

And there really is absolutely _no_ sane way to get rid of that broken
warning except to just disable the warning itself. All other
alternatives are actively broken - adding a Pragma only makes the code
worse and illegible, and removing the lower bounds check again only
makes the code worse.

So this is a compiler warning that actively encourages worse code. It
needs to *die*. It doesn't fix anything.

And the people who point out that it can show bugs - absolutely *ANY*
warning can do that. That doesn't make a warning good. Any code can
have bugs in it.

The sparse warning I outlined (and that Vincent wrote up and tested
and made into a proper patch) was actually showing interesting issues
in a much better way.

And that sparse warning could certainly be improved on too - I think
that one too would be better if it noticed "oh, it's a pure range
check, so let's not warn even when the code otherwise looks dodgy".

But at least it didn't warn for obviously good code like the horrid
broken type-range warning does.

                  Linus

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 19:36   ` Dan Carpenter
@ 2025-12-18 22:31     ` Vincent Mailhol
  2025-12-19  6:56       ` Dan Carpenter
  2025-12-19 22:21       ` Vincent Mailhol
  0 siblings, 2 replies; 15+ messages in thread
From: Vincent Mailhol @ 2025-12-18 22:31 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba, Linus Torvalds, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 22:31     ` Vincent Mailhol
@ 2025-12-19  6:56       ` Dan Carpenter
  2025-12-19 22:21       ` Vincent Mailhol
  1 sibling, 0 replies; 15+ messages in thread
From: Dan Carpenter @ 2025-12-19  6:56 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba, Linus Torvalds, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On Thu, Dec 18, 2025 at 11:31:40PM +0100, Vincent Mailhol wrote:
> 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"?

I meant Clang...  Sorry.  Doh.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 22:06     ` David Laight
  2025-12-18 22:19       ` Linus Torvalds
@ 2025-12-19  7:08       ` Dan Carpenter
  1 sibling, 0 replies; 15+ messages in thread
From: Dan Carpenter @ 2025-12-19  7:08 UTC (permalink / raw)
  To: David Laight
  Cc: Linus Torvalds, Vincent Mailhol, Nathan Chancellor,
	Nicolas Schier, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Chris Mason, David Sterba, linux-kbuild,
	linux-sparse, linux-kernel, llvm, dri-devel, linux-btrfs

On Thu, Dec 18, 2025 at 10:06:51PM +0000, David Laight wrote:
> On Fri, 19 Dec 2025 08:34:05 +1200
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Fri, 19 Dec 2025 at 08:26, David Laight <david.laight.linux@gmail.com> wrote:
> > >
> > > One possibility is to conditionally add _Pragma()  
> > 
> > No. That compiler warning is pure and utter garbage. I have pointed it
> > out fopr *years*, and compiler people don't get it.
> > 
> > So that warning just needs to die. It's shit. It's wrong.
> 
> True - especially for code like:
> 	if (x < 0 || x > limit)
> 		return ...
> where the code is correct even with 'accidental' conversion of a
> negative signed value to a large unsigned one.
> 
> clang seems to have a dozen similar warnings, all of which are a PITA
> for kernel code - like rejecting !(4 << 16).

In this example is 4 a literal or do we at least know that 4 is
non-zero?

I really thought I had a check for that in Smatch but I guess I
don't...

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 18:50 [PATCH 0/2] kbuild: remove gcc's -Wtype-limits Vincent Mailhol
                   ` (2 preceding siblings ...)
  2025-12-18 20:26 ` [PATCH 0/2] kbuild: remove gcc's -Wtype-limits David Laight
@ 2025-12-19  7:33 ` Nicolas Schier
  2025-12-19 22:06   ` Vincent Mailhol
  3 siblings, 1 reply; 15+ messages in thread
From: Nicolas Schier @ 2025-12-19  7:33 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Chris Mason, David Sterba, Linus Torvalds,
	linux-kbuild, linux-sparse, linux-kernel, llvm, dri-devel,
	linux-btrfs

On Thu, Dec 18, 2025 at 07:50:00PM +0100, Vincent Mailhol wrote:
> I often read on the mailing list people saying "who cares about W=2
> builds anyway?". At least I do. Not that I want to fix all of them,
> but on some occasions, such as new driver submissions, I have often
> found a couple valid diagnostics in the W=2 output.
> 
> That said, the annoying thing is that W=2 is heavily polluted by one
> warning: -Wtype-limits. Try a gcc W=2 build on any file and see the
> results for yourself. I suspect this to be the reason why so few
> people are using W=2.
> 
> This series removes gcc's -Wtype-limits in an attempt to make W=2 more
> useful. Those who do not use W=2 can continue to not use it if they
> want. Those who, like me, use it form time to time will get an
> improved experience from the reduced spam.
> 
> Extra details on statistics, past attempts and -Wtype-limits
> alternatives are given in the first patch description.
> 
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
> Vincent Mailhol (2):
>       kbuild: remove gcc's -Wtype-limits
>       kbuild: cleanup local -Wno-type-limits exceptions
> 
>  drivers/gpu/drm/Makefile | 1 -
>  fs/btrfs/Makefile        | 1 -
>  scripts/Makefile.warn    | 4 +++-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> ---
> base-commit: 3e7f562e20ee87a25e104ef4fce557d39d62fa85
> change-id: 20251205-remove_wtype-limits-c77eb46d09c2
> 
> Best regards,
> -- 
> Vincent Mailhol <mailhol@kernel.org>
> 

Thanks for the effort!  (This allows to revert commit dc7fe518b049
("overflow: Fix -Wtype-limits compilation warnings").)

Reviewed-by: Nicolas Schier <nsc@kernel.org>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] kbuild: remove gcc's -Wtype-limits
  2025-12-19  7:33 ` Nicolas Schier
@ 2025-12-19 22:06   ` Vincent Mailhol
  0 siblings, 0 replies; 15+ messages in thread
From: Vincent Mailhol @ 2025-12-19 22:06 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Nick Desaulniers, Bill Wendling, Justin Stitt, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Chris Mason, David Sterba, Linus Torvalds, linux-kbuild,
	linux-sparse, linux-kernel, llvm, dri-devel, linux-btrfs

On 19/12/2025 at 08:33, Nicolas Schier wrote:
> On Thu, Dec 18, 2025 at 07:50:00PM +0100, Vincent Mailhol wrote:

(...)

> Thanks for the effort!  (This allows to revert commit dc7fe518b049
> ("overflow: Fix -Wtype-limits compilation warnings").)

Ack. I will send a v2 with more patches to cleanup some -Wtype-limits workarounds.

> Reviewed-by: Nicolas Schier <nsc@kernel.org>

Thanks!

Yours sincerely,
Vincent Mailhol


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] kbuild: remove gcc's -Wtype-limits
  2025-12-18 22:31     ` Vincent Mailhol
  2025-12-19  6:56       ` Dan Carpenter
@ 2025-12-19 22:21       ` Vincent Mailhol
  1 sibling, 0 replies; 15+ messages in thread
From: Vincent Mailhol @ 2025-12-19 22:21 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Nathan Chancellor, Nicolas Schier, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Chris Mason,
	David Sterba, Linus Torvalds, linux-kbuild, linux-sparse,
	linux-kernel, llvm, dri-devel, linux-btrfs

On 18/12/2025 at 23:31, Vincent Mailhol wrote:

(...)

> 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.

I was a bit worried of that -Wtautological-unsigned-zero-compare got
zero findings so I reran a build but this time on an allyesconfig
(minus CONFIG_WERROR):

	$ make -j8 -s LLVM=1 CFLAGS_KERNEL="-Wtype-limits" 2>&1 | grep -o '\[-W\S*\]' | sort | uniq -c
	     29 [-Wtautological-type-limit-compare]
	     55 [-Wtautological-unsigned-enum-zero-compare]
	     76 [-Wtautological-unsigned-zero-compare]

This is closer than expected. And looking at the findings,
-Wtautological-unsigned-zero-compare also warns on some sane code
which is just doing some range checks.

(...)

> 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.

With this, I want to amend my conclusion. both
-Wtautological-unsigned-enum-zero-compare and
-Wtautological-unsigned-zero-compare should be kept disabled. The only
candidates are -Wtautological-type-limit-compare and
-Wtautological-unsigned-char-zero-compare.

-Wtautological-unsigned-char-zero-compare would need another study. It
seems that this warning is only triggered on platforms where char is
unsigned which explains why I did not see it when building on x86_64.

Well, I think I will stop this clang's -Wtype-limits study for the
moment. If anyone wants to continue the work please go ahead.


Yours sincerely,
Vincent Mailhol

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-12-19 22:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-18 18:50 [PATCH 0/2] kbuild: remove gcc's -Wtype-limits Vincent Mailhol
2025-12-18 18:50 ` [PATCH 1/2] " Vincent Mailhol
2025-12-18 19:36   ` Dan Carpenter
2025-12-18 22:31     ` Vincent Mailhol
2025-12-19  6:56       ` Dan Carpenter
2025-12-19 22:21       ` Vincent Mailhol
2025-12-18 18:50 ` [PATCH 2/2] kbuild: cleanup local -Wno-type-limits exceptions Vincent Mailhol
2025-12-18 20:24   ` David Sterba
2025-12-18 20:26 ` [PATCH 0/2] kbuild: remove gcc's -Wtype-limits David Laight
2025-12-18 20:34   ` Linus Torvalds
2025-12-18 22:06     ` David Laight
2025-12-18 22:19       ` Linus Torvalds
2025-12-19  7:08       ` Dan Carpenter
2025-12-19  7:33 ` Nicolas Schier
2025-12-19 22:06   ` Vincent Mailhol

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).