mainline build failure for arm64 allmodconfig with clang

mainline build failure for arm64 allmodconfig with clang

[Sudip Mukherjee (Codethink)]

Hi All,

The latest mainline kernel branch fails to build for arm64 allmodconfig
with clang. The errors are:

sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
                 SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
                 ^~~~~~~~~~~~~
./include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
        dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                 ~~~     ^~~~~~~~~~~
./include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                _p_func(dev, fmt, ##__VA_ARGS__);                       \
                             ~~~    ^~~~~~~~~~~
./include/uapi/sound/sof/abi.h:30:23: note: expanded from macro 'SOF_ABI_MAJOR'
#define SOF_ABI_MAJOR 3
                      ^
sound/soc/sof/ipc3-topology.c:2343:19: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
                 SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
                                ^~~~~~~~~~~~~
./include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
        dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                 ~~~     ^~~~~~~~~~~
./include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                _p_func(dev, fmt, ##__VA_ARGS__);                       \
                             ~~~    ^~~~~~~~~~~
./include/uapi/sound/sof/abi.h:31:23: note: expanded from macro 'SOF_ABI_MINOR'
#define SOF_ABI_MINOR 23
                      ^~
sound/soc/sof/ipc3-topology.c:2343:34: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
                 SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
                                               ^~~~~~~~~~~~~
./include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
        dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                 ~~~     ^~~~~~~~~~~
./include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                _p_func(dev, fmt, ##__VA_ARGS__);                       \
                             ~~~    ^~~~~~~~~~~
./include/uapi/sound/sof/abi.h:32:23: note: expanded from macro 'SOF_ABI_PATCH'
#define SOF_ABI_PATCH 0
                      ^
3 errors generated.

drivers/ntb/hw/idt/ntb_hw_idt.c:2409:28: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
                                "\t%hhu-%hhu.\t", idx, idx + cnt - 1);
                                        ~~~~           ^~~~~~~~~~~~~
                                        %d
drivers/ntb/hw/idt/ntb_hw_idt.c:2438:29: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
                                        "\t%hhu-%hhu.\t", idx, idx + cnt - 1);
                                                ~~~~           ^~~~~~~~~~~~~
                                                %d
drivers/ntb/hw/idt/ntb_hw_idt.c:2484:15: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
                        idx, data, src, ndev->peers[src].port);
                                   ^~~
3 errors generated.

For both, git bisect points to 0af5cb349a2c ("Merge tag 'kbuild-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")

The last good build for arm64 clang was with e394ff83bbca ("Merge tag 'nfsd-6.0' of git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux")

I will be happy to test any patch or provide any extra log if needed.


--
Regards
Sudip

Re: mainline build failure for arm64 allmodconfig with clang

[Nathan Chancellor]

Hi Sudip,

On Thu, Aug 11, 2022 at 08:36:24AM +0100, Sudip Mukherjee (Codethink) wrote:
> Hi All,
> 
> The latest mainline kernel branch fails to build for arm64 allmodconfig
> with clang. The errors are:
> 
> sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
>                  SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>                  ^~~~~~~~~~~~~
> ./include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
>         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
>                                                                  ~~~     ^~~~~~~~~~~
> ./include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
>                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>                              ~~~    ^~~~~~~~~~~
> ./include/uapi/sound/sof/abi.h:30:23: note: expanded from macro 'SOF_ABI_MAJOR'
> #define SOF_ABI_MAJOR 3
>                       ^
> sound/soc/sof/ipc3-topology.c:2343:19: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
>                  SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>                                 ^~~~~~~~~~~~~
> ./include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
>         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
>                                                                  ~~~     ^~~~~~~~~~~
> ./include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
>                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>                              ~~~    ^~~~~~~~~~~
> ./include/uapi/sound/sof/abi.h:31:23: note: expanded from macro 'SOF_ABI_MINOR'
> #define SOF_ABI_MINOR 23
>                       ^~
> sound/soc/sof/ipc3-topology.c:2343:34: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
>                  SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>                                                ^~~~~~~~~~~~~
> ./include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
>         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
>                                                                  ~~~     ^~~~~~~~~~~
> ./include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
>                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>                              ~~~    ^~~~~~~~~~~
> ./include/uapi/sound/sof/abi.h:32:23: note: expanded from macro 'SOF_ABI_PATCH'
> #define SOF_ABI_PATCH 0
>                       ^
> 3 errors generated.
> 
> drivers/ntb/hw/idt/ntb_hw_idt.c:2409:28: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
>                                 "\t%hhu-%hhu.\t", idx, idx + cnt - 1);
>                                         ~~~~           ^~~~~~~~~~~~~
>                                         %d
> drivers/ntb/hw/idt/ntb_hw_idt.c:2438:29: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
>                                         "\t%hhu-%hhu.\t", idx, idx + cnt - 1);
>                                                 ~~~~           ^~~~~~~~~~~~~
>                                                 %d
> drivers/ntb/hw/idt/ntb_hw_idt.c:2484:15: error: format specifies type 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
>                         idx, data, src, ndev->peers[src].port);
>                                    ^~~
> 3 errors generated.
> 
> For both, git bisect points to 0af5cb349a2c ("Merge tag 'kbuild-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")

