2021-09-17 06:13:02

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2 0/2] Harden clang against unknown flag options

Hi all,

This series cleans up an issue that was noticed by the kernel test robot
where flags that clang does not implement support for are
unconditionally added to the command line, which causes all subsequent
calls to cc-{disable-warning,option} to fail, meaning developers are
flooded with unnecessary and pointless warnings.

The first patch handles the problematic flags with cc-option for clang
and the second patch ensures we catch new additions of unknown flags so
that they can be handled properly.

I intend for this to be merged via the kbuild tree but it can go via
-tip if there is any objection to that.

Cheers,
Nathan

v1 -> v2: https://lore.kernel.org/r/[email protected]/

* Patch 1: Change prefix to "x86/build" (Borislav).

* Patch 1: Add link to v1 thread for more context (Borislav).

* Patch 1: Add Borislav's ack.

* Patch 2: Expand comment in source to make it clear that clang only
warns on certain unimplemented optimization flags.

* Series: Add Nick's review.

Nathan Chancellor (2):
x86/build: Do not add -falign flags unconditionally for clang
kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS

arch/x86/Makefile_32.cpu | 12 +++++++++---
scripts/Makefile.clang | 5 +++++
2 files changed, 14 insertions(+), 3 deletions(-)


base-commit: a9086b878b7fd65894eb8cb1fa395dd469970566
--
2.33.0


2021-09-17 06:14:23

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2 1/2] x86/build: Do not add -falign flags unconditionally for clang

clang does not support -falign-jumps and only recently gained support
for -falign-loops. When one of the configuration options that adds these
flags is enabled, clang warns and all cc-{disable-warning,option} that
follow fail because -Werror gets added to test for the presence of this
warning:

clang-14: warning: optimization flag '-falign-jumps=0' is not supported
[-Wignored-optimization-argument]

To resolve this, add a couple of cc-option calls when building with
clang; gcc has supported these options since 3.2 so there is no point in
testing for their support. -falign-functions was implemented in clang-7,
-falign-loops was implemented in clang-14, and -falign-jumps has not
been implemented yet.

Link: https://lore.kernel.org/r/[email protected]/
Link: https://lore.kernel.org/r/[email protected]/
Reported-by: kernel test robot <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Acked-by: Borislav Petkov <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
arch/x86/Makefile_32.cpu | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index e7355f8b51c2..94834c4b5e5e 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -4,6 +4,12 @@

tune = $(call cc-option,-mtune=$(1),$(2))

+ifdef CONFIG_CC_IS_CLANG
+align := -falign-functions=0 $(call cc-option,-falign-jumps=0) $(call cc-option,-falign-loops=0)
+else
+align := -falign-functions=0 -falign-jumps=0 -falign-loops=0
+endif
+
cflags-$(CONFIG_M486SX) += -march=i486
cflags-$(CONFIG_M486) += -march=i486
cflags-$(CONFIG_M586) += -march=i586
@@ -19,11 +25,11 @@ cflags-$(CONFIG_MK6) += -march=k6
# They make zero difference whatsosever to performance at this time.
cflags-$(CONFIG_MK7) += -march=athlon
cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8,-march=athlon)
-cflags-$(CONFIG_MCRUSOE) += -march=i686 -falign-functions=0 -falign-jumps=0 -falign-loops=0
-cflags-$(CONFIG_MEFFICEON) += -march=i686 $(call tune,pentium3) -falign-functions=0 -falign-jumps=0 -falign-loops=0
+cflags-$(CONFIG_MCRUSOE) += -march=i686 $(align)
+cflags-$(CONFIG_MEFFICEON) += -march=i686 $(call tune,pentium3) $(align)
cflags-$(CONFIG_MWINCHIPC6) += $(call cc-option,-march=winchip-c6,-march=i586)
cflags-$(CONFIG_MWINCHIP3D) += $(call cc-option,-march=winchip2,-march=i586)
-cflags-$(CONFIG_MCYRIXIII) += $(call cc-option,-march=c3,-march=i486) -falign-functions=0 -falign-jumps=0 -falign-loops=0
+cflags-$(CONFIG_MCYRIXIII) += $(call cc-option,-march=c3,-march=i486) $(align)
cflags-$(CONFIG_MVIAC3_2) += $(call cc-option,-march=c3-2,-march=i686)
cflags-$(CONFIG_MVIAC7) += -march=i686
cflags-$(CONFIG_MCORE2) += -march=i686 $(call tune,core2)

