All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] LLVM: make x86_64 kernel build with clang.
@ 2017-03-17  0:15 Michael Davidson
  2017-03-17  0:15 ` [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS Michael Davidson
                   ` (7 more replies)
  0 siblings, 8 replies; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

This patch set is sufficient to get the x86_64 kernel to build
and boot correctly with clang-3.8 or greater.

The resulting build still has about 300 warnings, very few of
which appear to be significant. Most of them should be fixable
with some minor code refactoring although a few of them, such
as the complaints about implict conversions between enumerated
types may be candidates for just being disabled.

Michael Davidson (7):
  Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
  Makefile, x86, LLVM: disable unsupported optimization flags
  x86, LLVM: suppress clang warnings about unaligned accesses
  x86, boot, LLVM: #undef memcpy etc in string.c
  x86, boot, LLVM: Use regparm=0 for memcpy and memset
  md/raid10, LLVM: get rid of variable length array
  crypto, x86, LLVM: aesni - fix token pasting

 Makefile                                |  4 ++++
 arch/x86/Makefile                       |  7 +++++++
 arch/x86/boot/copy.S                    | 15 +++++++++++++--
 arch/x86/boot/string.c                  |  9 +++++++++
 arch/x86/boot/string.h                  | 13 +++++++++++++
 arch/x86/crypto/aes_ctrby8_avx-x86_64.S |  7 ++-----
 drivers/md/raid10.c                     |  9 ++++-----
 7 files changed, 52 insertions(+), 12 deletions(-)

-- 
2.12.0.367.g23dc2f6d3c-goog


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

