[PATCH 0/2] Harden clang against unknown flag options

[PATCH 0/2] Harden clang against unknown flag options

[Nathan Chancellor]

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.

I hope the patches in and of themselves are reasonable and
non-controversial. This is based on Masahiro's kbuild tree as there was
a fairly large refactor around where clang's flags were added so I
figured it would be best to go there with an x86 ack since the first
patch does not depend on anything in -tip.

Cheers,
Nathan

Nathan Chancellor (2):
  x86: 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   |  4 ++++
 2 files changed, 13 insertions(+), 3 deletions(-)


base-commit: fb3fdea450305d932d933d7e75eead0477249d8e
-- 
2.33.0

[PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Nathan Chancellor]

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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 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 cd3056759880..e8c65f990afd 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -10,6 +10,12 @@ else
 tune		= $(call cc-option,-mcpu=$(1),$(2))
 endif
 
+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
@@ -25,11 +31,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)
-- 
2.33.0

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Fangrui Song]

On 2021-08-23, Nathan Chancellor 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:

[I implemented clang -falign-loops :) It doesn't affect LTO, though.
LTO ld.lld may use -Wl,-mllvm,-align-loops=32 for now.  ]

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

grub made a similar mistake:) It thought the availability of -falign-X
implies the availability of other -falign-*
https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00076.html

>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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/
>Reported-by: kernel test robot <lkp@intel.com>
>Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>---
> 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 cd3056759880..e8c65f990afd 100644
>--- a/arch/x86/Makefile_32.cpu
>+++ b/arch/x86/Makefile_32.cpu
>@@ -10,6 +10,12 @@ else
> tune		= $(call cc-option,-mcpu=$(1),$(2))
> endif
>
>+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
>@@ -25,11 +31,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)
>-- 
>2.33.0

https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html says
"If n is not specified or is zero, use a machine-dependent default."

Unless some other files specify -falign-loops=N and expect 0 to reset to
the machine default, -falign-jumps=0 -falign-loops=0 -falign-functions=0 should just be dropped.

BTW: I believe GCC 8 (likely when fixing another issue with a large refactor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84100) introduced a bug
that -falign-X=0 was essentially -falign-X=1.
GCC 11.0 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247) fixed the bug.

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Nathan Chancellor]

On Mon, Aug 23, 2021 at 07:56:47PM -0700, Fangrui Song wrote:
> On 2021-08-23, Nathan Chancellor 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:
> 
> [I implemented clang -falign-loops :) It doesn't affect LTO, though.
> LTO ld.lld may use -Wl,-mllvm,-align-loops=32 for now.  ]
> 
> > clang-14: warning: optimization flag '-falign-jumps=0' is not supported
> > [-Wignored-optimization-argument]
> 
> grub made a similar mistake:) It thought the availability of -falign-X
> implies the availability of other -falign-*
> https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00076.html
> 
> > 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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > 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 cd3056759880..e8c65f990afd 100644
> > --- a/arch/x86/Makefile_32.cpu
> > +++ b/arch/x86/Makefile_32.cpu
> > @@ -10,6 +10,12 @@ else
> > tune		= $(call cc-option,-mcpu=$(1),$(2))
> > endif
> > 
> > +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
> > @@ -25,11 +31,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)
> > -- 
> > 2.33.0
> 
> https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html says
> "If n is not specified or is zero, use a machine-dependent default."
> 
> Unless some other files specify -falign-loops=N and expect 0 to reset to
> the machine default, -falign-jumps=0 -falign-loops=0 -falign-functions=0 should just be dropped.

Grepping the tree, I see:

rg "align-(functions|jumps|loops)"
Makefile
977:KBUILD_CFLAGS += -falign-functions=64

arch/x86/Makefile
101:        KBUILD_CFLAGS += $(call cc-option,-falign-jumps=1)
104:        KBUILD_CFLAGS += $(call cc-option,-falign-loops=1)