base-commit: a9086b878b7fd65894eb8cb1fa395dd469970566
--
2.33.0

2021-09-17 07:46:09

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS

Similar to commit 589834b3a009 ("kbuild: Add
-Werror=unknown-warning-option to CLANG_FLAGS").

Clang ignores certain GCC flags that it has not implemented, only
emitting a warning:

$ echo | clang -fsyntax-only -falign-jumps -x c -
clang-14: warning: optimization flag '-falign-jumps' is not supported
[-Wignored-optimization-argument]

When one of these flags gets added to KBUILD_CFLAGS unconditionally, all
subsequent cc-{disable-warning,option} calls fail because -Werror was
added to these invocations to turn the above warning and the equivalent
-W flag warning into errors.

To catch the presence of these flags earlier, turn
-Wignored-optimization-argument into an error so that the flags can
either be implemented or ignored via cc-option and there are no more
weird errors.

Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
scripts/Makefile.clang | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
index 4cce8fd0779c..51fc23e2e9e5 100644
--- a/scripts/Makefile.clang
+++ b/scripts/Makefile.clang
@@ -29,7 +29,12 @@ CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
else
CLANG_FLAGS += -fintegrated-as
endif
+# By default, clang only warns when it encounters an unknown warning flag or
+# certain optimization flags it knows it has not implemented.
+# Make it behave more like gcc by erroring when these flags are encountered
+# so they can be implemented or wrapped in cc-option.
CLANG_FLAGS += -Werror=unknown-warning-option
+CLANG_FLAGS += -Werror=ignored-optimization-argument
KBUILD_CFLAGS += $(CLANG_FLAGS)
KBUILD_AFLAGS += $(CLANG_FLAGS)
export CLANG_FLAGS
--
2.33.0

2021-09-19 12:05:51

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] x86/build: Do not add -falign flags unconditionally for clang

