Message-ID: <CAHk-=wgJA=e-CLcvU5LRKu0bMLeAewXtOM6as1hFVeQAVkMPbg@mail.gmail.com>
Date:   Thu, 11 Aug 2022 12:35:26 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Joe Perches <joe@...ches.com>,
        Nathan Chancellor <nathan@...nel.org>,
        "Sudip Mukherjee (Codethink)" <sudipm.mukherjee@...il.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        clang-built-linux <llvm@...ts.linux.dev>,
        Justin Stitt <justinstitt@...gle.com>
Subject: Re: mainline build failure for arm64 allmodconfig with clang

On Thu, Aug 11, 2022 at 11:39 AM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
>
> Generally, printing an int with %hhu may truncate depending on the
> value of the int.

Yes.

HOWEVER.

That truncation is *LITERALLY THE MAIN REASON TO EVER USE %hhu IN THE
FIRST PLACE*.

See the issue?

Warning about "this may truncate bits" when the main reason to use
that format string in the first place is said bit truncation is kind
of stupid, isn't it?

I suspect most people have no idea what '%hhu' means in the first
place, and probably have to look it up. It's a pretty specialized
thing, with a pretty specialized reason.

The most common reason that I've ever seen for using it has been "oh,
I have a 'char', and I don't know or care about the sign of it, I want
to print it out consistently, and %hhu does that for me".

And often that char isn't actually a 'char', it is actually an 'int',
either because you have situations like 'getch()', or you have simply
just the usual C expression rules, ie you have something like

        isprint(c) ? c : '.'

where even if 'c' is of type 'char', the end result is 'int'.

And guess what? Truncating out those sign bits - if char is signed,
which it may or may not be - is then what the whole and only point of
it is.

Really.

So if a compiler warns about it, the compiler is BROKEN.

And if a compiler writer says "well, then you should have added a cast
to 'unsigned char'", then the compiler writer is clueless and WRONG,
because if you add the cast, then the '%hhu' becomes pointless again,
and you would have been better off using the simpler and more obvious
%u (or the even more obvious %d).

See what I'm saying when I'm emphasizing that THE MAIN REASON TO USE
'%hhu' IS BECAUSE YOU KNOW AND WANT THAT TRUNCATION.

> Perhaps there's something different we can be doing for literals though.

No.

Look, literals just make that warning look obviously stupid. With a
literal, even a less-than-gifted rodent goes "that warning is stupid".
Adding a cast to 'unsigned char' looks stupid to begin with, but it is
then *extra* stupid when you know the function is a varargs functions
and it will just be cast right back to 'int' anyway.

It happens to be what at least one kernel use was, and it happens to
be what my example was, just to *really* point out how stupid that
warning is.

But the deeper truth is that the warning is wrong even for the case
where there are upper bits, because that's when '%hhu' really matters.

So that warning only makes '%hhu' pointless.

The only thing that warning shows is that some clang person understodd
the *syntax* of %hhu, but didn't understand the *meaning* of it, and
the use of it.

> I don't care which you pick, but let's be consistent?

The thing is, I don't care at all if some kernel developer decides to
use '%hhu'. It's odd, but it's not wrong. It's perhaps worth
discouraging just because it's so odd and unusual, but at the same
time I really don't care very deeply.

But what I *do* care about is when a compiler is so broken that it
gives completely mindless warnings because it doesn't understand what
it going on.

At that point I go "let's turn off this warning, because the people
who did that warning are clearly not doing the right thing, and that
warning causes pointless problems".

And until clang fixes their idiotic warning policy, that -Wformat
stays turned off.

Now, if you can show that the clang people understand why that warning
is completely bogus and broken, adn they've fixed it in their
development tree, at that point I'm willing to turn the warning on
again and work around it in the kernel.

But as long as clang just gets it fundamentally wrong, and as long as
clang developers continue to think that their broken warning is
"correct", then I refuse to turn that garbage on.

See where I'm coming from? The warning is fundamentally broken.

I'm willing to work around compiler bugs in the kernel. But if the
compiler people in question don't even think they are bugs, I'm not
working around them, I'm turning them off.

This is not that different from the whole "type-based alias analysis"
thing. It's fundamentally broken garbage, and it gets turned off.

See the difference?

One is "oh, compiler bug, we'll work around it".

The other is "the compiler is being actively wrong unless you pass the
right flag to the compiler".

Right now it appears that clang is being actively wrong, and that
passing '-Wformat' to clang is simply the wrong thing to do. Because
that warning has clearly been actively added. It's not some oversight,
it's literally extra code just to mess things up.

But the moment I hear that clang removed that warning in their
development tree, it turns from active malice to just a mostly
harmless bug.

               Linus

Powered by blists - more mailing lists