Re: [PATCH] kbuild: treat char as always signed

From: Segher Boessenkool <segher@kernel.crashing.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-toolchains@vger.kernel.org,
	Masahiro Yamada <masahiroy@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] kbuild: treat char as always signed
Date: Wed, 19 Oct 2022 12:43:45 -0500	[thread overview]
Message-ID: <20221019174345.GM25951@gate.crashing.org> (raw)
In-Reply-To: <CAHk-=wiMWk2t8FHn0iqVVe1mn62OTAD6ffL5rn9Eeu021H9d1Q@mail.gmail.com>

Hi!

On Wed, Oct 19, 2022 at 10:14:20AM -0700, Linus Torvalds wrote:
> On Wed, Oct 19, 2022 at 9:57 AM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > This is an ABI change.  It is also hugely detrimental to generated
> > code quality on architectures that make the saner choice (that is, have
> > most instructions zero-extend byte quantities).
> 
> Yeah, I agree. We should just accept the standard wording, and be
> aware that 'char' has indeterminate signedness.

And plain "char" is a separate type from "signed char" and "unsigned
char" both.

> But:
> 
> > Instead, don't actively disable the compiler warnings that catch such
> > cases?  So start with removing footguns like
> >
> >   # disable pointer signed / unsigned warnings in gcc 4.0
> >   KBUILD_CFLAGS += -Wno-pointer-sign
> 
> Nope, that won't fly.
> 
> The pointer-sign thing doesn't actually help (ie it won't find places
> where you actually compare a char), and it causes untold damage in
> doing completely insane things.

When I did this more than a decade ago there indeed was a LOT of noise,
mostly caused by dubious code.  I do agree many cases detected are not
very important, but it also revealed cases where a filesystem's disk
format changed (atarifs or amigafs or such iirc) -- many cases it is
annoying to be reminded of sloppy code, but in some cases it detects
crucial problems.

> Seriously, -Wpointer-sign is not just useless, it's actively _evil_.

Then suggest something better?  Or suggest improvements to the existing
warning?

This warning is part of -Wall, most people must not have problems with
it (or people are so apathetic about this that they have not complained
about it).

It is easy to improve your code when the compiler detects problems like
this.  Of course after such a long time of lax code sanity enforcement
you get all warnings at once :-/

> The fact that you suggest that clearly means that you've never used
> it.

Ah, ad hominems.  Great.

Segher