linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Harden clang against unknown flag options
@ 2021-08-24  2:26 Nathan Chancellor
  2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Nathan Chancellor @ 2021-08-24  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, Masahiro Yamada
  Cc: H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-08-24  2:26 [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor
@ 2021-08-24  2:26 ` Nathan Chancellor
  2021-08-24  2:56   ` Fangrui Song
                     ` (2 more replies)
  2021-08-24  2:26 ` [PATCH 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS Nathan Chancellor
  2021-09-13 18:08 ` [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor
  2 siblings, 3 replies; 11+ messages in thread
From: Nathan Chancellor @ 2021-08-24  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, Masahiro Yamada
  Cc: H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, Nathan Chancellor, kernel test robot

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS
  2021-08-24  2:26 [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor
  2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
@ 2021-08-24  2:26 ` Nathan Chancellor
  2021-08-25 22:27   ` Nick Desaulniers
  2021-09-13 18:08 ` [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor
  2 siblings, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2021-08-24  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, Masahiro Yamada
  Cc: H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
@ 2021-08-24  2:56   ` Fangrui Song
  2021-08-24 21:53     ` Nathan Chancellor
  2021-08-25 22:32   ` Nick Desaulniers
  2021-09-16 17:18   ` Borislav Petkov
  2 siblings, 1 reply; 11+ messages in thread
From: Fangrui Song @ 2021-08-24  2:56 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Masahiro Yamada, H. Peter Anvin, Nick Desaulniers, linux-kernel,
	linux-kbuild, clang-built-linux, llvm, kernel test robot

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-08-24  2:56   ` Fangrui Song
@ 2021-08-24 21:53     ` Nathan Chancellor
  0 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2021-08-24 21:53 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Masahiro Yamada, H. Peter Anvin, Nick Desaulniers, linux-kernel,
	linux-kbuild, clang-built-linux, llvm, kernel test robot

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS
  2021-08-24  2:26 ` [PATCH 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS Nathan Chancellor
@ 2021-08-25 22:27   ` Nick Desaulniers
  0 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2021-08-25 22:27 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Masahiro Yamada, H. Peter Anvin, linux-kernel, linux-kbuild,
	clang-built-linux, llvm

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
  2021-08-24  2:56   ` Fangrui Song
@ 2021-08-25 22:32   ` Nick Desaulniers
  2021-09-16 17:18   ` Borislav Petkov
  2 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2021-08-25 22:32 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Masahiro Yamada, H. Peter Anvin, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, kernel test robot

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Harden clang against unknown flag options
  2021-08-24  2:26 [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor
  2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
  2021-08-24  2:26 ` [PATCH 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS Nathan Chancellor
@ 2021-09-13 18:08 ` Nathan Chancellor
  2 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2021-09-13 18:08 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, Masahiro Yamada
  Cc: H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
  2021-08-24  2:56   ` Fangrui Song
  2021-08-25 22:32   ` Nick Desaulniers
@ 2021-09-16 17:18   ` Borislav Petkov
  2021-09-16 18:42     ` Nathan Chancellor
  2 siblings, 1 reply; 11+ messages in thread
From: Borislav Petkov @ 2021-09-16 17:18 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masahiro Yamada,
	H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, kernel test robot

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-09-16 17:18   ` Borislav Petkov
@ 2021-09-16 18:42     ` Nathan Chancellor
  2021-09-16 19:05       ` Borislav Petkov
  0 siblings, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2021-09-16 18:42 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masahiro Yamada,
	H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, kernel test robot

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang
  2021-09-16 18:42     ` Nathan Chancellor
@ 2021-09-16 19:05       ` Borislav Petkov
  0 siblings, 0 replies; 11+ messages in thread
From: Borislav Petkov @ 2021-09-16 19:05 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Thomas Gleixner, Ingo Molnar, x86, Masahiro Yamada,
	H. Peter Anvin, Nick Desaulniers, linux-kernel, linux-kbuild,
	clang-built-linux, llvm, kernel test robot

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-09-16 19:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24  2:26 [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor
2021-08-24  2:26 ` [PATCH 1/2] x86: Do not add -falign flags unconditionally for clang Nathan Chancellor
2021-08-24  2:56   ` Fangrui Song
2021-08-24 21:53     ` Nathan Chancellor
2021-08-25 22:32   ` Nick Desaulniers
2021-09-16 17:18   ` Borislav Petkov
2021-09-16 18:42     ` Nathan Chancellor
2021-09-16 19:05       ` Borislav Petkov
2021-08-24  2:26 ` [PATCH 2/2] kbuild: Add -Werror=ignored-optimization-argument to CLANG_FLAGS Nathan Chancellor
2021-08-25 22:27   ` Nick Desaulniers
2021-09-13 18:08 ` [PATCH 0/2] Harden clang against unknown flag options Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).