arch/x86/Makefile_32.cpu
28:cflags-$(CONFIG_MCRUSOE)     += -march=i686 -falign-functions=0 -falign-jumps=0 -falign-loops=0
29:cflags-$(CONFIG_MEFFICEON)   += -march=i686 $(call tune,pentium3) -falign-functions=0 -falign-jumps=0 -falign-loops=0
32:cflags-$(CONFIG_MCYRIXIII)   += $(call cc-option,-march=c3,-march=i486) -falign-functions=0 -falign-jumps=0 -falign-loops=0

arch/ia64/Makefile
26:                -falign-functions=32 -frename-registers -fno-optimize-sibling-calls

The two cc-options calls in arch/x86/Makefile are for x86_64 only and
the Makefile use of -falign-functions=64 is for
DEBUG_FORCE_FUNCTION_ALIGN_64B, which is a debug option so it does not
seem like the flags are going to get overridden in a normal case.

However, I read the GCC docs as if functions are not aligned by default
and -falign-functions / -falign-functions=0 aligns them to a machine
specific default, so I am not sure if these flags can just be dropped?
These flags have been in the tree for 19 years though and there is very
little history that I can find around why they are there.

https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/tree/arch/i386/Makefile?id=7a2deb32924142696b8174cdf9b38cd72a11fc96

-O2 turns on -falign-{functions,jumps,loops} by default but the kernel
can use -Os, which omits those, so it is possible that is why they are
there? Some input from the x86 folks might be helpful around this :)

> BTW: I believe GCC 8 (likely when fixing another issue with a large refactor
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84100) introduced a bug
> that -falign-X=0 was essentially -falign-X=1.
> GCC 11.0 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96247) fixed the bug.

Cheers,
Nathan

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Nick Desaulniers]

On Mon, Aug 23, 2021 at 7:27 PM Nathan Chancellor <nathan@kernel.org> 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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  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 cd3056759880..e8c65f990afd 100644
> --- a/arch/x86/Makefile_32.cpu
> +++ b/arch/x86/Makefile_32.cpu
> @@ -10,6 +10,12 @@ else
>  tune           = $(call cc-option,-mcpu=$(1),$(2))
>  endif
>
> +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
> @@ -25,11 +31,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)
> --
> 2.33.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Nick Desaulniers]

On Mon, Aug 23, 2021 at 7:27 PM Nathan Chancellor <nathan@kernel.org> 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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  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 cd3056759880..e8c65f990afd 100644
> --- a/arch/x86/Makefile_32.cpu
> +++ b/arch/x86/Makefile_32.cpu
> @@ -10,6 +10,12 @@ else
>  tune           = $(call cc-option,-mcpu=$(1),$(2))
>  endif
>
> +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
> @@ -25,11 +31,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)
> --
> 2.33.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Borislav Petkov]

On Mon, Aug 23, 2021 at 07:26:39PM -0700, Nathan Chancellor wrote:

A couple of nitpicks:

> Subject: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

Make that prefix into "x86/build: " 

> 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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/

Also, there should be a second Link: tag which points to this mail
thread so that we can find it later, when we dig for the "why we did
that" question :)

I.e.,

Link: 20210824022640.2170859-2-nathan@kernel.org

> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  arch/x86/Makefile_32.cpu | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

with that:

Acked-by: Borislav Petkov <bp@suse.de>

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Nathan Chancellor]

On 9/16/2021 10:18 AM, Borislav Petkov wrote:
> On Mon, Aug 23, 2021 at 07:26:39PM -0700, Nathan Chancellor wrote:
> 
> A couple of nitpicks:
> 
>> Subject: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
> 
> Make that prefix into "x86/build: "

Done, I'll be sure to keep that prefix in mind for future flag-based 
changes.

>> 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/YSQE2f5teuvKLkON@Ryzen-9-3900X.localdomain/
> 
> Also, there should be a second Link: tag which points to this mail
> thread so that we can find it later, when we dig for the "why we did
> that" question :)
> 
> I.e.,
> 
> Link: 20210824022640.2170859-2-nathan@kernel.org

