Add x86-64 target for Clang+um and update user-offsets.c to use Clang-friendly assembler, similar to the fix from commit cf0c3e68aa81 ("kbuild: fix asm-offset generation to work with clang").
This lets me run KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py config --make_options LLVM=1 ... $ ./tools/testing/kunit/kunit.py run --make_options LLVM=1 ...
Cc: Jeff Dike jdike@addtoit.com Cc: Richard Weinberger richard@nod.at Cc: Anton Ivanov anton.ivanov@cambridgegreys.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: David Gow davidgow@google.com Cc: linux-um@lists.infradead.org Cc: linux-kbuild@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook keescook@chromium.org --- arch/x86/um/user-offsets.c | 4 ++-- scripts/Makefile.clang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c index bae61554abcc..d9071827b515 100644 --- a/arch/x86/um/user-offsets.c +++ b/arch/x86/um/user-offsets.c @@ -10,10 +10,10 @@ #include <asm/types.h>
#define DEFINE(sym, val) \ - asm volatile("\n->" #sym " %0 " #val : : "i" (val)) + asm volatile("\n.ascii "->" #sym " %0 " #val """: : "i" (val))
#define DEFINE_LONGS(sym, val) \ - asm volatile("\n->" #sym " %0 " #val : : "i" (val/sizeof(unsigned long))) + asm volatile("\n.ascii "->" #sym " %0 " #val """: : "i" (val/sizeof(unsigned long)))
void foo(void) { diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..857b23de51c6 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := x86_64-linux-gnu CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
On Wed, Feb 16, 2022 at 04:28:43PM -0800, Kees Cook wrote:
Add x86-64 target for Clang+um and update user-offsets.c to use Clang-friendly assembler, similar to the fix from commit cf0c3e68aa81
Clang-friendly assembly?
("kbuild: fix asm-offset generation to work with clang").
This lets me run KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py config --make_options LLVM=1 ... $ ./tools/testing/kunit/kunit.py run --make_options LLVM=1 ...
Cc: Jeff Dike jdike@addtoit.com Cc: Richard Weinberger richard@nod.at Cc: Anton Ivanov anton.ivanov@cambridgegreys.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: David Gow davidgow@google.com Cc: linux-um@lists.infradead.org Cc: linux-kbuild@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook keescook@chromium.org
I am not super familiar with UML but this seems reasonable.
Reviewed-by: Nathan Chancellor nathan@kernel.org
One small nit below if you have to send a v2, not sure it is worth it otherwise.
arch/x86/um/user-offsets.c | 4 ++-- scripts/Makefile.clang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c index bae61554abcc..d9071827b515 100644 --- a/arch/x86/um/user-offsets.c +++ b/arch/x86/um/user-offsets.c @@ -10,10 +10,10 @@ #include <asm/types.h> #define DEFINE(sym, val) \
- asm volatile("\n->" #sym " %0 " #val : : "i" (val))
- asm volatile("\n.ascii "->" #sym " %0 " #val """: : "i" (val))
#define DEFINE_LONGS(sym, val) \
- asm volatile("\n->" #sym " %0 " #val : : "i" (val/sizeof(unsigned long)))
- asm volatile("\n.ascii "->" #sym " %0 " #val """: : "i" (val/sizeof(unsigned long)))
void foo(void) { diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..857b23de51c6 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := x86_64-linux-gnu
It might be nice to keep this in alphabetical order.
CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH)) ifeq ($(CROSS_COMPILE),) -- 2.30.2
On Thu, Feb 17, 2022 at 8:28 AM Kees Cook keescook@chromium.org wrote:
Add x86-64 target for Clang+um and update user-offsets.c to use Clang-friendly assembler, similar to the fix from commit cf0c3e68aa81 ("kbuild: fix asm-offset generation to work with clang").
This lets me run KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py config --make_options LLVM=1 ... $ ./tools/testing/kunit/kunit.py run --make_options LLVM=1 ...
Cc: Jeff Dike jdike@addtoit.com Cc: Richard Weinberger richard@nod.at Cc: Anton Ivanov anton.ivanov@cambridgegreys.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: David Gow davidgow@google.com Cc: linux-um@lists.infradead.org Cc: linux-kbuild@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook keescook@chromium.org
Thanks, this worked fine for me, with one small note:
I get the following warning with clang (13.0.1) under UML (but not under x86_64): clang: warning: argument unused during compilation: '-mno-global-merge' [-Wunused-command-line-argument]
It's not really a problem unless -Werror is enabled, though, so this is still definitely an improvement over clang not working at all.
With that caveat, this is: Tested-by: David Gow davidgow@google.com
Cheers, -- David
arch/x86/um/user-offsets.c | 4 ++-- scripts/Makefile.clang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c index bae61554abcc..d9071827b515 100644 --- a/arch/x86/um/user-offsets.c +++ b/arch/x86/um/user-offsets.c @@ -10,10 +10,10 @@ #include <asm/types.h>
#define DEFINE(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val))#define DEFINE_LONGS(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val/sizeof(unsigned long)))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val/sizeof(unsigned long)))void foo(void) { diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..857b23de51c6 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := x86_64-linux-gnu CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
2.30.2
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220217002843.2312603-1-keescoo....
On Thu, Feb 17, 2022 at 9:28 AM Kees Cook keescook@chromium.org wrote:
Add x86-64 target for Clang+um and update user-offsets.c to use Clang-friendly assembler, similar to the fix from commit cf0c3e68aa81 ("kbuild: fix asm-offset generation to work with clang").
This lets me run KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py config --make_options LLVM=1 ... $ ./tools/testing/kunit/kunit.py run --make_options LLVM=1 ...
Cc: Jeff Dike jdike@addtoit.com Cc: Richard Weinberger richard@nod.at Cc: Anton Ivanov anton.ivanov@cambridgegreys.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: David Gow davidgow@google.com Cc: linux-um@lists.infradead.org Cc: linux-kbuild@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook keescook@chromium.org
arch/x86/um/user-offsets.c | 4 ++-- scripts/Makefile.clang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c index bae61554abcc..d9071827b515 100644 --- a/arch/x86/um/user-offsets.c +++ b/arch/x86/um/user-offsets.c @@ -10,10 +10,10 @@ #include <asm/types.h>
#define DEFINE(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val))#define DEFINE_LONGS(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val/sizeof(unsigned long)))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val/sizeof(unsigned long)))void foo(void) { diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..857b23de51c6 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := x86_64-linux-gnu
Does this work for the i386 host?
UML supports i386 and x86_64 as the host architecture as of now, but this always compiles UML for x86_64?
CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
2.30.2
On Thu, Feb 17, 2022 at 01:54:58PM +0900, Masahiro Yamada wrote:
On Thu, Feb 17, 2022 at 9:28 AM Kees Cook keescook@chromium.org wrote:
Add x86-64 target for Clang+um and update user-offsets.c to use Clang-friendly assembler, similar to the fix from commit cf0c3e68aa81 ("kbuild: fix asm-offset generation to work with clang").
This lets me run KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py config --make_options LLVM=1 ... $ ./tools/testing/kunit/kunit.py run --make_options LLVM=1 ...
Cc: Jeff Dike jdike@addtoit.com Cc: Richard Weinberger richard@nod.at Cc: Anton Ivanov anton.ivanov@cambridgegreys.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: David Gow davidgow@google.com Cc: linux-um@lists.infradead.org Cc: linux-kbuild@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook keescook@chromium.org
arch/x86/um/user-offsets.c | 4 ++-- scripts/Makefile.clang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c index bae61554abcc..d9071827b515 100644 --- a/arch/x86/um/user-offsets.c +++ b/arch/x86/um/user-offsets.c @@ -10,10 +10,10 @@ #include <asm/types.h>
#define DEFINE(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val))#define DEFINE_LONGS(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val/sizeof(unsigned long)))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val/sizeof(unsigned long)))void foo(void) { diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..857b23de51c6 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := x86_64-linux-gnu
Does this work for the i386 host?
UML supports i386 and x86_64 as the host architecture as of now, but this always compiles UML for x86_64?
I think the current code will work because arch/x86/Makefile.um includes -m32 for CONFIG_X86_32, which will implicitly change x86_64-linux-gnu into a 32-bit target triple:
$ echo | clang --target=x86_64-linux-gnu -x c -c -o test.o -
$ file test.o test.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
$ echo | clang --target=x86_64-linux-gnu -m32 -x c -c -o test.o -
$ file test.o test.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), not stripped
In fact, we rely on this for ARCH=i386 LLVM=1 right now, as it uses x86_64-linux-gnu for the target flag.
While UML only supports x86, maybe it is worth using SUBARCH instead of hardcoding the triple? No strong opinion around that though.
diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..87285b76adb2 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
2.30.2
-- Best Regards Masahiro Yamada
On Fri, Feb 18, 2022 at 2:41 AM Nathan Chancellor nathan@kernel.org wrote:
On Thu, Feb 17, 2022 at 01:54:58PM +0900, Masahiro Yamada wrote:
On Thu, Feb 17, 2022 at 9:28 AM Kees Cook keescook@chromium.org wrote:
Add x86-64 target for Clang+um and update user-offsets.c to use Clang-friendly assembler, similar to the fix from commit cf0c3e68aa81 ("kbuild: fix asm-offset generation to work with clang").
This lets me run KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py config --make_options LLVM=1 ... $ ./tools/testing/kunit/kunit.py run --make_options LLVM=1 ...
Cc: Jeff Dike jdike@addtoit.com Cc: Richard Weinberger richard@nod.at Cc: Anton Ivanov anton.ivanov@cambridgegreys.com Cc: Masahiro Yamada masahiroy@kernel.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: David Gow davidgow@google.com Cc: linux-um@lists.infradead.org Cc: linux-kbuild@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook keescook@chromium.org
arch/x86/um/user-offsets.c | 4 ++-- scripts/Makefile.clang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c index bae61554abcc..d9071827b515 100644 --- a/arch/x86/um/user-offsets.c +++ b/arch/x86/um/user-offsets.c @@ -10,10 +10,10 @@ #include <asm/types.h>
#define DEFINE(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val))#define DEFINE_LONGS(sym, val) \
asm volatile("\n->" #sym " %0 " #val : : "i" (val/sizeof(unsigned long)))
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"": : "i" (val/sizeof(unsigned long)))void foo(void) { diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..857b23de51c6 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := x86_64-linux-gnu
Does this work for the i386 host?
UML supports i386 and x86_64 as the host architecture as of now, but this always compiles UML for x86_64?
I think the current code will work because arch/x86/Makefile.um includes -m32 for CONFIG_X86_32, which will implicitly change x86_64-linux-gnu into a 32-bit target triple:
Ah, you are right!
$ echo | clang --target=x86_64-linux-gnu -x c -c -o test.o -
$ file test.o test.o: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
$ echo | clang --target=x86_64-linux-gnu -m32 -x c -c -o test.o -
$ file test.o test.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), not stripped
In fact, we rely on this for ARCH=i386 LLVM=1 right now, as it uses x86_64-linux-gnu for the target flag.
While UML only supports x86, maybe it is worth using SUBARCH instead of hardcoding the triple? No strong opinion around that though.
diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang index 51fc23e2e9e5..87285b76adb2 100644 --- a/scripts/Makefile.clang +++ b/scripts/Makefile.clang @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH))
LGTM.
I also thought of not passing --target at all for ARCH=um, but we decided to override --target all the time (for reproducibility?). Anyway, Nathan's way is clean, and looks OK to me.
CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
ifeq ($(CROSS_COMPILE),)
2.30.2
-- Best Regards Masahiro Yamada
-- Best Regards Masahiro Yamada
linux-kselftest-mirror@lists.linaro.org
-
David Gow -
Kees Cook -
Masahiro Yamada -
Nathan Chancellor