On Fri, Sep 17, 2021 at 3:42 AM Nathan Chancellor <[email protected]> wrote:
>
> clang does not support -falign-jumps and only recently gained support
> for -falign-loops. When one of the configuration options that adds these
> flags is enabled, clang warns and all cc-{disable-warning,option} that
> follow fail because -Werror gets added to test for the presence of this
> warning:
>
> clang-14: warning: optimization flag '-falign-jumps=0' is not supported
> [-Wignored-optimization-argument]
>
> To resolve this, add a couple of cc-option calls when building with
> clang; gcc has supported these options since 3.2 so there is no point in
> testing for their support. -falign-functions was implemented in clang-7,
> -falign-loops was implemented in clang-14, and -falign-jumps has not
> been implemented yet.
>
> Link: https://lore.kernel.org/r/[email protected]/
> Link: https://lore.kernel.org/r/[email protected]/
> Reported-by: kernel test robot <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Acked-by: Borislav Petkov <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> arch/x86/Makefile_32.cpu | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
> index e7355f8b51c2..94834c4b5e5e 100644
> --- a/arch/x86/Makefile_32.cpu
> +++ b/arch/x86/Makefile_32.cpu
> @@ -4,6 +4,12 @@
>
> tune = $(call cc-option,-mtune=$(1),$(2))
>
> +ifdef CONFIG_CC_IS_CLANG
> +align := -falign-functions=0 $(call cc-option,-falign-jumps=0) $(call cc-option,-falign-loops=0)
> +else
> +align := -falign-functions=0 -falign-jumps=0 -falign-loops=0
> +endif
> +
> cflags-$(CONFIG_M486SX) += -march=i486
> cflags-$(CONFIG_M486) += -march=i486
> cflags-$(CONFIG_M586) += -march=i586
> @@ -19,11 +25,11 @@ cflags-$(CONFIG_MK6) += -march=k6
> # They make zero difference whatsosever to performance at this time.
> cflags-$(CONFIG_MK7) += -march=athlon
> cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8,-march=athlon)
> -cflags-$(CONFIG_MCRUSOE) += -march=i686 -falign-functions=0 -falign-jumps=0 -falign-loops=0
> -cflags-$(CONFIG_MEFFICEON) += -march=i686 $(call tune,pentium3) -falign-functions=0 -falign-jumps=0 -falign-loops=0
> +cflags-$(CONFIG_MCRUSOE) += -march=i686 $(align)
> +cflags-$(CONFIG_MEFFICEON) += -march=i686 $(call tune,pentium3) $(align)
> cflags-$(CONFIG_MWINCHIPC6) += $(call cc-option,-march=winchip-c6,-march=i586)
> cflags-$(CONFIG_MWINCHIP3D) += $(call cc-option,-march=winchip2,-march=i586)
> -cflags-$(CONFIG_MCYRIXIII) += $(call cc-option,-march=c3,-march=i486) -falign-functions=0 -falign-jumps=0 -falign-loops=0
> +cflags-$(CONFIG_MCYRIXIII) += $(call cc-option,-march=c3,-march=i486) $(align)
> cflags-$(CONFIG_MVIAC3_2) += $(call cc-option,-march=c3-2,-march=i686)
> cflags-$(CONFIG_MVIAC7) += -march=i686
> cflags-$(CONFIG_MCORE2) += -march=i686 $(call tune,core2)
>
> base-commit: a9086b878b7fd65894eb8cb1fa395dd469970566
> --
> 2.33.0
>

Applied to linux-kbuild. Thanks.


--
Best Regards
Masahiro Yamada

2021-09-19 12:05:59

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS

On Fri, Sep 17, 2021 at 3:42 AM Nathan Chancellor <[email protected]> wrote:
>
> Similar to commit 589834b3a009 ("kbuild: Add
> -Werror=unknown-warning-option to CLANG_FLAGS").
>
> Clang ignores certain GCC flags that it has not implemented, only
> emitting a warning:
>
> $ echo | clang -fsyntax-only -falign-jumps -x c -
> clang-14: warning: optimization flag '-falign-jumps' is not supported
> [-Wignored-optimization-argument]
>
> When one of these flags gets added to KBUILD_CFLAGS unconditionally, all
> subsequent cc-{disable-warning,option} calls fail because -Werror was
> added to these invocations to turn the above warning and the equivalent
> -W flag warning into errors.
>
> To catch the presence of these flags earlier, turn
> -Wignored-optimization-argument into an error so that the flags can
> either be implemented or ignored via cc-option and there are no more
> weird errors.
>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---

Applied to linux-kbuild. Thanks.



> scripts/Makefile.clang | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 4cce8fd0779c..51fc23e2e9e5 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -29,7 +29,12 @@ CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> else
> CLANG_FLAGS += -fintegrated-as
> endif
> +# By default, clang only warns when it encounters an unknown warning flag or
> +# certain optimization flags it knows it has not implemented.
> +# Make it behave more like gcc by erroring when these flags are encountered
> +# so they can be implemented or wrapped in cc-option.
> CLANG_FLAGS += -Werror=unknown-warning-option
> +CLANG_FLAGS += -Werror=ignored-optimization-argument
> KBUILD_CFLAGS += $(CLANG_FLAGS)
> KBUILD_AFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> --
> 2.33.0
>


--
Best Regards
Masahiro Yamada