Sure thing, kind of hard to do that on the initial submission but I will 
do it for the v2 shortly :)

>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>> ---
>>   arch/x86/Makefile_32.cpu | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> with that:
> 
> Acked-by: Borislav Petkov <bp@suse.de>

Thank you for the ack. The conflicting changes that I mentioned in the 
cover letter have been merged in 5.15-rc1 so if you guys want to take 
these changes via -tip, just holler for an ack from Masahiro on the 
second patch on v2 (but I am going with the assumption this will be 
merged via the kbuild tree).

Cheers,
Nathan

Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang

[Borislav Petkov]

On Thu, Sep 16, 2021 at 11:42:19AM -0700, Nathan Chancellor wrote:
> Done, I'll be sure to keep that prefix in mind for future flag-based
> changes.

Yeah, what you could do in the future is

git log <filename>

and see the previous prefixes. But not that important - we fix those
usually before applying.

> Sure thing, kind of hard to do that on the initial submission but I will do
> it for the v2 shortly :)

Haha, very hard. :-)

> Thank you for the ack. The conflicting changes that I mentioned in the cover
> letter have been merged in 5.15-rc1 so if you guys want to take these
> changes via -tip, just holler for an ack from Masahiro on the second patch
> on v2 (but I am going with the assumption this will be merged via the kbuild
> tree).

I'm fine either way. So whatever Masahiro prefers.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Nathan Chancellor]

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.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 scripts/Makefile.clang | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
index 4cce8fd0779c..2fe38a9fdc11 100644
--- a/scripts/Makefile.clang
+++ b/scripts/Makefile.clang
@@ -29,7 +29,11 @@ CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
 else
 CLANG_FLAGS	+= -fintegrated-as
 endif
+# By default, clang only warns on unknown warning or optimization flags
+# 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

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

[Nick Desaulniers]

On Mon, Aug 23, 2021 at 7:27 PM Nathan Chancellor <nathan@kernel.org> 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.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Good idea.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  scripts/Makefile.clang | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 4cce8fd0779c..2fe38a9fdc11 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -29,7 +29,11 @@ CLANG_FLAGS  += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
>  else
>  CLANG_FLAGS    += -fintegrated-as
>  endif
> +# By default, clang only warns on unknown warning or optimization flags
> +# 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
>


-- 
Thanks,
~Nick Desaulniers

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

[Nick Desaulniers]

On Mon, Aug 23, 2021 at 7:27 PM Nathan Chancellor <nathan@kernel.org> 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.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Good idea.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  scripts/Makefile.clang | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 4cce8fd0779c..2fe38a9fdc11 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -29,7 +29,11 @@ CLANG_FLAGS  += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
>  else
>  CLANG_FLAGS    += -fintegrated-as
>  endif
> +# By default, clang only warns on unknown warning or optimization flags
> +# 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
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 0/2] Harden clang against unknown flag options

[Nathan Chancellor]

On Mon, Aug 23, 2021 at 07:26:38PM -0700, Nathan Chancellor wrote:
> 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.
> 
> I hope the patches in and of themselves are reasonable and
> non-controversial. This is based on Masahiro's kbuild tree as there was
> a fairly large refactor around where clang's flags were added so I
> figured it would be best to go there with an x86 ack since the first
> patch does not depend on anything in -tip.
> 
> Cheers,
> Nathan
> 
> Nathan Chancellor (2):
>   x86: 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   |  4 ++++
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> 
> base-commit: fb3fdea450305d932d933d7e75eead0477249d8e
> -- 
> 2.33.0
> 
> 

Hello x86 folks,

Would you guys be able to give me feedback on this series or accept it?
We are continuously getting false positive warning reports from i386 randconfigs.

https://lore.kernel.org/r/ece61908-f8eb-4c45-5d5f-bfc52f3b8f45@kernel.org/
https://lore.kernel.org/r/YT+RqrkQAOVhbkWu@archlinux-ax161/

Cheers,
Nathan