Right, these are exposed by commit 258fafcd0683 ("Makefile.extrawarn:
re-enable -Wformat for clang"). They both have fixes in -next so I am
hoping they will be in Linus's tree soon:

b7bf23c0865f ("ASoC: SOF: ipc3-topology: Fix clang -Wformat warning")
a44252d5c3bb ("ntb: idt: fix clang -Wformat warnings")

Cheers,
Nathan

Re: mainline build failure for arm64 allmodconfig with clang

[Linus Torvalds]

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.

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

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

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.

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

Re: mainline build failure for arm64 allmodconfig with clang

[Nick Desaulniers]

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

Re: mainline build failure for arm64 allmodconfig with clang

[Linus Torvalds]

On Thu, Aug 11, 2022 at 11:39 AM Nick Desaulniers
<ndesaulniers@google.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

Re: mainline build failure for arm64 allmodconfig with clang

[Nick Desaulniers]

On Thu, Aug 11, 2022 at 12:35 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Aug 11, 2022 at 11:39 AM Nick Desaulniers
> <ndesaulniers@google.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?

Yeah, I guess adding a truncate to the caller is kind of unnecessary
if you're still going to use %hhd anyways.  What are your thoughts on
this bug I've filed?
https://github.com/llvm/llvm-project/issues/57102

-- 
Thanks,
~Nick Desaulniers

Re: mainline build failure for arm64 allmodconfig with clang

[Linus Torvalds]

On Thu, Aug 11, 2022 at 3:04 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Yeah, I guess adding a truncate to the caller is kind of unnecessary
> if you're still going to use %hhd anyways.  What are your thoughts on
> this bug I've filed?
> https://github.com/llvm/llvm-project/issues/57102

I really wish that bug report was about how *wrong* it is to warn
about that thing.

Sure, a cast adds pointless code to the caller, and is overhead, but
that isn't really the argument here.

The argument really is that "if you force the value to always be a
char, then %hhd is pointless"

Basically, %hhu and %hhd simply do not make sense if you always
require the value to be cast to the right range, because for those
cases the much simpler and more straightforward %d and %u already
work.

So a warning that says "you didn't cast it to the rigth range" is
truly fundamentally broken.

So it's not about "a cast generates worse code". It's really about
"that warning is WRONG".

          Linus

RE: mainline build failure for arm64 allmodconfig with clang

[David Laight]

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

Which is, of course, invalid C :-)
The domain of the isxxxx() functions is 'the values of char
cast to unsigned int' and EOF.
So typically [-1 .. 255].
Passing a signed char variable to isprint() is likely to
generate undesired behaviour (like a code dump).

It has to be said that I did a full trawl through the NetBSD
userspace code base to fix all the isxxx() calls and failed
to find a single one that could actually pass EOF.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[PATCH] Makefile.extrawarn: re-enable -Wformat for clang; take 2

[Nick Desaulniers]

-Wformat was recently re-enabled for builds with clang, then quickly
re-disabled, due to concerns stemming from the frequency of default
argument promotion related warning instances.

commit 258fafcd0683 ("Makefile.extrawarn: re-enable -Wformat for clang")
commit 21f9c8a13bb2 ("Revert "Makefile.extrawarn: re-enable -Wformat for clang"")

ISO WG14 has ratified N2562 to address default argument promotion
explicitly for printf, as part of the upcoming ISO C2X standard.

The behavior of clang was changed in clang-16 to not warn for the cited
cases in all language modes.

Add a version check, so that users of clang-16 now get the full effect
of -Wformat. For older clang versions, re-enable flags under the
-Wformat group that way users still get some useful checks related to
format strings, without noisy default argument promotion warnings. I
intentionally omitted -Wformat-y2k and -Wformat-security from being
re-enabled, which are also part of -Wformat in clang-16.

Link: https://github.com/ClangBuiltLinux/linux/issues/378
Link: https://github.com/llvm/llvm-project/issues/57102
Link: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
Suggested-by: Justin Stitt <jstitt007@gmail.com>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Youngmin Nam <youngmin.nam@samsung.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Linus, I figured I'd send this to you to see whether you'd prefer to
apply it, or let it "ride the trains" up through the kbuild tree? I do
have another series I'm working on to improve the compiler version
checks
<https://lore.kernel.org/llvm/20220831184408.2778264-4-ndesaulniers@google.com/>
where I can/will improve the checks used here, but I'm also interested in
having something that might backport cleanly to stable.

 scripts/Makefile.extrawarn | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 0621c39a3955..6ae482158bc4 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -47,7 +47,19 @@ else
 
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += -Wno-initializer-overrides
+# Clang before clang-16 would warn on default argument promotions.
+ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
+# Disable -Wformat
 KBUILD_CFLAGS += -Wno-format
+# Then re-enable flags that were part of the -Wformat group that aren't
+# problematic.
+KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
+KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
+# Requires clang-12+.
+ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
+KBUILD_CFLAGS += -Wformat-insufficient-args
+endif
+endif
 KBUILD_CFLAGS += -Wno-sign-compare
 KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
 KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare

