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

Commit Message

Nick Desaulniers Sept. 1, 2022, 5:59 p.m. UTC

-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 <[e-mail]>
Suggested-by: Nathan Chancellor <[e-mail]>
Suggested-by: Youngmin Nam <[e-mail]>
Signed-off-by: Nick Desaulniers <[e-mail]>
---
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/[e-mail]/>
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(+)


base-commit: 2880e1a175b9f31798f9d9482ee49187f61b5539

Comments

Nathan Chancellor Sept. 1, 2022, 6:06 p.m. UTC | #1

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 <[e-mail]>
> Suggested-by: Nathan Chancellor <[e-mail]>
> Suggested-by: Youngmin Nam <[e-mail]>
> Signed-off-by: Nick Desaulniers <[e-mail]>

Reviewed-by: Nathan Chancellor <[e-mail]>

> ---
> 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/[e-mail]/>
> 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
>

Masahiro Yamada Sept. 3, 2022, 6:22 p.m. UTC | #2

On Fri, Sep 2, 2022 at 2:59 AM Nick Desaulniers <[e-mail]> 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 <[e-mail]>
> Suggested-by: Nathan Chancellor <[e-mail]>
> Suggested-by: Youngmin Nam <[e-mail]>
> Signed-off-by: Nick Desaulniers <[e-mail]>
> ---
> 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/[e-mail]/>
> 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 <[e-mail]>

Patch

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