Re: mainline build failure for arm64 allmodconfig with clang

[Nick Desaulniers <ndesaulniers@google.com>]

On Thu, Aug 11, 2022 at 8:39 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Aug 11, 2022 at 8:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Right, these are exposed by commit 258fafcd0683 ("Makefile.extrawarn:
> > re-enable -Wformat for clang").
>
> Christ. Why is clang's format warning SO COMPLETELY BROKEN?
>
> The warning is *WRONG*, for chrissake. Printing an 'int' with '%hhu'
> is perfectly fine, and has well-defined semantics, and is what you
> *want* to do in some cases.

Generally, printing an int with %hhu may truncate depending on the
value of the int.

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

> I'm going to turn it off again, because honestly, this is a clang bug.
> I don't care one whit if there are pending "fixes" for this clang bug,
> until those fixes are in *clang*, not in the correct kernel code.
>
> For chrissake, the value it is trying to print out as a char is '3'.

If your referring to SOF_ABI_MAJOR from

commit b7bf23c0865f ("ASoC: SOF: ipc3-topology: Fix clang -Wformat warning")

in -next, 3 is an int literal.  No truncation occurs, sure, but just
use the correct format flag!

Otherwise please also considering reverting
commit cbacb5ab0aa0 ("docs: printk-formats: Stop encouraging use of
unnecessary %h[xudi] and %hh[xudi]")
since for the past 3 years, we've been recommending that kernel
developers not use %h or %hh.  You allude to this in your "Admittedly,
" note in
commit 21f9c8a13bb2 ("Revert "Makefile.extrawarn: re-enable -Wformat
for clang"")
. Otherwise, please reinstate this patch.

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

Because having explicit documented practices then reverting things
when those are followed is quite obnoxious.

> But even if it wasn't, and even if you wanted to print out "0xf365" as
> a "char" value, then that is how C varargs *work*. It's an "int".

This is a different case than using a literal value in which no
truncation would occur.  (Your points about 3 and 'a' (no truncation)
are distinct from 0xf365 (truncation)).

It would be anomolous to the compiler whether the truncation in such a
case was intentional vs accidental.

printf("%hhx\n", 0xf365); // -Wformat: warning: format specifies type
'unsigned char' but the argument has type 'int'

should be

printf("%hhx\n", (unsigned char)0xf365); // intentional truncation, no warning

A cast in that case helps inform the compiler that "I know what I'm
doing," and a comment helps code reviewers & maintainers.

> In fact, even a *character* is an "int". This program:
>
>         #include <stdio.h>
>
>         int main(int argc, char **argv)
>         {
>                 printf("%hhu\n", 'a');
>         }
>
> generates a warning with "clang -Wformat", and dammit, if you are a
> clang developer and you see no problem with that warning, then I don't
> know what to say.

Yeah, that is noisy.  I think if we had an argument that is a literal,
we should be able to tell then and there whether that value would
result in truncation (and avoiding diagnosing if no truncation occurs,
or split that into -Wformat-me-harder so that we could set
-Wno-format-me-harder).

printf("%hhu\n", 256); // should this produce a warning? Which
compilers do so? ;)

Though, isn't %c the correct format flag for characters?

>
> Nathan, please make clang people see some sense.
>
> Because no, I'm not in the least interested in getting kernel "fixes"
> for this issue. -Wformat for clang goes away until people have gotten
> their heads extracted from their derrières.
>
> This is ridiculous.
>
>               Linus



--
Thanks,
~Nick Desaulniers