base-commit: 2880e1a175b9f31798f9d9482ee49187f61b5539
-- 
2.37.2.789.g6183377224-goog

Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang; take 2

[Nathan Chancellor]

On Thu, Sep 01, 2022 at 10:59:13AM -0700, Nick Desaulniers wrote:
> -Wformat was recently re-enabled for builds with clang, then quickly
> re-disabled, due to concerns stemming from the frequency of default
> argument promotion related warning instances.
> 
> commit 258fafcd0683 ("Makefile.extrawarn: re-enable -Wformat for clang")
> commit 21f9c8a13bb2 ("Revert "Makefile.extrawarn: re-enable -Wformat for clang"")
> 
> ISO WG14 has ratified N2562 to address default argument promotion
> explicitly for printf, as part of the upcoming ISO C2X standard.
> 
> The behavior of clang was changed in clang-16 to not warn for the cited
> cases in all language modes.
> 
> Add a version check, so that users of clang-16 now get the full effect
> of -Wformat. For older clang versions, re-enable flags under the
> -Wformat group that way users still get some useful checks related to
> format strings, without noisy default argument promotion warnings. I
> intentionally omitted -Wformat-y2k and -Wformat-security from being
> re-enabled, which are also part of -Wformat in clang-16.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Link: https://github.com/llvm/llvm-project/issues/57102
> Link: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
> Suggested-by: Justin Stitt <jstitt007@gmail.com>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Youngmin Nam <youngmin.nam@samsung.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> Linus, I figured I'd send this to you to see whether you'd prefer to
> apply it, or let it "ride the trains" up through the kbuild tree? I do
> have another series I'm working on to improve the compiler version
> checks
> <https://lore.kernel.org/llvm/20220831184408.2778264-4-ndesaulniers@google.com/>
> where I can/will improve the checks used here, but I'm also interested in
> having something that might backport cleanly to stable.
> 
>  scripts/Makefile.extrawarn | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 0621c39a3955..6ae482158bc4 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -47,7 +47,19 @@ else
>  
>  ifdef CONFIG_CC_IS_CLANG
>  KBUILD_CFLAGS += -Wno-initializer-overrides
> +# Clang before clang-16 would warn on default argument promotions.
> +ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -lt 160000 ] && echo y),y)
> +# Disable -Wformat
>  KBUILD_CFLAGS += -Wno-format
> +# Then re-enable flags that were part of the -Wformat group that aren't
> +# problematic.
> +KBUILD_CFLAGS += -Wformat-extra-args -Wformat-invalid-specifier
> +KBUILD_CFLAGS += -Wformat-zero-length -Wnonnull
> +# Requires clang-12+.
> +ifeq ($(shell [ $(CONFIG_CLANG_VERSION) -ge 120000 ] && echo y),y)
> +KBUILD_CFLAGS += -Wformat-insufficient-args
> +endif
> +endif
>  KBUILD_CFLAGS += -Wno-sign-compare
>  KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
>  KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
> 
> base-commit: 2880e1a175b9f31798f9d9482ee49187f61b5539
> -- 
> 2.37.2.789.g6183377224-goog
> 

Re: [PATCH] Makefile.extrawarn: re-enable -Wformat for clang; take 2

[Masahiro Yamada]

On Fri, Sep 2, 2022 at 2:59 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> -Wformat was recently re-enabled for builds with clang, then quickly
> re-disabled, due to concerns stemming from the frequency of default
> argument promotion related warning instances.
>
> commit 258fafcd0683 ("Makefile.extrawarn: re-enable -Wformat for clang")
> commit 21f9c8a13bb2 ("Revert "Makefile.extrawarn: re-enable -Wformat for clang"")
>
> ISO WG14 has ratified N2562 to address default argument promotion
> explicitly for printf, as part of the upcoming ISO C2X standard.
>
> The behavior of clang was changed in clang-16 to not warn for the cited
> cases in all language modes.
>
> Add a version check, so that users of clang-16 now get the full effect
> of -Wformat. For older clang versions, re-enable flags under the
> -Wformat group that way users still get some useful checks related to
> format strings, without noisy default argument promotion warnings. I
> intentionally omitted -Wformat-y2k and -Wformat-security from being
> re-enabled, which are also part of -Wformat in clang-16.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Link: https://github.com/llvm/llvm-project/issues/57102
> Link: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
> Suggested-by: Justin Stitt <jstitt007@gmail.com>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Youngmin Nam <youngmin.nam@samsung.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Linus, I figured I'd send this to you to see whether you'd prefer to
> apply it, or let it "ride the trains" up through the kbuild tree? I do
> have another series I'm working on to improve the compiler version
> checks
> <https://lore.kernel.org/llvm/20220831184408.2778264-4-ndesaulniers@google.com/>
> where I can/will improve the checks used here, but I'm also interested in
> having something that might backport cleanly to stable.

Linus was addressed, so I just reviewed it.

Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>


-- 
Best Regards
Masahiro Yamada