* [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-04-03 22:49   ` Matthias Kaehlcke
  2017-04-21  7:49     ` Masahiro Yamada
  2017-03-17  0:15 ` [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags Michael Davidson
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
for clang.

Signed-off-by: Michael Davidson <md@google.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index b841fb36beb2..b21fd0ca2946 100644
--- a/Makefile
+++ b/Makefile
@@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
 # See modpost pattern 2
 KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
 KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
+KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
+KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
 else
 
 # These warnings generated too much noise in a regular build.
-- 
2.12.0.367.g23dc2f6d3c-goog

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

* [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
  2017-03-17  0:15 ` [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-03-17 21:32   ` H. Peter Anvin
  2017-04-05 18:08   ` Masahiro Yamada
  2017-03-17  0:15 ` [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses Michael Davidson
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

Unfortunately, while clang generates a warning about these flags
being unsupported it still exits with a status of 0 so we have
to explicitly disable them instead of just using a cc-option check.

Signed-off-by: Michael Davidson <md@google.com>
---
 Makefile          | 2 ++
 arch/x86/Makefile | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index b21fd0ca2946..5e97e5fc1eea 100644
--- a/Makefile
+++ b/Makefile
@@ -629,7 +629,9 @@ ARCH_AFLAGS :=
 ARCH_CFLAGS :=
 include arch/$(SRCARCH)/Makefile
 
+ifneq ($(cc-name),clang)
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
+endif
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d449337a360..894a8d18bf97 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -87,11 +87,13 @@ else
         KBUILD_AFLAGS += -m64
         KBUILD_CFLAGS += -m64
 
+ifneq ($(cc-name),clang)
         # Align jump targets to 1 byte, not the default 16 bytes:
         KBUILD_CFLAGS += -falign-jumps=1
 
         # Pack loops tightly as well:
         KBUILD_CFLAGS += -falign-loops=1
+endif
 
         # Don't autogenerate traditional x87 instructions
         KBUILD_CFLAGS += $(call cc-option,-mno-80387)
-- 
2.12.0.367.g23dc2f6d3c-goog


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

* [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
  2017-03-17  0:15 ` [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS Michael Davidson
  2017-03-17  0:15 ` [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-03-17 23:50   ` hpa
  2017-03-17  0:15 ` [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c Michael Davidson
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

Suppress clang warnings about potential unaliged accesses
to members in packed structs. This gets rid of almost 10,000
warnings about accesses to the ring 0 stack pointer in the TSS.

Signed-off-by: Michael Davidson <md@google.com>
---
 arch/x86/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 894a8d18bf97..7f21703c475d 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -128,6 +128,11 @@ endif
         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
 endif
 
+ifeq ($(cc-name),clang)
+# Suppress clang warnings about potential unaligned accesses.
+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
+endif
+
 ifdef CONFIG_X86_X32
 	x32_ld_ok := $(call try-run,\
 			/bin/echo -e '1: .quad 1b' | \
-- 
2.12.0.367.g23dc2f6d3c-goog


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

* [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
                   ` (2 preceding siblings ...)
  2017-03-17  0:15 ` [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-06-22 22:31   ` Matthias Kaehlcke
  2017-03-17  0:15 ` [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset Michael Davidson
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

undef memcpy and friends in boot/string.c so that the functions
defined here will have the correct names, otherwise we end up
up trying to redefine __builtin_memcpy etc.
Surprisingly, gcc allows this (and, helpfully, discards the
__builtin_ prefix from the function name when compiling it),
but clang does not.

Adding these #undef's appears to preserve what I assume was
the original intent of the code.

Signed-off-by: Michael Davidson <md@google.com>
---
 arch/x86/boot/string.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index 5457b02fc050..b40266850869 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -16,6 +16,15 @@
 #include "ctype.h"
 #include "string.h"
 
+/*
+ * Undef these macros so that the functions that we provide
+ * here will have the correct names regardless of how string.h
+ * may have chosen to #define them.
+ */
+#undef memcpy
+#undef memset
+#undef memcmp
+
 int memcmp(const void *s1, const void *s2, size_t len)
 {
 	bool diff;
-- 
2.12.0.367.g23dc2f6d3c-goog

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

* [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
                   ` (3 preceding siblings ...)
  2017-03-17  0:15 ` [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-03-17 12:08   ` Peter Zijlstra
  2017-03-17  0:15 ` [PATCH 6/7] md/raid10, LLVM: get rid of variable length array Michael Davidson
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

Use the standard regparm=0 calling convention for memcpy and
memset when building with clang.

This is a work around for a long standing clang bug
(see https://llvm.org/bugs/show_bug.cgi?id=3997) where
clang always uses the standard regparm=0 calling convention
for any implcit calls to memcpy and memset that it generates
(eg for structure assignments and initialization) even if an
alternate calling convention such as regparm=3 has been specified.

Signed-off-by: Michael Davidson <md@google.com>
---
 arch/x86/boot/copy.S   | 15 +++++++++++++--
 arch/x86/boot/string.h | 13 +++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/copy.S b/arch/x86/boot/copy.S
index 1eb7d298b47d..57142d1ad0d2 100644
--- a/arch/x86/boot/copy.S
+++ b/arch/x86/boot/copy.S
@@ -18,6 +18,12 @@
 	.text
 
 GLOBAL(memcpy)
+#ifdef	__clang__	/* Use normal ABI calling conventions */
+	movw	4(%esp), %ax
+	movw	8(%esp), %dx
+	movw	12(%esp), %cx
+#endif
+_memcpy:
 	pushw	%si
 	pushw	%di
 	movw	%ax, %di
@@ -34,6 +40,11 @@ GLOBAL(memcpy)
 ENDPROC(memcpy)
 
 GLOBAL(memset)
+#ifdef	__clang__	/* Use normal ABI calling conventions */
+	movw	4(%esp), %ax
+	movw	8(%esp), %dx
+	movw	12(%esp), %cx
+#endif
 	pushw	%di
 	movw	%ax, %di
 	movzbl	%dl, %eax
@@ -52,7 +63,7 @@ GLOBAL(copy_from_fs)
 	pushw	%ds
 	pushw	%fs
 	popw	%ds
-	calll	memcpy
+	calll	_memcpy
 	popw	%ds
 	retl
 ENDPROC(copy_from_fs)
@@ -61,7 +72,7 @@ GLOBAL(copy_to_fs)
 	pushw	%es
 	pushw	%fs
 	popw	%es
-	calll	memcpy
+	calll	_memcpy
 	popw	%es
 	retl
 ENDPROC(copy_to_fs)
diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
index 113588ddb43f..e735cccb3fc8 100644
--- a/arch/x86/boot/string.h
+++ b/arch/x86/boot/string.h
@@ -6,8 +6,21 @@
 #undef memset
 #undef memcmp
 
+/*
+ * Use normal ABI calling conventions - i.e. regparm(0) -
+ * for memcpy() and memset() if we are building the real
+ * mode setup code with clang since clang may make implicit
+ * calls to these functions that assume regparm(0).
+ */
+#if defined(_SETUP) && defined(__clang__)
+void __attribute__((regparm(0))) *memcpy(void *dst, const void *src,
+					 size_t len);
+void __attribute__((regparm(0))) *memset(void *dst, int c, size_t len);
+#else
 void *memcpy(void *dst, const void *src, size_t len);
 void *memset(void *dst, int c, size_t len);
+#endif
+
 int memcmp(const void *s1, const void *s2, size_t len);
 
 /*
-- 
2.12.0.367.g23dc2f6d3c-goog


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

* [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
                   ` (4 preceding siblings ...)
  2017-03-17  0:15 ` [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-03-17 12:08   ` Peter Zijlstra
  2017-03-17  0:15 ` [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting Michael Davidson
  2017-03-17  8:17 ` [PATCH 0/7] LLVM: make x86_64 kernel build with clang Dmitry Vyukov
  7 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

Replace a variable length array in a struct by allocating
the memory for the entire struct in a char array on the stack.

Signed-off-by: Michael Davidson <md@google.com>
---
 drivers/md/raid10.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 063c43d83b72..158ebdff782c 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
 	/* Use sync reads to get the blocks from somewhere else */
 	int sectors = r10_bio->sectors;
 	struct r10conf *conf = mddev->private;
-	struct {
-		struct r10bio r10_bio;
-		struct r10dev devs[conf->copies];
-	} on_stack;
-	struct r10bio *r10b = &on_stack.r10_bio;
+	char on_stack_r10_bio[sizeof(struct r10bio) +
+			      conf->copies * sizeof(struct r10dev)]
+			      __aligned(__alignof__(struct r10bio));
+	struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
 	int slot = 0;
 	int idx = 0;
 	struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
-- 
2.12.0.367.g23dc2f6d3c-goog


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

* [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
                   ` (5 preceding siblings ...)
  2017-03-17  0:15 ` [PATCH 6/7] md/raid10, LLVM: get rid of variable length array Michael Davidson
@ 2017-03-17  0:15 ` Michael Davidson
  2017-04-03 23:14   ` Matthias Kaehlcke
  2017-03-17  8:17 ` [PATCH 0/7] LLVM: make x86_64 kernel build with clang Dmitry Vyukov
  7 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-03-17  0:15 UTC (permalink / raw)
  To: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid,
	Michael Davidson

aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting
of character sequences that are not valid preprocessor tokens.
While this is allowed when preprocessing assembler files it exposes
an incompatibilty between the clang and gcc preprocessors where
clang does not strip leading white space from macro parameters,
leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting
in a token with a space character embedded in it.

While this could be fixed by deleting the offending space character,
the assembler is perfectly capable of handling the token pasting
correctly for itself so it seems preferable to let it do so and to
get rid or the CONCAT(), DDQ() and XMM() preprocessor macros.

Signed-off-by: Michael Davidson <md@google.com>
---
 arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
index a916c4a61165..5f6a5af9c489 100644
--- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
+++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
@@ -65,7 +65,6 @@
 #include <linux/linkage.h>
 #include <asm/inst.h>
 
-#define CONCAT(a,b)	a##b
 #define VMOVDQ		vmovdqu
 
 #define xdata0		%xmm0
@@ -92,8 +91,6 @@
 #define num_bytes	%r8
 
 #define tmp		%r10
-#define	DDQ(i)		CONCAT(ddq_add_,i)
-#define	XMM(i)		CONCAT(%xmm, i)
 #define	DDQ_DATA	0
 #define	XDATA		1
 #define KEY_128		1
@@ -131,12 +128,12 @@ ddq_add_8:
 /* generate a unique variable for ddq_add_x */
 
 .macro setddq n
-	var_ddq_add = DDQ(\n)
+	var_ddq_add = ddq_add_\n
 .endm
 
 /* generate a unique variable for xmm register */
 .macro setxdata n
-	var_xdata = XMM(\n)
+	var_xdata = %xmm\n
 .endm
 
 /* club the numeric 'id' to the symbol 'name' */
-- 
2.12.0.367.g23dc2f6d3c-goog

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

* Re: [PATCH 0/7] LLVM: make x86_64 kernel build with clang.
  2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
                   ` (6 preceding siblings ...)
  2017-03-17  0:15 ` [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting Michael Davidson
@ 2017-03-17  8:17 ` Dmitry Vyukov
  7 siblings, 0 replies; 50+ messages in thread
From: Dmitry Vyukov @ 2017-03-17  8:17 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Matthias Kaehlcke, x86, open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 1:15 AM, Michael Davidson <md@google.com> wrote:
> This patch set is sufficient to get the x86_64 kernel to build
> and boot correctly with clang-3.8 or greater.
>
> The resulting build still has about 300 warnings, very few of
> which appear to be significant. Most of them should be fixable
> with some minor code refactoring although a few of them, such
> as the complaints about implict conversions between enumerated
> types may be candidates for just being disabled.

Thanks, Michael!
This will help us a lot with KMSAN (uninit use detector) and code coverage.


> Michael Davidson (7):
>   Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
>   Makefile, x86, LLVM: disable unsupported optimization flags
>   x86, LLVM: suppress clang warnings about unaligned accesses
>   x86, boot, LLVM: #undef memcpy etc in string.c
>   x86, boot, LLVM: Use regparm=0 for memcpy and memset
>   md/raid10, LLVM: get rid of variable length array
>   crypto, x86, LLVM: aesni - fix token pasting
>
>  Makefile                                |  4 ++++
>  arch/x86/Makefile                       |  7 +++++++
>  arch/x86/boot/copy.S                    | 15 +++++++++++++--
>  arch/x86/boot/string.c                  |  9 +++++++++
>  arch/x86/boot/string.h                  | 13 +++++++++++++
>  arch/x86/crypto/aes_ctrby8_avx-x86_64.S |  7 ++-----
>  drivers/md/raid10.c                     |  9 ++++-----
>  7 files changed, 52 insertions(+), 12 deletions(-)
>
> --
> 2.12.0.367.g23dc2f6d3c-goog
>

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17  0:15 ` [PATCH 6/7] md/raid10, LLVM: get rid of variable length array Michael Davidson
@ 2017-03-17 12:08   ` Peter Zijlstra
  2017-03-17 12:31     ` Alexander Potapenko
  0 siblings, 1 reply; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 12:08 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild,
	linux-kernel, linux-crypto, linux-raid

On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
> Replace a variable length array in a struct by allocating
> the memory for the entire struct in a char array on the stack.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  drivers/md/raid10.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 063c43d83b72..158ebdff782c 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
>  	/* Use sync reads to get the blocks from somewhere else */
>  	int sectors = r10_bio->sectors;
>  	struct r10conf *conf = mddev->private;
> -	struct {
> -		struct r10bio r10_bio;
> -		struct r10dev devs[conf->copies];
> -	} on_stack;
> -	struct r10bio *r10b = &on_stack.r10_bio;
> +	char on_stack_r10_bio[sizeof(struct r10bio) +
> +			      conf->copies * sizeof(struct r10dev)]
> +			      __aligned(__alignof__(struct r10bio));
> +	struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
>  	int slot = 0;
>  	int idx = 0;
>  	struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;


That's disgusting. Why not fix LLVM to support this?

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

* Re: [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset
  2017-03-17  0:15 ` [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset Michael Davidson
@ 2017-03-17 12:08   ` Peter Zijlstra
  2017-06-22 22:38     ` H. Peter Anvin
  0 siblings, 1 reply; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 12:08 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild,
	linux-kernel, linux-crypto, linux-raid

On Thu, Mar 16, 2017 at 05:15:18PM -0700, Michael Davidson wrote:
> Use the standard regparm=0 calling convention for memcpy and
> memset when building with clang.
> 
> This is a work around for a long standing clang bug
> (see https://llvm.org/bugs/show_bug.cgi?id=3997) where
> clang always uses the standard regparm=0 calling convention
> for any implcit calls to memcpy and memset that it generates
> (eg for structure assignments and initialization) even if an
> alternate calling convention such as regparm=3 has been specified.

Seriously, fix LLVM already.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 12:08   ` Peter Zijlstra
@ 2017-03-17 12:31     ` Alexander Potapenko
  2017-03-17 12:32       ` Alexander Potapenko
  2017-03-17 12:44       ` Peter Zijlstra
  0 siblings, 2 replies; 50+ messages in thread
From: Alexander Potapenko @ 2017-03-17 12:31 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 1:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
>> Replace a variable length array in a struct by allocating
>> the memory for the entire struct in a char array on the stack.
>>
>> Signed-off-by: Michael Davidson <md@google.com>
>> ---
>>  drivers/md/raid10.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 063c43d83b72..158ebdff782c 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
>>       /* Use sync reads to get the blocks from somewhere else */
>>       int sectors = r10_bio->sectors;
>>       struct r10conf *conf = mddev->private;
>> -     struct {
>> -             struct r10bio r10_bio;
>> -             struct r10dev devs[conf->copies];
>> -     } on_stack;
>> -     struct r10bio *r10b = &on_stack.r10_bio;
>> +     char on_stack_r10_bio[sizeof(struct r10bio) +
>> +                           conf->copies * sizeof(struct r10dev)]
>> +                           __aligned(__alignof__(struct r10bio));
>> +     struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
>>       int slot = 0;
>>       int idx = 0;
>>       struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
>
>
> That's disgusting. Why not fix LLVM to support this?

IIUC there's only a handful of VLAIS instances in LLVM code, why not
just drop them for the sake of better code portability?
(To quote Linus, "this feature is an abomination":
https://lkml.org/lkml/2013/9/23/500)

-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 12:31     ` Alexander Potapenko
@ 2017-03-17 12:32       ` Alexander Potapenko
  2017-03-17 18:03         ` Borislav Petkov
  2017-03-17 12:44       ` Peter Zijlstra
  1 sibling, 1 reply; 50+ messages in thread
From: Alexander Potapenko @ 2017-03-17 12:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 1:31 PM, Alexander Potapenko <glider@google.com> wrote:
> On Fri, Mar 17, 2017 at 1:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
>>> Replace a variable length array in a struct by allocating
>>> the memory for the entire struct in a char array on the stack.
>>>
>>> Signed-off-by: Michael Davidson <md@google.com>
>>> ---
>>>  drivers/md/raid10.c | 9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index 063c43d83b72..158ebdff782c 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>>> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
>>>       /* Use sync reads to get the blocks from somewhere else */
>>>       int sectors = r10_bio->sectors;
>>>       struct r10conf *conf = mddev->private;
>>> -     struct {
>>> -             struct r10bio r10_bio;
>>> -             struct r10dev devs[conf->copies];
>>> -     } on_stack;
>>> -     struct r10bio *r10b = &on_stack.r10_bio;
>>> +     char on_stack_r10_bio[sizeof(struct r10bio) +
>>> +                           conf->copies * sizeof(struct r10dev)]
>>> +                           __aligned(__alignof__(struct r10bio));
>>> +     struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
>>>       int slot = 0;
>>>       int idx = 0;
>>>       struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
>>
>>
>> That's disgusting. Why not fix LLVM to support this?
>
> IIUC there's only a handful of VLAIS instances in LLVM code, why not
Sorry, "kernel code", not "LLVM code".
> just drop them for the sake of better code portability?
> (To quote Linus, "this feature is an abomination":
> https://lkml.org/lkml/2013/9/23/500)
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 12:31     ` Alexander Potapenko
  2017-03-17 12:32       ` Alexander Potapenko
@ 2017-03-17 12:44       ` Peter Zijlstra
  2017-03-17 18:52         ` Michael Davidson
  1 sibling, 1 reply; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 12:44 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 01:31:23PM +0100, Alexander Potapenko wrote:
> On Fri, Mar 17, 2017 at 1:08 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Mar 16, 2017 at 05:15:19PM -0700, Michael Davidson wrote:
> >> Replace a variable length array in a struct by allocating
> >> the memory for the entire struct in a char array on the stack.
> >>
> >> Signed-off-by: Michael Davidson <md@google.com>
> >> ---
> >>  drivers/md/raid10.c | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> >> index 063c43d83b72..158ebdff782c 100644
> >> --- a/drivers/md/raid10.c
> >> +++ b/drivers/md/raid10.c
> >> @@ -4654,11 +4654,10 @@ static int handle_reshape_read_error(struct mddev *mddev,
> >>       /* Use sync reads to get the blocks from somewhere else */
> >>       int sectors = r10_bio->sectors;
> >>       struct r10conf *conf = mddev->private;
> >> -     struct {
> >> -             struct r10bio r10_bio;
> >> -             struct r10dev devs[conf->copies];
> >> -     } on_stack;
> >> -     struct r10bio *r10b = &on_stack.r10_bio;
> >> +     char on_stack_r10_bio[sizeof(struct r10bio) +
> >> +                           conf->copies * sizeof(struct r10dev)]
> >> +                           __aligned(__alignof__(struct r10bio));
> >> +     struct r10bio *r10b = (struct r10bio *)on_stack_r10_bio;
> >>       int slot = 0;
> >>       int idx = 0;
> >>       struct bio_vec *bvec = r10_bio->master_bio->bi_io_vec;
> >
> >
> > That's disgusting. Why not fix LLVM to support this?
> 
> IIUC there's only a handful of VLAIS instances in LLVM code, why not
> just drop them for the sake of better code portability?
> (To quote Linus, "this feature is an abomination":
> https://lkml.org/lkml/2013/9/23/500)

Be that as it may; what you construct above is disgusting. Surely the
code can be refactored to not look like dog vomit?

Also; its not immediately obvious conf->copies is 'small' and this
doesn't blow up the stack; I feel that deserves a comment somewhere.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 12:32       ` Alexander Potapenko
@ 2017-03-17 18:03         ` Borislav Petkov
  2017-03-17 18:47           ` Dmitry Vyukov
  0 siblings, 1 reply; 50+ messages in thread
From: Borislav Petkov @ 2017-03-17 18:03 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Peter Zijlstra, Michael Davidson, Michal Marek, Thomas Gleixner,
	Ingo Molnar, H. Peter Anvin, Herbert Xu, David S. Miller,
	Shaohua Li, Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild,
	LKML, linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 01:32:00PM +0100, Alexander Potapenko wrote:
> > IIUC there's only a handful of VLAIS instances in LLVM code, why not
> Sorry, "kernel code", not "LLVM code".
> > just drop them for the sake of better code portability?

And what happens if someone else adds a variable thing like this
somewhere else, builds with gcc, everything's fine and patch gets
applied? Or something else llvm can't stomach.

Does that mean there'll be the occasional, every-so-often whack-a-mole
patchset from someone, fixing the kernel build with llvm yet again?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 18:03         ` Borislav Petkov
@ 2017-03-17 18:47           ` Dmitry Vyukov
  2017-03-17 18:57             ` Borislav Petkov
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Vyukov @ 2017-03-17 18:47 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all

On Fri, Mar 17, 2017 at 7:03 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 17, 2017 at 01:32:00PM +0100, Alexander Potapenko wrote:
>> > IIUC there's only a handful of VLAIS instances in LLVM code, why not
>> Sorry, "kernel code", not "LLVM code".
>> > just drop them for the sake of better code portability?
>
> And what happens if someone else adds a variable thing like this
> somewhere else, builds with gcc, everything's fine and patch gets
> applied? Or something else llvm can't stomach.
>
> Does that mean there'll be the occasional, every-so-often whack-a-mole
> patchset from someone, fixing the kernel build with llvm yet again?


This problem is more general and is not specific to clang. It equally
applies to different versions of gcc, different arches and different
configs (namely, anything else than what a developer used for
testing). A known, reasonably well working solution to this problem is
a system of try bots that test patches before commit with different
compilers/configs/archs. We already have such system in the form of
0-day bots. It would be useful to extend it with clang as soon as
kernel builds.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 12:44       ` Peter Zijlstra
@ 2017-03-17 18:52         ` Michael Davidson
  2017-03-17 19:27           ` Peter Zijlstra
  0 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-03-17 18:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Be that as it may; what you construct above is disgusting. Surely the
> code can be refactored to not look like dog vomit?
>
> Also; its not immediately obvious conf->copies is 'small' and this
> doesn't blow up the stack; I feel that deserves a comment somewhere.
>

I agree that the code is horrible.

It is, in fact, exactly the same solution that was used to remove
variable length arrays in structs from several of the crypto drivers a
few years ago - see the definition of SHASH_DESC_ON_STACK() in
"crypto/hash.h" - I did not, however, hide the horrors in a macro
preferring to leave the implementation visible as a warning to whoever
might touch the code next.

I believe that the actual stack usage is exactly the same as it was previously.

I can certainly wrap this  up in a macro and add comments with
appropriately dire warnings in it if you feel that is both necessary
and sufficient.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 18:47           ` Dmitry Vyukov
@ 2017-03-17 18:57             ` Borislav Petkov
  2017-03-17 19:05               ` Dmitry Vyukov
  0 siblings, 1 reply; 50+ messages in thread
From: Borislav Petkov @ 2017-03-17 18:57 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all

On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
> This problem is more general and is not specific to clang. It equally
> applies to different versions of gcc, different arches and different
> configs (namely, anything else than what a developer used for
> testing).

I guess. We do carry a bunch of gcc workarounds along with the cc-*
macros in scripts/Kbuild.include.

> A known, reasonably well working solution to this problem is
> a system of try bots that test patches before commit with different
> compilers/configs/archs. We already have such system in the form of
> 0-day bots. It would be useful to extend it with clang as soon as
> kernel builds.

Has someone actually already talked to Fengguang about it?

Oh, and the stupid question: why the effort to build the kernel
with clang at all? Just because or are there some actual, palpable
advantages?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 18:57             ` Borislav Petkov
@ 2017-03-17 19:05               ` Dmitry Vyukov
  2017-03-17 19:26                   ` Peter Zijlstra
  2017-03-18  0:41                 ` Fengguang Wu
  0 siblings, 2 replies; 50+ messages in thread
From: Dmitry Vyukov @ 2017-03-17 19:05 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 17, 2017 at 7:57 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
>> This problem is more general and is not specific to clang. It equally
>> applies to different versions of gcc, different arches and different
>> configs (namely, anything else than what a developer used for
>> testing).
>
> I guess. We do carry a bunch of gcc workarounds along with the cc-*
> macros in scripts/Kbuild.include.
>
>> A known, reasonably well working solution to this problem is
>> a system of try bots that test patches before commit with different
>> compilers/configs/archs. We already have such system in the form of
>> 0-day bots. It would be useful to extend it with clang as soon as
>> kernel builds.
>
> Has someone actually already talked to Fengguang about it?

+Fengguang

> Oh, and the stupid question: why the effort to build the kernel
> with clang at all? Just because or are there some actual, palpable
> advantages?

On our side it is:
 - clang make it possible to implement KMSAN (dynamic detection of
uses of uninit memory)
 - better code coverage for fuzzing
 - why simpler and faster development (e.g. we can port our user-space
hardening technologies -- CFI and SafeStack)

You can also find some reasons in the Why section of LLVM-Linux project:
http://llvm.linuxfoundation.org/index.php/Main_Page

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 19:05               ` Dmitry Vyukov
@ 2017-03-17 19:26                   ` Peter Zijlstra
  2017-03-18  0:41                 ` Fengguang Wu
  1 sibling, 0 replies; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 19:26 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> You can also find some reasons in the Why section of LLVM-Linux project:
> http://llvm.linuxfoundation.org/index.php/Main_Page

>From that:

 - LLVM/Clang is a fast moving project with many things fixed quickly
   and features added.

So what's the deal with that 5 year old bug you want us to work around?

Also, clang doesn't support asm cc flags output and a few other
extensions last time I checked.


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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
@ 2017-03-17 19:26                   ` Peter Zijlstra
  0 siblings, 0 replies; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 19:26 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> You can also find some reasons in the Why section of LLVM-Linux project:
> http://llvm.linuxfoundation.org/index.php/Main_Page

From that:

 - LLVM/Clang is a fast moving project with many things fixed quickly
   and features added.

So what's the deal with that 5 year old bug you want us to work around?

Also, clang doesn't support asm cc flags output and a few other
extensions last time I checked.


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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 18:52         ` Michael Davidson
@ 2017-03-17 19:27           ` Peter Zijlstra
  2017-03-17 20:04             ` hpa
  0 siblings, 1 reply; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 19:27 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Be that as it may; what you construct above is disgusting. Surely the
> > code can be refactored to not look like dog vomit?
> >
> > Also; its not immediately obvious conf->copies is 'small' and this
> > doesn't blow up the stack; I feel that deserves a comment somewhere.
> >
> 
> I agree that the code is horrible.
> 
> It is, in fact, exactly the same solution that was used to remove
> variable length arrays in structs from several of the crypto drivers a
> few years ago - see the definition of SHASH_DESC_ON_STACK() in
> "crypto/hash.h" - I did not, however, hide the horrors in a macro
> preferring to leave the implementation visible as a warning to whoever
> might touch the code next.
> 
> I believe that the actual stack usage is exactly the same as it was previously.
> 
> I can certainly wrap this  up in a macro and add comments with
> appropriately dire warnings in it if you feel that is both necessary
> and sufficient.

We got away with ugly in the past, so we should get to do it again?

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 19:26                   ` Peter Zijlstra
  (?)
@ 2017-03-17 19:29                   ` Peter Zijlstra
  2017-03-24 13:50                     ` Dmitry Vyukov
  -1 siblings, 1 reply; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-17 19:29 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 17, 2017 at 08:26:42PM +0100, Peter Zijlstra wrote:
> On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> > You can also find some reasons in the Why section of LLVM-Linux project:
> > http://llvm.linuxfoundation.org/index.php/Main_Page
> 
> From that:
> 
>  - LLVM/Clang is a fast moving project with many things fixed quickly
>    and features added.
> 
> So what's the deal with that 5 year old bug you want us to work around?
> 
> Also, clang doesn't support asm cc flags output and a few other
> extensions last time I checked.
> 

Another great one:

 - BSD License (some people prefer this license to the GPL)

Seems a very weak argument to make when talking about the Linux Kernel
which is very explicitly GPLv2 (and not later).

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 19:27           ` Peter Zijlstra
@ 2017-03-17 20:04             ` hpa
  2017-03-24 13:47               ` Dmitry Vyukov
  0 siblings, 1 reply; 50+ messages in thread
From: hpa @ 2017-03-17 20:04 UTC (permalink / raw)
  To: Peter Zijlstra, Michael Davidson
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Dmitry Vyukov,
	Matthias Kaehlcke, x86, linux-kbuild, LKML, linux-crypto,
	linux-raid

On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
>> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra
><peterz@infradead.org> wrote:
>> >
>> > Be that as it may; what you construct above is disgusting. Surely
>the
>> > code can be refactored to not look like dog vomit?
>> >
>> > Also; its not immediately obvious conf->copies is 'small' and this
>> > doesn't blow up the stack; I feel that deserves a comment
>somewhere.
>> >
>> 
>> I agree that the code is horrible.
>> 
>> It is, in fact, exactly the same solution that was used to remove
>> variable length arrays in structs from several of the crypto drivers
>a
>> few years ago - see the definition of SHASH_DESC_ON_STACK() in
>> "crypto/hash.h" - I did not, however, hide the horrors in a macro
>> preferring to leave the implementation visible as a warning to
>whoever
>> might touch the code next.
>> 
>> I believe that the actual stack usage is exactly the same as it was
>previously.
>> 
>> I can certainly wrap this  up in a macro and add comments with
>> appropriately dire warnings in it if you feel that is both necessary
>> and sufficient.
>
>We got away with ugly in the past, so we should get to do it again?

Seriously, you should have taken the hack the first time that this needs to be fixed.  Just because this is a fairly uncommon construct in the kernel doesn't mean it is not in userspace.

I would like to say this falls in the category of "fix your compiler this time".  Once is one thing, twice is unacceptable.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-03-17  0:15 ` [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags Michael Davidson
@ 2017-03-17 21:32   ` H. Peter Anvin
  2017-03-17 21:34     ` H. Peter Anvin
  2017-04-05 18:08   ` Masahiro Yamada
  1 sibling, 1 reply; 50+ messages in thread
From: H. Peter Anvin @ 2017-03-17 21:32 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid

On 03/16/17 17:15, Michael Davidson wrote:
> Unfortunately, while clang generates a warning about these flags
> being unsupported it still exits with a status of 0 so we have
> to explicitly disable them instead of just using a cc-option check.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  Makefile          | 2 ++
>  arch/x86/Makefile | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b21fd0ca2946..5e97e5fc1eea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -629,7 +629,9 @@ ARCH_AFLAGS :=
>  ARCH_CFLAGS :=
>  include arch/$(SRCARCH)/Makefile
>  
> +ifneq ($(cc-name),clang)
>  KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
> +endif
>  KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
>  
>  ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2d449337a360..894a8d18bf97 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -87,11 +87,13 @@ else
>          KBUILD_AFLAGS += -m64
>          KBUILD_CFLAGS += -m64
>  
> +ifneq ($(cc-name),clang)
>          # Align jump targets to 1 byte, not the default 16 bytes:
>          KBUILD_CFLAGS += -falign-jumps=1
>  
>          # Pack loops tightly as well:
>          KBUILD_CFLAGS += -falign-loops=1
> +endif
>  
>          # Don't autogenerate traditional x87 instructions
>          KBUILD_CFLAGS += $(call cc-option,-mno-80387)
> 

NAK.  Fix your compiler, or use a wrapper script or something.  It is
absolutely *not* acceptable to disable this since future versions of
clang *should* support that.

That being said, it might make sense to look for a key pattern like
"(un|not )supported" on stderr the try-run macro.  Is there really no
-Wno- or -Werror= option to turn off this craziness?

	-hpa


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

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-03-17 21:32   ` H. Peter Anvin
@ 2017-03-17 21:34     ` H. Peter Anvin
  0 siblings, 0 replies; 50+ messages in thread
From: H. Peter Anvin @ 2017-03-17 21:34 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid

On 03/17/17 14:32, H. Peter Anvin wrote:
> 
> NAK.  Fix your compiler, or use a wrapper script or something.  It is
> absolutely *not* acceptable to disable this since future versions of
> clang *should* support that.
> 
> That being said, it might make sense to look for a key pattern like
> "(un|not )supported" on stderr the try-run macro.  Is there really no
> -Wno- or -Werror= option to turn off this craziness?
> 

Well, guess what... I found it myself.

-W{no-,error=}ignored-optimization-argument

Either variant will make this sane.

	-hpa



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

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-03-17  0:15 ` [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses Michael Davidson
@ 2017-03-17 23:50   ` hpa
  2017-04-03 23:01     ` Matthias Kaehlcke
  0 siblings, 1 reply; 50+ messages in thread
From: hpa @ 2017-03-17 23:50 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid

On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
>Suppress clang warnings about potential unaliged accesses
>to members in packed structs. This gets rid of almost 10,000
>warnings about accesses to the ring 0 stack pointer in the TSS.
>
>Signed-off-by: Michael Davidson <md@google.com>
>---
> arch/x86/Makefile | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>index 894a8d18bf97..7f21703c475d 100644
>--- a/arch/x86/Makefile
>+++ b/arch/x86/Makefile
>@@ -128,6 +128,11 @@ endif
>         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> endif
> 
>+ifeq ($(cc-name),clang)
>+# Suppress clang warnings about potential unaligned accesses.
>+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
>+endif
>+
> ifdef CONFIG_X86_X32
> 	x32_ld_ok := $(call try-run,\
> 			/bin/echo -e '1: .quad 1b' | \

Why conditional on clang?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 19:05               ` Dmitry Vyukov
  2017-03-17 19:26                   ` Peter Zijlstra
@ 2017-03-18  0:41                 ` Fengguang Wu
  1 sibling, 0 replies; 50+ messages in thread
From: Fengguang Wu @ 2017-03-18  0:41 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Peter Zijlstra,
	Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Matthias Kaehlcke, x86, open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all

Hi Dmitry,
 
On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
>On Fri, Mar 17, 2017 at 7:57 PM, Borislav Petkov <bp@alien8.de> wrote:
>> On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
>>> This problem is more general and is not specific to clang. It equally
>>> applies to different versions of gcc, different arches and different
>>> configs (namely, anything else than what a developer used for
>>> testing).
>>
>> I guess. We do carry a bunch of gcc workarounds along with the cc-*
>> macros in scripts/Kbuild.include.
>>
>>> A known, reasonably well working solution to this problem is
>>> a system of try bots that test patches before commit with different
>>> compilers/configs/archs. We already have such system in the form of
>>> 0-day bots. It would be useful to extend it with clang as soon as
>>> kernel builds.
>>
>> Has someone actually already talked to Fengguang about it?
>
>+Fengguang

I've actually tried clang long time ago. It quickly fails the build
for vanilla kernel. So it really depends on when the various clang
build fix patches can be accepted into mainline kernel.

Thanks,
Fengguang

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 20:04             ` hpa
@ 2017-03-24 13:47               ` Dmitry Vyukov
  2017-03-24 14:09                 ` Peter Zijlstra
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Vyukov @ 2017-03-24 13:47 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Peter Zijlstra, Michael Davidson, Alexander Potapenko,
	Michal Marek, Thomas Gleixner, Ingo Molnar, Herbert Xu,
	David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid

On Fri, Mar 17, 2017 at 9:04 PM,  <hpa@zytor.com> wrote:
> On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>>On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
>>> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra
>><peterz@infradead.org> wrote:
>>> >
>>> > Be that as it may; what you construct above is disgusting. Surely
>>the
>>> > code can be refactored to not look like dog vomit?
>>> >
>>> > Also; its not immediately obvious conf->copies is 'small' and this
>>> > doesn't blow up the stack; I feel that deserves a comment
>>somewhere.
>>> >
>>>
>>> I agree that the code is horrible.
>>>
>>> It is, in fact, exactly the same solution that was used to remove
>>> variable length arrays in structs from several of the crypto drivers
>>a
>>> few years ago - see the definition of SHASH_DESC_ON_STACK() in
>>> "crypto/hash.h" - I did not, however, hide the horrors in a macro
>>> preferring to leave the implementation visible as a warning to
>>whoever
>>> might touch the code next.
>>>
>>> I believe that the actual stack usage is exactly the same as it was
>>previously.
>>>
>>> I can certainly wrap this  up in a macro and add comments with
>>> appropriately dire warnings in it if you feel that is both necessary
>>> and sufficient.
>>
>>We got away with ugly in the past, so we should get to do it again?
>
> Seriously, you should have taken the hack the first time that this needs to be fixed.  Just because this is a fairly uncommon construct in the kernel doesn't mean it is not in userspace.


There is a reason why it is fairly uncommon in kernel.
Initially it was used more widely, but then there was a decision to
drop all uses of this feature. Namely:
Linus: "We should definitely drop it. The feature is an abomination".
https://lkml.org/lkml/2013/9/23/500
I really don't understand why you cling onto this last use of the
feature. Having a single use of a compiler extension on an error path
of a non-mandatory driver does not look like a great idea to me. Let's
just kill it off outside of clang discussion.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-17 19:29                   ` Peter Zijlstra
@ 2017-03-24 13:50                     ` Dmitry Vyukov
  2017-03-24 14:10                       ` Peter Zijlstra
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Vyukov @ 2017-03-24 13:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 17, 2017 at 8:29 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 17, 2017 at 08:26:42PM +0100, Peter Zijlstra wrote:
>> On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
>> > You can also find some reasons in the Why section of LLVM-Linux project:
>> > http://llvm.linuxfoundation.org/index.php/Main_Page
>>
>> From that:
>>
>>  - LLVM/Clang is a fast moving project with many things fixed quickly
>>    and features added.
>>
>> So what's the deal with that 5 year old bug you want us to work around?
>>
>> Also, clang doesn't support asm cc flags output and a few other
>> extensions last time I checked.
>>
>
> Another great one:
>
>  - BSD License (some people prefer this license to the GPL)
>
> Seems a very weak argument to make when talking about the Linux Kernel
> which is very explicitly GPLv2 (and not later).

OK, I guess should not have referenced the llvm-linux page.
So here are reasons on our side that I am ready to vouch:

 - clang make it possible to implement KMSAN (dynamic detection of
uses of uninit memory)
 - better code coverage for fuzzing
 - why simpler and faster development (e.g. we can port our user-space
hardening technologies -- CFI and SafeStack)

Michael is on a different team and has own reasons to do this.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-24 13:47               ` Dmitry Vyukov
@ 2017-03-24 14:09                 ` Peter Zijlstra
  0 siblings, 0 replies; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-24 14:09 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: H. Peter Anvin, Michael Davidson, Alexander Potapenko,
	Michal Marek, Thomas Gleixner, Ingo Molnar, Herbert Xu,
	David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid

On Fri, Mar 24, 2017 at 02:47:15PM +0100, Dmitry Vyukov wrote:
> > Seriously, you should have taken the hack the first time that this
> > needs to be fixed.  Just because this is a fairly uncommon construct
> > in the kernel doesn't mean it is not in userspace.
> 
> There is a reason why it is fairly uncommon in kernel.

So first off; its not entirely clear that the code as it exists it
correct. From a cursory reading of it and surrounding code, there is no
actual upper limit on the variable. If I were stupid enough to make a
raid with 64 devices I'd get a huge on-stack structure.

Since you're touching it; you should check these things.

And secondly, refactor the code to not look like dog vomit. You can do
more than the absolute minimal patch to make it compile, I'm sure.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-24 13:50                     ` Dmitry Vyukov
@ 2017-03-24 14:10                       ` Peter Zijlstra
  2017-03-24 14:22                         ` Dmitry Vyukov
  0 siblings, 1 reply; 50+ messages in thread
From: Peter Zijlstra @ 2017-03-24 14:10 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 24, 2017 at 02:50:24PM +0100, Dmitry Vyukov wrote:
> OK, I guess should not have referenced the llvm-linux page.
> So here are reasons on our side that I am ready to vouch:
> 
>  - clang make it possible to implement KMSAN (dynamic detection of
> uses of uninit memory)

How does GCC make this impossible?

>  - better code coverage for fuzzing

How so? Why can't the same be achieved using GCC?

>  - why simpler and faster development (e.g. we can port our user-space
> hardening technologies -- CFI and SafeStack)

That's just because you've already implemented this in clang, right? So
less work for you. Not because its impossible.

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

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
  2017-03-24 14:10                       ` Peter Zijlstra
@ 2017-03-24 14:22                         ` Dmitry Vyukov
  0 siblings, 0 replies; 50+ messages in thread
From: Dmitry Vyukov @ 2017-03-24 14:22 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke, x86,
	open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all, Fengguang Wu

On Fri, Mar 24, 2017 at 3:10 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 24, 2017 at 02:50:24PM +0100, Dmitry Vyukov wrote:
>> OK, I guess should not have referenced the llvm-linux page.
>> So here are reasons on our side that I am ready to vouch:
>>
>>  - clang make it possible to implement KMSAN (dynamic detection of
>> uses of uninit memory)
>
> How does GCC make this impossible?

Too complex and too difficult to implement correctly on all corner
cases. All other sanitizers were ported to gcc very quickly, but msan
wasn't. Nobody is brave enough to even approach it.

>>  - better code coverage for fuzzing
>
> How so? Why can't the same be achieved using GCC?

Same reason.

>>  - why simpler and faster development (e.g. we can port our user-space
>> hardening technologies -- CFI and SafeStack)
>
> That's just because you've already implemented this in clang, right? So
> less work for you. Not because its impossible.

I am not saying that it's impossible. It would just require
unreasonable amount of time, and then perpetual maintenance to fix
corner cases and regressions.

For background: I implemented the current fuzzing coverage (KCOV) in
gcc, and user-space tsan instrumentation in gcc.

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

* Re: [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
  2017-03-17  0:15 ` [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS Michael Davidson
@ 2017-04-03 22:49   ` Matthias Kaehlcke
  2017-04-21  7:49     ` Masahiro Yamada
  1 sibling, 0 replies; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-04-03 22:49 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid

El Thu, Mar 16, 2017 at 05:15:14PM -0700 Michael Davidson ha dit:

> Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
> for clang.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b841fb36beb2..b21fd0ca2946 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
>  # See modpost pattern 2
>  KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
>  KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
>  else
>  
>  # These warnings generated too much noise in a regular build.

Ping, any feedback on this patch?

Cheers

Matthias

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

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-03-17 23:50   ` hpa
@ 2017-04-03 23:01     ` Matthias Kaehlcke
  2017-04-13 23:14       ` Matthias Kaehlcke
  0 siblings, 1 reply; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-04-03 23:01 UTC (permalink / raw)
  To: hpa
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid

El Fri, Mar 17, 2017 at 04:50:19PM -0700 hpa@zytor.com ha dit:

> On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
> >Suppress clang warnings about potential unaliged accesses
> >to members in packed structs. This gets rid of almost 10,000
> >warnings about accesses to the ring 0 stack pointer in the TSS.
> >
> >Signed-off-by: Michael Davidson <md@google.com>
> >---
> > arch/x86/Makefile | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> >diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >index 894a8d18bf97..7f21703c475d 100644
> >--- a/arch/x86/Makefile
> >+++ b/arch/x86/Makefile
> >@@ -128,6 +128,11 @@ endif
> >         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > endif
> > 
> >+ifeq ($(cc-name),clang)
> >+# Suppress clang warnings about potential unaligned accesses.
> >+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> >+endif
> >+
> > ifdef CONFIG_X86_X32
> > 	x32_ld_ok := $(call try-run,\
> > 			/bin/echo -e '1: .quad 1b' | \
> 
> Why conditional on clang?

My understanding is that this warning is clang specific, it is not
listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

Cheers

Matthias

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

* Re: [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting
  2017-03-17  0:15 ` [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting Michael Davidson
@ 2017-04-03 23:14   ` Matthias Kaehlcke
  0 siblings, 0 replies; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-04-03 23:14 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Herbert Xu,
	David S. Miller, Alexander Potapenko, Dmitry Vyukov, x86,
	linux-kernel, linux-crypto

El Thu, Mar 16, 2017 at 05:15:20PM -0700 Michael Davidson ha dit:

> aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting
> of character sequences that are not valid preprocessor tokens.
> While this is allowed when preprocessing assembler files it exposes
> an incompatibilty between the clang and gcc preprocessors where
> clang does not strip leading white space from macro parameters,
> leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting
> in a token with a space character embedded in it.
> 
> While this could be fixed by deleting the offending space character,
> the assembler is perfectly capable of handling the token pasting
> correctly for itself so it seems preferable to let it do so and to
> get rid or the CONCAT(), DDQ() and XMM() preprocessor macros.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> index a916c4a61165..5f6a5af9c489 100644
> --- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> +++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> @@ -65,7 +65,6 @@
>  #include <linux/linkage.h>
>  #include <asm/inst.h>
>  
> -#define CONCAT(a,b)	a##b
>  #define VMOVDQ		vmovdqu
>  
>  #define xdata0		%xmm0
> @@ -92,8 +91,6 @@
>  #define num_bytes	%r8
>  
>  #define tmp		%r10
> -#define	DDQ(i)		CONCAT(ddq_add_,i)
> -#define	XMM(i)		CONCAT(%xmm, i)
>  #define	DDQ_DATA	0
>  #define	XDATA		1
>  #define KEY_128		1
> @@ -131,12 +128,12 @@ ddq_add_8:
>  /* generate a unique variable for ddq_add_x */
>  
>  .macro setddq n
> -	var_ddq_add = DDQ(\n)
> +	var_ddq_add = ddq_add_\n
>  .endm
>  
>  /* generate a unique variable for xmm register */
>  .macro setxdata n
> -	var_xdata = XMM(\n)
> +	var_xdata = %xmm\n
>  .endm
>  
>  /* club the numeric 'id' to the symbol 'name' */

Any feedback on this patch?

Thanks

Matthias

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

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-03-17  0:15 ` [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags Michael Davidson
  2017-03-17 21:32   ` H. Peter Anvin
@ 2017-04-05 18:08   ` Masahiro Yamada
  2017-04-05 19:01     ` Matthias Kaehlcke
  1 sibling, 1 reply; 50+ messages in thread
From: Masahiro Yamada @ 2017-04-05 18:08 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Matthias Kaehlcke, X86 ML,
	Linux Kbuild mailing list, Linux Kernel Mailing List,
	linux-crypto, linux-raid

Hi Michael,

2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
> Unfortunately, while clang generates a warning about these flags
> being unsupported it still exits with a status of 0 so we have
> to explicitly disable them instead of just using a cc-option check.
>
> Signed-off-by: Michael Davidson <md@google.com>


Instead, does the following work for you?
https://patchwork.kernel.org/patch/9657285/


You need to use
$(call cc-option, ...)
for -falign-jumps=1 and -falign-loops=1



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-04-05 18:08   ` Masahiro Yamada
@ 2017-04-05 19:01     ` Matthias Kaehlcke
  2017-04-05 19:11       ` Michael Davidson
  0 siblings, 1 reply; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-04-05 19:01 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Alexander Potapenko, Dmitry Vyukov, X86 ML,
	Linux Kbuild mailing list, Linux Kernel Mailing List,
	linux-crypto, linux-raid

Hi Masahiro,

El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit:

> 2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
> > Unfortunately, while clang generates a warning about these flags
> > being unsupported it still exits with a status of 0 so we have
> > to explicitly disable them instead of just using a cc-option check.
> >
> > Signed-off-by: Michael Davidson <md@google.com>
> 
> 
> Instead, does the following work for you?
> https://patchwork.kernel.org/patch/9657285/

Thanks for the pointer, I was about to give this change (or rather its
ancestor) a rework myself :)

> You need to use
> $(call cc-option, ...)
> for -falign-jumps=1 and -falign-loops=1

I can confirm that this works.

Thanks

Matthias

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

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-04-05 19:01     ` Matthias Kaehlcke
@ 2017-04-05 19:11       ` Michael Davidson
  2017-04-10 14:54         ` Masahiro Yamada
  0 siblings, 1 reply; 50+ messages in thread
From: Michael Davidson @ 2017-04-05 19:11 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Masahiro Yamada, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Alexander Potapenko, Dmitry Vyukov, X86 ML,
	Linux Kbuild mailing list, Linux Kernel Mailing List,
	linux-crypto, linux-raid

It "works" for the cases that I currently care about but I have to say
that I am uneasy about adding -Werror to the cc-option test in this
way.

Suppose that one of the *other* flags that is implicitly passed to the
compiler by cc-option - eg something that was explicitly specified in
$(KBUILD_CFLAGS) - triggers a warning. In that case all calls to
cc-option will silently fail because of the -Werror and valid options
will not be detected correctly.

If everyone is OK with that because "it shouldn't normally ever
happen" then that is fine, but if does result in a subtle change from
existing behavior (and a trap that I almost immediately fell into
after applying a similar patch).

On Wed, Apr 5, 2017 at 12:01 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi Masahiro,
>
> El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit:
>
>> 2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
>> > Unfortunately, while clang generates a warning about these flags
>> > being unsupported it still exits with a status of 0 so we have
>> > to explicitly disable them instead of just using a cc-option check.
>> >
>> > Signed-off-by: Michael Davidson <md@google.com>
>>
>>
>> Instead, does the following work for you?
>> https://patchwork.kernel.org/patch/9657285/
>
> Thanks for the pointer, I was about to give this change (or rather its
> ancestor) a rework myself :)
>
>> You need to use
>> $(call cc-option, ...)
>> for -falign-jumps=1 and -falign-loops=1
>
> I can confirm that this works.
>
> Thanks
>
> Matthias

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

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
  2017-04-05 19:11       ` Michael Davidson
@ 2017-04-10 14:54         ` Masahiro Yamada
  0 siblings, 0 replies; 50+ messages in thread
From: Masahiro Yamada @ 2017-04-10 14:54 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Matthias Kaehlcke, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Alexander Potapenko, Dmitry Vyukov, X86 ML,
	Linux Kbuild mailing list, Linux Kernel Mailing List,
	linux-crypto, linux-raid, Arnd Bergmann

Hi.


2017-04-06 4:11 GMT+09:00 Michael Davidson <md@google.com>:
> It "works" for the cases that I currently care about but I have to say
> that I am uneasy about adding -Werror to the cc-option test in this
> way.
>
> Suppose that one of the *other* flags that is implicitly passed to the
> compiler by cc-option - eg something that was explicitly specified in
> $(KBUILD_CFLAGS) - triggers a warning. In that case all calls to
> cc-option will silently fail because of the -Werror and valid options
> will not be detected correctly.

Theoretically, options explicitly specified in KBUILD_CFLAGS
should be always valid.
Options that may not be supported in some cases
should be wrapped with $(call cc-option ).



> If everyone is OK with that because "it shouldn't normally ever
> happen" then that is fine, but if does result in a subtle change from
> existing behavior (and a trap that I almost immediately fell into
> after applying a similar patch).

There is a rare case where a particular combination fails
(such as the conflict between -pg and -ffunction-sections
as reported in https://patchwork.kernel.org/patch/9624573/).

In a such case, we may end up with swapping the order,
but this should not happen quite often.



> On Wed, Apr 5, 2017 at 12:01 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
>> Hi Masahiro,
>>
>> El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit:
>>
>>> 2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
>>> > Unfortunately, while clang generates a warning about these flags
>>> > being unsupported it still exits with a status of 0 so we have
>>> > to explicitly disable them instead of just using a cc-option check.
>>> >
>>> > Signed-off-by: Michael Davidson <md@google.com>
>>>
>>>
>>> Instead, does the following work for you?
>>> https://patchwork.kernel.org/patch/9657285/
>>
>> Thanks for the pointer, I was about to give this change (or rather its
>> ancestor) a rework myself :)
>>
>>> You need to use
>>> $(call cc-option, ...)
>>> for -falign-jumps=1 and -falign-loops=1
>>
>> I can confirm that this works.
>>
>> Thanks
>>
>> Matthias
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-04-03 23:01     ` Matthias Kaehlcke
@ 2017-04-13 23:14       ` Matthias Kaehlcke
  2017-04-13 23:55         ` H. Peter Anvin
  0 siblings, 1 reply; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-04-13 23:14 UTC (permalink / raw)
  To: hpa
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid

El Mon, Apr 03, 2017 at 04:01:58PM -0700 Matthias Kaehlcke ha dit:

> El Fri, Mar 17, 2017 at 04:50:19PM -0700 hpa@zytor.com ha dit:
> 
> > On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
> > >Suppress clang warnings about potential unaliged accesses
> > >to members in packed structs. This gets rid of almost 10,000
> > >warnings about accesses to the ring 0 stack pointer in the TSS.
> > >
> > >Signed-off-by: Michael Davidson <md@google.com>
> > >---
> > > arch/x86/Makefile | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > >diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > >index 894a8d18bf97..7f21703c475d 100644
> > >--- a/arch/x86/Makefile
> > >+++ b/arch/x86/Makefile
> > >@@ -128,6 +128,11 @@ endif
> > >         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > > endif
> > > 
> > >+ifeq ($(cc-name),clang)
> > >+# Suppress clang warnings about potential unaligned accesses.
> > >+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> > >+endif
> > >+
> > > ifdef CONFIG_X86_X32
> > > 	x32_ld_ok := $(call try-run,\
> > > 			/bin/echo -e '1: .quad 1b' | \
> > 
> > Why conditional on clang?
> 
> My understanding is that this warning is clang specific, it is not
> listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

Actually this warning affects other platforms besides x86
(e.g. arm64), I'll submit a patch that disables the warning on all
platforms.

Cheers

Matthias

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

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-04-13 23:14       ` Matthias Kaehlcke
@ 2017-04-13 23:55         ` H. Peter Anvin
  2017-04-14  0:23           ` Matthias Kaehlcke
  0 siblings, 1 reply; 50+ messages in thread
From: H. Peter Anvin @ 2017-04-13 23:55 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid

On 04/13/17 16:14, Matthias Kaehlcke wrote:
> El Mon, Apr 03, 2017 at 04:01:58PM -0700 Matthias Kaehlcke ha dit:
> 
>> El Fri, Mar 17, 2017 at 04:50:19PM -0700 hpa@zytor.com ha dit:
>>
>>> On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
>>>> Suppress clang warnings about potential unaliged accesses
>>>> to members in packed structs. This gets rid of almost 10,000
>>>> warnings about accesses to the ring 0 stack pointer in the TSS.
>>>>
>>>> Signed-off-by: Michael Davidson <md@google.com>
>>>> ---
>>>> arch/x86/Makefile | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>>>> index 894a8d18bf97..7f21703c475d 100644
>>>> --- a/arch/x86/Makefile
>>>> +++ b/arch/x86/Makefile
>>>> @@ -128,6 +128,11 @@ endif
>>>>         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
>>>> endif
>>>>
>>>> +ifeq ($(cc-name),clang)
>>>> +# Suppress clang warnings about potential unaligned accesses.
>>>> +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
>>>> +endif
>>>> +
>>>> ifdef CONFIG_X86_X32
>>>> 	x32_ld_ok := $(call try-run,\
>>>> 			/bin/echo -e '1: .quad 1b' | \
>>>
>>> Why conditional on clang?
>>
>> My understanding is that this warning is clang specific, it is not
>> listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
> 
> Actually this warning affects other platforms besides x86
> (e.g. arm64), I'll submit a patch that disables the warning on all
> platforms.
> 

Drop the ifeq ($(cc-name),clang).

You should assume that if you have to add one of those ifeq's then you
are doing something fundamentally wrong.

	-hpa



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

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-04-13 23:55         ` H. Peter Anvin
@ 2017-04-14  0:23           ` Matthias Kaehlcke
  2017-04-14  5:30             ` hpa
  0 siblings, 1 reply; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-04-14  0:23 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Masahiro Yamada, x86, linux-kbuild, linux-kernel

El Thu, Apr 13, 2017 at 04:55:00PM -0700 H. Peter Anvin ha dit:

> On 04/13/17 16:14, Matthias Kaehlcke wrote:
> > El Mon, Apr 03, 2017 at 04:01:58PM -0700 Matthias Kaehlcke ha dit:
> > 
> >> El Fri, Mar 17, 2017 at 04:50:19PM -0700 hpa@zytor.com ha dit:
> >>
> >>> On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
> >>>> Suppress clang warnings about potential unaliged accesses
> >>>> to members in packed structs. This gets rid of almost 10,000
> >>>> warnings about accesses to the ring 0 stack pointer in the TSS.
> >>>>
> >>>> Signed-off-by: Michael Davidson <md@google.com>
> >>>> ---
> >>>> arch/x86/Makefile | 5 +++++
> >>>> 1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >>>> index 894a8d18bf97..7f21703c475d 100644
> >>>> --- a/arch/x86/Makefile
> >>>> +++ b/arch/x86/Makefile
> >>>> @@ -128,6 +128,11 @@ endif
> >>>>         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> >>>> endif
> >>>>
> >>>> +ifeq ($(cc-name),clang)
> >>>> +# Suppress clang warnings about potential unaligned accesses.
> >>>> +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> >>>> +endif
> >>>> +
> >>>> ifdef CONFIG_X86_X32
> >>>> 	x32_ld_ok := $(call try-run,\
> >>>> 			/bin/echo -e '1: .quad 1b' | \
> >>>
> >>> Why conditional on clang?
> >>
> >> My understanding is that this warning is clang specific, it is not
> >> listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
> > 
> > Actually this warning affects other platforms besides x86
> > (e.g. arm64), I'll submit a patch that disables the warning on all
> > platforms.
> > 
> 
> Drop the ifeq ($(cc-name),clang).
> 
> You should assume that if you have to add one of those ifeq's then you
> are doing something fundamentally wrong.

Thanks, however in the case of the global Makefile it seems we should
put it inside the already existing clang section:

http://lxr.free-electrons.com/source/Makefile#L692

Cheers

Matthias

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

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
  2017-04-14  0:23           ` Matthias Kaehlcke
@ 2017-04-14  5:30             ` hpa
  0 siblings, 0 replies; 50+ messages in thread
From: hpa @ 2017-04-14  5:30 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Masahiro Yamada, x86, linux-kbuild, linux-kernel

On April 13, 2017 5:23:35 PM PDT, Matthias Kaehlcke <mka@chromium.org> wrote:
>El Thu, Apr 13, 2017 at 04:55:00PM -0700 H. Peter Anvin ha dit:
>
>> On 04/13/17 16:14, Matthias Kaehlcke wrote:
>> > El Mon, Apr 03, 2017 at 04:01:58PM -0700 Matthias Kaehlcke ha dit:
>> > 
>> >> El Fri, Mar 17, 2017 at 04:50:19PM -0700 hpa@zytor.com ha dit:
>> >>
>> >>> On March 16, 2017 5:15:16 PM PDT, Michael Davidson
><md@google.com> wrote:
>> >>>> Suppress clang warnings about potential unaliged accesses
>> >>>> to members in packed structs. This gets rid of almost 10,000
>> >>>> warnings about accesses to the ring 0 stack pointer in the TSS.
>> >>>>
>> >>>> Signed-off-by: Michael Davidson <md@google.com>
>> >>>> ---
>> >>>> arch/x86/Makefile | 5 +++++
>> >>>> 1 file changed, 5 insertions(+)
>> >>>>
>> >>>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>> >>>> index 894a8d18bf97..7f21703c475d 100644
>> >>>> --- a/arch/x86/Makefile
>> >>>> +++ b/arch/x86/Makefile
>> >>>> @@ -128,6 +128,11 @@ endif
>> >>>>         KBUILD_CFLAGS += $(call
>cc-option,-maccumulate-outgoing-args)
>> >>>> endif
>> >>>>
>> >>>> +ifeq ($(cc-name),clang)
>> >>>> +# Suppress clang warnings about potential unaligned accesses.
>> >>>> +KBUILD_CFLAGS += $(call cc-disable-warning,
>address-of-packed-member)
>> >>>> +endif
>> >>>> +
>> >>>> ifdef CONFIG_X86_X32
>> >>>> 	x32_ld_ok := $(call try-run,\
>> >>>> 			/bin/echo -e '1: .quad 1b' | \
>> >>>
>> >>> Why conditional on clang?
>> >>
>> >> My understanding is that this warning is clang specific, it is not
>> >> listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
>> > 
>> > Actually this warning affects other platforms besides x86
>> > (e.g. arm64), I'll submit a patch that disables the warning on all
>> > platforms.
>> > 
>> 
>> Drop the ifeq ($(cc-name),clang).
>> 
>> You should assume that if you have to add one of those ifeq's then
>you
>> are doing something fundamentally wrong.
>
>Thanks, however in the case of the global Makefile it seems we should
>put it inside the already existing clang section:
>
>http://lxr.free-electrons.com/source/Makefile#L692
>
>Cheers
>
>Matthias

We shouldn't, unless it will actively break non-clang builds...
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
  2017-03-17  0:15 ` [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS Michael Davidson
@ 2017-04-21  7:49     ` Masahiro Yamada
  2017-04-21  7:49     ` Masahiro Yamada
  1 sibling, 0 replies; 50+ messages in thread
From: Masahiro Yamada @ 2017-04-21  7:49 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Matthias Kaehlcke, X86 ML,
	Linux Kbuild mailing list, Linux Kernel Mailing List,
	linux-crypto, linux-raid

Hi Michael,


2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
> Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
> for clang.

>From the code-diff, it is apparent that
you added -no-integrated-as.

Rather, I'd like to see "why" in the git-log.

Obviously, clang needs this patch to build the kernel,
but can you describe the reason why the integrated assembler is bad?

With git-log reworded, I will pick up this shortly.

Thanks!



> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index b841fb36beb2..b21fd0ca2946 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
>  # See modpost pattern 2
>  KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
>  KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
>  else
>
>  # These warnings generated too much noise in a regular build.







-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS
@ 2017-04-21  7:49     ` Masahiro Yamada
  0 siblings, 0 replies; 50+ messages in thread
From: Masahiro Yamada @ 2017-04-21  7:49 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
	Dmitry Vyukov, Matthias Kaehlcke, X86 ML,
	Linux Kbuild mailing list, Linux Kernel Mailing List,
	linux-crypto, linux-raid

Hi Michael,


2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
> Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
> for clang.

From the code-diff, it is apparent that
you added -no-integrated-as.

Rather, I'd like to see "why" in the git-log.

Obviously, clang needs this patch to build the kernel,
but can you describe the reason why the integrated assembler is bad?

With git-log reworded, I will pick up this shortly.

Thanks!



> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index b841fb36beb2..b21fd0ca2946 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
>  # See modpost pattern 2
>  KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
>  KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
>  else
>
>  # These warnings generated too much noise in a regular build.







-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c
  2017-03-17  0:15 ` [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c Michael Davidson
@ 2017-06-22 22:31   ` Matthias Kaehlcke
  2017-06-22 22:37     ` H. Peter Anvin
  0 siblings, 1 reply; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-06-22 22:31 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Alexander Potapenko, Dmitry Vyukov, x86, linux-kernel,
	Arnd Bergmann

(removed some non-x86 lists and folks from recipients)

El Thu, Mar 16, 2017 at 05:15:17PM -0700 Michael Davidson ha dit:

> undef memcpy and friends in boot/string.c so that the functions
> defined here will have the correct names, otherwise we end up
> up trying to redefine __builtin_memcpy etc.
> Surprisingly, gcc allows this (and, helpfully, discards the
> __builtin_ prefix from the function name when compiling it),
> but clang does not.
> 
> Adding these #undef's appears to preserve what I assume was
> the original intent of the code.

Any comments on this patch?

> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  arch/x86/boot/string.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
> index 5457b02fc050..b40266850869 100644
> --- a/arch/x86/boot/string.c
> +++ b/arch/x86/boot/string.c
> @@ -16,6 +16,15 @@
>  #include "ctype.h"
>  #include "string.h"
>  
> +/*
> + * Undef these macros so that the functions that we provide
> + * here will have the correct names regardless of how string.h
> + * may have chosen to #define them.
> + */
> +#undef memcpy
> +#undef memset
> +#undef memcmp
> +
>  int memcmp(const void *s1, const void *s2, size_t len)
>  {
>  	bool diff;

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

* Re: [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c
  2017-06-22 22:31   ` Matthias Kaehlcke
@ 2017-06-22 22:37     ` H. Peter Anvin
  2017-06-30 18:32       ` Matthias Kaehlcke
  0 siblings, 1 reply; 50+ messages in thread
From: H. Peter Anvin @ 2017-06-22 22:37 UTC (permalink / raw)
  To: Matthias Kaehlcke, Michael Davidson
  Cc: Thomas Gleixner, Ingo Molnar, Alexander Potapenko, Dmitry Vyukov,
	x86, linux-kernel, Arnd Bergmann

On 06/22/17 15:31, Matthias Kaehlcke wrote:
> (removed some non-x86 lists and folks from recipients)
> 
> El Thu, Mar 16, 2017 at 05:15:17PM -0700 Michael Davidson ha dit:
> 
>> undef memcpy and friends in boot/string.c so that the functions
>> defined here will have the correct names, otherwise we end up
>> up trying to redefine __builtin_memcpy etc.
>> Surprisingly, gcc allows this (and, helpfully, discards the
>> __builtin_ prefix from the function name when compiling it),
>> but clang does not.
>>
>> Adding these #undef's appears to preserve what I assume was
>> the original intent of the code.
> 
> Any comments on this patch?
> 
>> Signed-off-by: Michael Davidson <md@google.com>
>> ---
>>  arch/x86/boot/string.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
>> index 5457b02fc050..b40266850869 100644
>> --- a/arch/x86/boot/string.c
>> +++ b/arch/x86/boot/string.c
>> @@ -16,6 +16,15 @@
>>  #include "ctype.h"
>>  #include "string.h"
>>  
>> +/*
>> + * Undef these macros so that the functions that we provide
>> + * here will have the correct names regardless of how string.h
>> + * may have chosen to #define them.
>> + */
>> +#undef memcpy
>> +#undef memset
>> +#undef memcmp
>> +
>>  int memcmp(const void *s1, const void *s2, size_t len)
>>  {
>>  	bool diff;

Acked-by: H. Peter Anvin <hpa@zytor.com>

	-hpa

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

* Re: [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset
  2017-03-17 12:08   ` Peter Zijlstra
@ 2017-06-22 22:38     ` H. Peter Anvin
  0 siblings, 0 replies; 50+ messages in thread
From: H. Peter Anvin @ 2017-06-22 22:38 UTC (permalink / raw)
  To: Peter Zijlstra, Michael Davidson
  Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, Herbert Xu,
	David S. Miller, Shaohua Li, Alexander Potapenko, Dmitry Vyukov,
	Matthias Kaehlcke, x86, linux-kbuild, linux-kernel, linux-crypto,
	linux-raid

On 03/17/17 05:08, Peter Zijlstra wrote:
> On Thu, Mar 16, 2017 at 05:15:18PM -0700, Michael Davidson wrote:
>> Use the standard regparm=0 calling convention for memcpy and
>> memset when building with clang.
>>
>> This is a work around for a long standing clang bug
>> (see https://llvm.org/bugs/show_bug.cgi?id=3997) where
>> clang always uses the standard regparm=0 calling convention
>> for any implcit calls to memcpy and memset that it generates
>> (eg for structure assignments and initialization) even if an
>> alternate calling convention such as regparm=3 has been specified.
> 
> Seriously, fix LLVM already.
> 

Yes, this is a real stinker, in no small part because once clang is
fixed to DTRT then this is actually broken...

	-hpa


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

* Re: [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c
  2017-06-22 22:37     ` H. Peter Anvin
@ 2017-06-30 18:32       ` Matthias Kaehlcke
  0 siblings, 0 replies; 50+ messages in thread
From: Matthias Kaehlcke @ 2017-06-30 18:32 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Michael Davidson, Thomas Gleixner, Ingo Molnar,
	Alexander Potapenko, Dmitry Vyukov, x86, linux-kernel,
	Arnd Bergmann

El Thu, Jun 22, 2017 at 03:37:23PM -0700 H. Peter Anvin ha dit:

> On 06/22/17 15:31, Matthias Kaehlcke wrote:
> > (removed some non-x86 lists and folks from recipients)
> > 
> > El Thu, Mar 16, 2017 at 05:15:17PM -0700 Michael Davidson ha dit:
> > 
> >> undef memcpy and friends in boot/string.c so that the functions
> >> defined here will have the correct names, otherwise we end up
> >> up trying to redefine __builtin_memcpy etc.
> >> Surprisingly, gcc allows this (and, helpfully, discards the
> >> __builtin_ prefix from the function name when compiling it),
> >> but clang does not.
> >>
> >> Adding these #undef's appears to preserve what I assume was
> >> the original intent of the code.
> > 
> > Any comments on this patch?
> > 
> >> Signed-off-by: Michael Davidson <md@google.com>
> >> ---
> >>  arch/x86/boot/string.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
> >> index 5457b02fc050..b40266850869 100644
> >> --- a/arch/x86/boot/string.c
> >> +++ b/arch/x86/boot/string.c
> >> @@ -16,6 +16,15 @@
> >>  #include "ctype.h"
> >>  #include "string.h"
> >>  
> >> +/*
> >> + * Undef these macros so that the functions that we provide
> >> + * here will have the correct names regardless of how string.h
> >> + * may have chosen to #define them.
> >> + */
> >> +#undef memcpy
> >> +#undef memset
> >> +#undef memcmp
> >> +
> >>  int memcmp(const void *s1, const void *s2, size_t len)
> >>  {
> >>  	bool diff;
> 
> Acked-by: H. Peter Anvin <hpa@zytor.com>

Ingo, do you plan to pick this change?

Thanks

Matthias

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

end of thread, other threads:[~2017-06-30 18:32 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17  0:15 [PATCH 0/7] LLVM: make x86_64 kernel build with clang Michael Davidson
2017-03-17  0:15 ` [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS Michael Davidson
2017-04-03 22:49   ` Matthias Kaehlcke
2017-04-21  7:49   ` Masahiro Yamada
2017-04-21  7:49     ` Masahiro Yamada
2017-03-17  0:15 ` [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags Michael Davidson
2017-03-17 21:32   ` H. Peter Anvin
2017-03-17 21:34     ` H. Peter Anvin
2017-04-05 18:08   ` Masahiro Yamada
2017-04-05 19:01     ` Matthias Kaehlcke
2017-04-05 19:11       ` Michael Davidson
2017-04-10 14:54         ` Masahiro Yamada
2017-03-17  0:15 ` [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses Michael Davidson
2017-03-17 23:50   ` hpa
2017-04-03 23:01     ` Matthias Kaehlcke
2017-04-13 23:14       ` Matthias Kaehlcke
2017-04-13 23:55         ` H. Peter Anvin
2017-04-14  0:23           ` Matthias Kaehlcke
2017-04-14  5:30             ` hpa
2017-03-17  0:15 ` [PATCH 4/7] x86, boot, LLVM: #undef memcpy etc in string.c Michael Davidson
2017-06-22 22:31   ` Matthias Kaehlcke
2017-06-22 22:37     ` H. Peter Anvin
2017-06-30 18:32       ` Matthias Kaehlcke
2017-03-17  0:15 ` [PATCH 5/7] x86, boot, LLVM: Use regparm=0 for memcpy and memset Michael Davidson
2017-03-17 12:08   ` Peter Zijlstra
2017-06-22 22:38     ` H. Peter Anvin
2017-03-17  0:15 ` [PATCH 6/7] md/raid10, LLVM: get rid of variable length array Michael Davidson
2017-03-17 12:08   ` Peter Zijlstra
2017-03-17 12:31     ` Alexander Potapenko
2017-03-17 12:32       ` Alexander Potapenko
2017-03-17 18:03         ` Borislav Petkov
2017-03-17 18:47           ` Dmitry Vyukov
2017-03-17 18:57             ` Borislav Petkov
2017-03-17 19:05               ` Dmitry Vyukov
2017-03-17 19:26                 ` Peter Zijlstra
2017-03-17 19:26                   ` Peter Zijlstra
2017-03-17 19:29                   ` Peter Zijlstra
2017-03-24 13:50                     ` Dmitry Vyukov
2017-03-24 14:10                       ` Peter Zijlstra
2017-03-24 14:22                         ` Dmitry Vyukov
2017-03-18  0:41                 ` Fengguang Wu
2017-03-17 12:44       ` Peter Zijlstra
2017-03-17 18:52         ` Michael Davidson
2017-03-17 19:27           ` Peter Zijlstra
2017-03-17 20:04             ` hpa
2017-03-24 13:47               ` Dmitry Vyukov
2017-03-24 14:09                 ` Peter Zijlstra
2017-03-17  0:15 ` [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting Michael Davidson
2017-04-03 23:14   ` Matthias Kaehlcke
2017-03-17  8:17 ` [PATCH 0/7] LLVM: make x86_64 kernel build with clang Dmitry Vyukov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.