[PATCH 00/12] tools/nolibc: improve LLVM/clang support

[PATCH 00/12] tools/nolibc: improve LLVM/clang support

[Thomas Weißschuh]

The current support for LLVM and clang in nolibc and its testsuite is
very limited.

* Various architectures plain do not compile
* The user *has* to specify "-Os" otherwise the program crashes
* Cross-compilation of the tests does not work
* Using clang is not wired up in run-tests.sh

This series extends this support.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (12):
      tools/nolibc: use clang-compatible asm syntax in arch-arm.h
      tools/nolibc: limit powerpc stack-protector workaround to GCC
      tools/nolibc: move entrypoint specifics to compiler.h
      tools/nolibc: use attribute((naked)) if available
      selftests/nolibc: report failure if no testcase passed
      selftests/nolibc: avoid passing NULL to printf("%s")
      selftests/nolibc: determine $(srctree) first
      selftests/nolibc: setup objtree without Makefile.include
      selftests/nolibc: add support for LLVM= parameter
      selftests/nolibc: add cc-option compatible with clang cross builds
      selftests/nolibc: run-tests.sh: avoid overwriting CFLAGS_EXTRA
      selftests/nolibc: run-tests.sh: allow building through LLVM

 tools/include/nolibc/arch-aarch64.h          |  4 ++--
 tools/include/nolibc/arch-arm.h              |  8 ++++----
 tools/include/nolibc/arch-i386.h             |  4 ++--
 tools/include/nolibc/arch-loongarch.h        |  4 ++--
 tools/include/nolibc/arch-mips.h             |  4 ++--
 tools/include/nolibc/arch-powerpc.h          |  6 +++---
 tools/include/nolibc/arch-riscv.h            |  4 ++--
 tools/include/nolibc/arch-s390.h             |  4 ++--
 tools/include/nolibc/arch-x86_64.h           |  4 ++--
 tools/include/nolibc/compiler.h              | 12 ++++++++++++
 tools/testing/selftests/nolibc/Makefile      | 27 ++++++++++++++++-----------
 tools/testing/selftests/nolibc/nolibc-test.c |  4 ++--
 tools/testing/selftests/nolibc/run-tests.sh  | 20 ++++++++++++++++----
 13 files changed, 67 insertions(+), 38 deletions(-)
---
base-commit: 0db287736bc586fcd5a2925518ef09eec6924803
change-id: 20240727-nolibc-llvm-3fad68590d4c

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 01/12] tools/nolibc: use clang-compatible asm syntax in arch-arm.h

[Thomas Weißschuh]

The clang assembler rejects the current syntax.
Switch to a syntax accepted by both GCC and clang.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/arch-arm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index cae4afa7c1c7..d1c19d973e55 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -188,8 +188,8 @@
 void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
 {
 	__asm__ volatile (
-		"mov %r0, sp\n"         /* save stack pointer to %r0, as arg1 of _start_c */
-		"and ip, %r0, #-8\n"    /* sp must be 8-byte aligned in the callee        */
+		"mov r0, sp\n"          /* save stack pointer to %r0, as arg1 of _start_c */
+		"and ip, r0, #-8\n"     /* sp must be 8-byte aligned in the callee        */
 		"mov sp, ip\n"
 		"bl  _start_c\n"        /* transfer to c runtime                          */
 	);

-- 
2.45.2

[PATCH 02/12] tools/nolibc: limit powerpc stack-protector workaround to GCC

[Thomas Weißschuh]

As mentioned in the comment, the workaround for
__attribute__((no_stack_protector)) is only necessary on GCC.
Avoid applying the workaround on clang, as clang does not recognize
__attribute__((__optimize__)) and would fail.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/arch-powerpc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/include/nolibc/arch-powerpc.h b/tools/include/nolibc/arch-powerpc.h
index ac212e6185b2..41ebd394b90c 100644
--- a/tools/include/nolibc/arch-powerpc.h
+++ b/tools/include/nolibc/arch-powerpc.h
@@ -172,7 +172,7 @@
 	_ret;                                                                \
 })
 
-#ifndef __powerpc64__
+#if !defined(__powerpc64__) && !defined(__clang__)
 /* FIXME: For 32-bit PowerPC, with newer gcc compilers (e.g. gcc 13.1.0),
  * "omit-frame-pointer" fails with __attribute__((no_stack_protector)) but
  * works with __attribute__((__optimize__("-fno-stack-protector")))

-- 
2.45.2

[PATCH 03/12] tools/nolibc: move entrypoint specifics to compiler.h

[Thomas Weißschuh]

The specific attributes for the _start entrypoint are duplicated for
each architecture.
Deduplicate it into a dedicated #define into compiler.h.
This make the code shorter and will make it easier to adapt for clang
compatibility.

For clang compatibility, the epilogue will also need to be adapted, so
move that one, too.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/arch-aarch64.h   | 4 ++--
 tools/include/nolibc/arch-arm.h       | 4 ++--
 tools/include/nolibc/arch-i386.h      | 4 ++--
 tools/include/nolibc/arch-loongarch.h | 4 ++--
 tools/include/nolibc/arch-mips.h      | 4 ++--
 tools/include/nolibc/arch-powerpc.h   | 4 ++--
 tools/include/nolibc/arch-riscv.h     | 4 ++--
 tools/include/nolibc/arch-s390.h      | 4 ++--
 tools/include/nolibc/arch-x86_64.h    | 4 ++--
 tools/include/nolibc/compiler.h       | 3 +++
 10 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index b23ac1f04035..d5b182115664 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -142,13 +142,13 @@
 })
 
 /* startup code */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		"mov x0, sp\n"          /* save stack pointer to x0, as arg1 of _start_c */
 		"and sp, x0, -16\n"     /* sp must be 16-byte aligned in the callee      */
 		"bl  _start_c\n"        /* transfer to c runtime                         */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 #endif /* _NOLIBC_ARCH_AARCH64_H */
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index d1c19d973e55..8e8a681a0740 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -185,7 +185,7 @@
 })
 
 /* startup code */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		"mov r0, sp\n"          /* save stack pointer to %r0, as arg1 of _start_c */
@@ -193,7 +193,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		"mov sp, ip\n"
 		"bl  _start_c\n"        /* transfer to c runtime                          */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #endif /* _NOLIBC_ARCH_ARM_H */
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index 28c26a00a762..a464e3ae17fd 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -162,7 +162,7 @@
  * 2) The deepest stack frame should be set to zero
  *
  */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		"xor  %ebp, %ebp\n"       /* zero the stack frame                                */
@@ -174,7 +174,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		"call _start_c\n"         /* transfer to c runtime                               */
 		"hlt\n"                   /* ensure it does not return                           */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #endif /* _NOLIBC_ARCH_I386_H */
diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
index 3f8ef8f86c0f..b3edb10be4c7 100644
--- a/tools/include/nolibc/arch-loongarch.h
+++ b/tools/include/nolibc/arch-loongarch.h
@@ -149,14 +149,14 @@
 #endif
 
 /* startup code */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		"move          $a0, $sp\n"         /* save stack pointer to $a0, as arg1 of _start_c */
 		LONG_BSTRINS " $sp, $zero, 3, 0\n" /* $sp must be 16-byte aligned                    */
 		"bl            _start_c\n"         /* transfer to c runtime                          */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #endif /* _NOLIBC_ARCH_LOONGARCH_H */
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index 62cc50ef3288..afa5adc8c76b 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -179,7 +179,7 @@
 })
 
 /* startup code, note that it's called __start on MIPS */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector __start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector __start(void)
 {
 	__asm__ volatile (
 		".set push\n"
@@ -198,7 +198,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		" nop\n"                 /* delayed slot                                   */
 		".set pop\n"
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #endif /* _NOLIBC_ARCH_MIPS_H */
diff --git a/tools/include/nolibc/arch-powerpc.h b/tools/include/nolibc/arch-powerpc.h
index 41ebd394b90c..c0a2d08e4e51 100644
--- a/tools/include/nolibc/arch-powerpc.h
+++ b/tools/include/nolibc/arch-powerpc.h
@@ -184,7 +184,7 @@
 #endif /* !__powerpc64__ */
 
 /* startup code */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 #ifdef __powerpc64__
 #if _CALL_ELF == 2
@@ -215,7 +215,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		"bl     _start_c\n"     /* transfer to c runtime                         */
 	);
 #endif
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #endif /* _NOLIBC_ARCH_POWERPC_H */
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index 1927c643c739..90da3b328f6f 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -140,7 +140,7 @@
 })
 
 /* startup code */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		".option push\n"
@@ -151,7 +151,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		"andi sp, a0, -16\n"      /* sp must be 16-byte aligned                    */
 		"call _start_c\n"         /* transfer to c runtime                         */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #endif /* _NOLIBC_ARCH_RISCV_H */
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index 5d60fd43f883..09123861a140 100644
--- a/tools/include/nolibc/arch-s390.h
+++ b/tools/include/nolibc/arch-s390.h
@@ -139,7 +139,7 @@
 })
 
 /* startup code */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		"lgr	%r2, %r15\n"          /* save stack pointer to %r2, as arg1 of _start_c */
@@ -147,7 +147,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		"xc	0(8,%r15), 0(%r15)\n" /* clear backchain                                */
 		"brasl	%r14, _start_c\n"     /* transfer to c runtime                          */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 struct s390_mmap_arg_struct {
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index 68609f421934..98cb693cc38c 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -161,7 +161,7 @@
  * 2) The deepest stack frame should be zero (the %rbp).
  *
  */
-void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn)) __entrypoint __no_stack_protector _start(void)
 {
 	__asm__ volatile (
 		"xor  %ebp, %ebp\n"       /* zero the stack frame                            */
@@ -170,7 +170,7 @@ void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_
 		"call _start_c\n"         /* transfer to c runtime                           */
 		"hlt\n"                   /* ensure it does not return                       */
 	);
-	__builtin_unreachable();
+	__entrypoint_epilogue();
 }
 
 #define NOLIBC_ARCH_HAS_MEMMOVE
diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
index beddc3665d69..fe3701863634 100644
--- a/tools/include/nolibc/compiler.h
+++ b/tools/include/nolibc/compiler.h
@@ -6,6 +6,9 @@
 #ifndef _NOLIBC_COMPILER_H
 #define _NOLIBC_COMPILER_H
 
+#define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
+#define __entrypoint_epilogue() __builtin_unreachable()
+
 #if defined(__SSP__) || defined(__SSP_STRONG__) || defined(__SSP_ALL__) || defined(__SSP_EXPLICIT__)
 
 #define _NOLIBC_STACKPROTECTOR

-- 
2.45.2

[PATCH 04/12] tools/nolibc: use attribute((naked)) if available

[Thomas Weißschuh]

The current entrypoint attributes optimize("Os", "omit-frame-pointer")
are intended to avoid all compiler generated code, like function
porologue and epilogue.
This is the exact usecase implemented by the attribute "naked".

Unfortunately this is not implemented by GCC for all targets,
so only use it where available.
This also provides compatibility with clang, which recognizes the
"naked" attribute but not the previously used attribute "optimized".

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/compiler.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
index fe3701863634..f77bb7d3e1a8 100644
--- a/tools/include/nolibc/compiler.h
+++ b/tools/include/nolibc/compiler.h
@@ -9,6 +9,15 @@
 #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
 #define __entrypoint_epilogue() __builtin_unreachable()
 
+#if defined(__has_attribute)
+#  if __has_attribute(naked)
+#    undef  __entrypoint
+#    define __entrypoint __attribute__((naked))
+#    undef __entrypoint_epilogue
+#    define __entrypoint_epilogue()
+#  endif
+#endif /* defined(__has_attribute) */
+
 #if defined(__SSP__) || defined(__SSP_STRONG__) || defined(__SSP_ALL__) || defined(__SSP_EXPLICIT__)
 
 #define _NOLIBC_STACKPROTECTOR

-- 
2.45.2

[PATCH 05/12] selftests/nolibc: report failure if no testcase passed

[Thomas Weißschuh]

When nolibc-test is so broken, it doesn't even start,
don't report success.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 3fbabab46958..46dfbb50fae5 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -157,7 +157,7 @@ LDFLAGS :=
 
 REPORT  ?= awk '/\[OK\][\r]*$$/{p++} /\[FAIL\][\r]*$$/{if (!f) printf("\n"); f++; print;} /\[SKIPPED\][\r]*$$/{s++} \
 		END{ printf("\n%3d test(s): %3d passed, %3d skipped, %3d failed => status: ", p+s+f, p, s, f); \
-		if (f) printf("failure\n"); else if (s) printf("warning\n"); else printf("success\n");; \
+		if (f || !p) printf("failure\n"); else if (s) printf("warning\n"); else printf("success\n");; \
 		printf("\nSee all results in %s\n", ARGV[1]); }'
 
 help:

-- 
2.45.2

[PATCH 06/12] selftests/nolibc: avoid passing NULL to printf("%s")

[Thomas Weißschuh]

Clang on higher optimization levels detects that NULL is passed to
printf("%s") and warns about it.
Avoid the warning.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 093d0512f4c5..8cbb51dca0cd 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -542,7 +542,7 @@ int expect_strzr(const char *expr, int llen)
 {
 	int ret = 0;
 
-	llen += printf(" = <%s> ", expr);
+	llen += printf(" = <%s> ", expr ? expr : "(null)");
 	if (expr) {
 		ret = 1;
 		result(llen, FAIL);
@@ -561,7 +561,7 @@ int expect_strnz(const char *expr, int llen)
 {
 	int ret = 0;
 
-	llen += printf(" = <%s> ", expr);
+	llen += printf(" = <%s> ", expr ? expr : "(null)");
 	if (!expr) {
 		ret = 1;
 		result(llen, FAIL);

-- 
2.45.2

[PATCH 07/12] selftests/nolibc: determine $(srctree) first

[Thomas Weißschuh]

Avoid needing relative includes.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/Makefile | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 46dfbb50fae5..803a4e1bbe24 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -1,9 +1,14 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for nolibc tests
-include ../../../scripts/Makefile.include
-include ../../../scripts/utilities.mak
+# we're in ".../tools/testing/selftests/nolibc"
+ifeq ($(srctree),)
+srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
+endif
+
+include $(srctree)/tools/scripts/Makefile.include
+include $(srctree)/tools/scripts/utilities.mak
 # We need this for the "cc-option" macro.
-include ../../../build/Build.include
+include $(srctree)/tools/build/Build.include
 
 ifneq ($(O),)
 ifneq ($(call is-absolute,$(O)),y)
@@ -11,11 +16,6 @@ $(error Only absolute O= parameters are supported)
 endif
 endif
 
-# we're in ".../tools/testing/selftests/nolibc"
-ifeq ($(srctree),)
-srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
-endif
-
 ifeq ($(ARCH),)
 include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)

-- 
2.45.2

[PATCH 08/12] selftests/nolibc: setup objtree without Makefile.include

[Thomas Weißschuh]

Makefile.include has multiple uses. In addition to the setup of various
variables based on "O=", for which it is used currently, it can also set
up variables based on "LLVM=".
Unfortunately using it for both at the same time would require a big
ugly reshuffling of the nolibc Makefile.
As we want to use its llvm handling in the future, reimplement its
objtree := $(O).

While at it, also move "$(objtree) ?= $(srctree)" for consistency.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 803a4e1bbe24..8000bc3c408b 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -5,7 +5,6 @@ ifeq ($(srctree),)
 srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
 endif
 
-include $(srctree)/tools/scripts/Makefile.include
 include $(srctree)/tools/scripts/utilities.mak
 # We need this for the "cc-option" macro.
 include $(srctree)/tools/build/Build.include
@@ -14,6 +13,9 @@ ifneq ($(O),)
 ifneq ($(call is-absolute,$(O)),y)
 $(error Only absolute O= parameters are supported)
 endif
+objtree := $(O)
+else
+objtree ?= $(srctree)
 endif
 
 ifeq ($(ARCH),)
@@ -21,8 +23,6 @@ include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)
 endif
 
-objtree ?= $(srctree)
-
 # XARCH extends the kernel's ARCH with a few variants of the same
 # architecture that only differ by the configuration, the toolchain
 # and the Qemu program used. It is copied as-is into ARCH except for

-- 
2.45.2

[PATCH 09/12] selftests/nolibc: add support for LLVM= parameter

[Thomas Weißschuh]

Makefile.include can modify CC and CFLAGS for usage with clang.
Make use of it.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 8000bc3c408b..cdff317c35f2 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -155,6 +155,9 @@ CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wex
 		$(CFLAGS_$(XARCH)) $(CFLAGS_STACKPROTECTOR) $(CFLAGS_EXTRA)
 LDFLAGS :=
 
+# Modify CFLAGS based on LLVM=
+include $(srctree)/tools/scripts/Makefile.include
+
 REPORT  ?= awk '/\[OK\][\r]*$$/{p++} /\[FAIL\][\r]*$$/{if (!f) printf("\n"); f++; print;} /\[SKIPPED\][\r]*$$/{s++} \
 		END{ printf("\n%3d test(s): %3d passed, %3d skipped, %3d failed => status: ", p+s+f, p, s, f); \
 		if (f || !p) printf("failure\n"); else if (s) printf("warning\n"); else printf("success\n");; \

-- 
2.45.2

[PATCH 10/12] selftests/nolibc: add cc-option compatible with clang cross builds

[Thomas Weißschuh]

The cc-option macro from Build.include is not compatible with clang
cross builds, as it does not respect the "--target" and similar flags,
set up by Mekfile.include.

Provide a custom variant which works correctly.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index cdff317c35f2..b8577086e008 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -6,8 +6,8 @@ srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
 endif
 
 include $(srctree)/tools/scripts/utilities.mak
-# We need this for the "cc-option" macro.
-include $(srctree)/tools/build/Build.include
+# We need this for the "__cc-option" macro.
+include $(srctree)/scripts/Makefile.compiler
 
 ifneq ($(O),)
 ifneq ($(call is-absolute,$(O)),y)
@@ -23,6 +23,8 @@ include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)
 endif
 
+cc-option = $(call __cc-option, $(CC),$(CLANG_CROSS_FLAGS),$(1),$(2))
+
 # XARCH extends the kernel's ARCH with a few variants of the same
 # architecture that only differ by the configuration, the toolchain
 # and the Qemu program used. It is copied as-is into ARCH except for

-- 
2.45.2

[PATCH 11/12] selftests/nolibc: run-tests.sh: avoid overwriting CFLAGS_EXTRA

[Thomas Weißschuh]

If the user specified their own CFLAGS_EXTRA these should not be
overwritten by `-e`.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/run-tests.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh
index 0446e6326a40..324509b99e2c 100755
--- a/tools/testing/selftests/nolibc/run-tests.sh
+++ b/tools/testing/selftests/nolibc/run-tests.sh
@@ -15,7 +15,7 @@ download_location="${cache_dir}/crosstools/"
 build_location="$(realpath "${cache_dir}"/nolibc-tests/)"
 perform_download=0
 test_mode=system
-CFLAGS_EXTRA="-Werror"
+werror=1
 archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv s390 loongarch"
 
 TEMP=$(getopt -o 'j:d:c:b:a:m:peh' -n "$0" -- "$@")
@@ -69,7 +69,7 @@ while true; do
 			test_mode="$2"
 			shift 2; continue ;;
 		'-e')
-			CFLAGS_EXTRA=""
+			werror=0
 			shift; continue ;;
 		'-h')
 			print_usage
@@ -140,6 +140,9 @@ test_arch() {
 	ct_abi=$(crosstool_abi "$1")
 	cross_compile=$(realpath "${download_location}gcc-${crosstool_version}-nolibc/${ct_arch}-${ct_abi}/bin/${ct_arch}-${ct_abi}-")
 	build_dir="${build_location}/${arch}"
+	if [ "$werror" -ne 0 ]; then
+		CFLAGS_EXTRA="$CFLAGS_EXTRA -Werror"
+	fi
 	MAKE=(make -j"${nproc}" XARCH="${arch}" CROSS_COMPILE="${cross_compile}" O="${build_dir}")
 
 	mkdir -p "$build_dir"

-- 
2.45.2

[PATCH 12/12] selftests/nolibc: run-tests.sh: allow building through LLVM

[Thomas Weißschuh]

The nolibc tests can now be properly built with LLVM.
Expose this through run-tests.sh.
Not all architectures are compatible, add a list for those.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/run-tests.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/run-tests.sh b/tools/testing/selftests/nolibc/run-tests.sh
index 324509b99e2c..64e598ea1930 100755
--- a/tools/testing/selftests/nolibc/run-tests.sh
+++ b/tools/testing/selftests/nolibc/run-tests.sh
@@ -16,9 +16,10 @@ build_location="$(realpath "${cache_dir}"/nolibc-tests/)"
 perform_download=0
 test_mode=system
 werror=1
+llvm=
 archs="i386 x86_64 arm64 arm mips32le mips32be ppc ppc64 ppc64le riscv s390 loongarch"
 
-TEMP=$(getopt -o 'j:d:c:b:a:m:peh' -n "$0" -- "$@")
+TEMP=$(getopt -o 'j:d:c:b:a:m:pelh' -n "$0" -- "$@")
 
 eval set -- "$TEMP"
 unset TEMP
@@ -42,6 +43,7 @@ Options:
  -b [DIR]       Build location (default: ${build_location})
  -m [MODE]      Test mode user/system (default: ${test_mode})
  -e             Disable -Werror
+ -l             Build with LLVM/clang
 EOF
 }
 
@@ -71,6 +73,9 @@ while true; do
 		'-e')
 			werror=0
 			shift; continue ;;
+		'-l')
+			llvm=1
+			shift; continue ;;
 		'-h')
 			print_usage
 			exit 0
@@ -84,6 +89,10 @@ done
 
 if [[ -n "$*" ]]; then
 	archs="$*"
+elif [[ "${llvm}" -eq 1 ]]; then
+	for broken in mips32le mips32be ppc64le s390; do
+		archs="${archs//$broken }"
+	done
 fi
 
 crosstool_arch() {
@@ -143,7 +152,7 @@ test_arch() {
 	if [ "$werror" -ne 0 ]; then
 		CFLAGS_EXTRA="$CFLAGS_EXTRA -Werror"
 	fi
-	MAKE=(make -j"${nproc}" XARCH="${arch}" CROSS_COMPILE="${cross_compile}" O="${build_dir}")
+	MAKE=(make -j"${nproc}" XARCH="${arch}" CROSS_COMPILE="${cross_compile}" LLVM="${llvm}" O="${build_dir}")
 
 	mkdir -p "$build_dir"
 	if [ "$test_mode" = "system" ] && [ ! -f "${build_dir}/.config" ]; then

-- 
2.45.2

Re: [PATCH 00/12] tools/nolibc: improve LLVM/clang support

[Shuah Khan]

On 7/28/24 04:09, Thomas Weißschuh wrote:
> The current support for LLVM and clang in nolibc and its testsuite is
> very limited.
> 
> * Various architectures plain do not compile
> * The user *has* to specify "-Os" otherwise the program crashes
> * Cross-compilation of the tests does not work
> * Using clang is not wired up in run-tests.sh
> 
> This series extends this support.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Thomas Weißschuh (12):
>        tools/nolibc: use clang-compatible asm syntax in arch-arm.h
>        tools/nolibc: limit powerpc stack-protector workaround to GCC
>        tools/nolibc: move entrypoint specifics to compiler.h
>        tools/nolibc: use attribute((naked)) if available
>        selftests/nolibc: report failure if no testcase passed
>        selftests/nolibc: avoid passing NULL to printf("%s")
>        selftests/nolibc: determine $(srctree) first
>        selftests/nolibc: setup objtree without Makefile.include
>        selftests/nolibc: add support for LLVM= parameter
>        selftests/nolibc: add cc-option compatible with clang cross builds
>        selftests/nolibc: run-tests.sh: avoid overwriting CFLAGS_EXTRA
>        selftests/nolibc: run-tests.sh: allow building through LLVM
> 

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH 00/12] tools/nolibc: improve LLVM/clang support

[Shuah Khan]

On 7/29/24 13:26, Shuah Khan wrote:
> On 7/28/24 04:09, Thomas Weißschuh wrote:
>> The current support for LLVM and clang in nolibc and its testsuite is
>> very limited.
>>
>> * Various architectures plain do not compile
>> * The user *has* to specify "-Os" otherwise the program crashes
>> * Cross-compilation of the tests does not work
>> * Using clang is not wired up in run-tests.sh
>>
>> This series extends this support.
>>
>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>> ---
>> Thomas Weißschuh (12):
>>        tools/nolibc: use clang-compatible asm syntax in arch-arm.h
>>        tools/nolibc: limit powerpc stack-protector workaround to GCC
>>        tools/nolibc: move entrypoint specifics to compiler.h
>>        tools/nolibc: use attribute((naked)) if available
>>        selftests/nolibc: report failure if no testcase passed
>>        selftests/nolibc: avoid passing NULL to printf("%s")
>>        selftests/nolibc: determine $(srctree) first
>>        selftests/nolibc: setup objtree without Makefile.include
>>        selftests/nolibc: add support for LLVM= parameter
>>        selftests/nolibc: add cc-option compatible with clang cross builds
>>        selftests/nolibc: run-tests.sh: avoid overwriting CFLAGS_EXTRA
>>        selftests/nolibc: run-tests.sh: allow building through LLVM
>>
> 
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

For the selftest changes.

thanks,
-- Shuah

Re: [PATCH 03/12] tools/nolibc: move entrypoint specifics to compiler.h

[Willy Tarreau]

Hi Thomas,

On Sun, Jul 28, 2024 at 12:09:57PM +0200, Thomas Weißschuh wrote:
> The specific attributes for the _start entrypoint are duplicated for
> each architecture.
> Deduplicate it into a dedicated #define into compiler.h.
> This make the code shorter and will make it easier to adapt for clang
> compatibility.
> 
> For clang compatibility, the epilogue will also need to be adapted, so
> move that one, too.

I'm fine with the general approach, however I think that if we start to
add specific attributes and macros like this, we should prefix them with
"nolibc" to make sure they won't collide with userland.

Thanks,
willy

Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

[Willy Tarreau]

On Sun, Jul 28, 2024 at 12:09:58PM +0200, Thomas Weißschuh wrote:
> The current entrypoint attributes optimize("Os", "omit-frame-pointer")
> are intended to avoid all compiler generated code, like function
> porologue and epilogue.
> This is the exact usecase implemented by the attribute "naked".
> 
> Unfortunately this is not implemented by GCC for all targets,
> so only use it where available.
> This also provides compatibility with clang, which recognizes the
> "naked" attribute but not the previously used attribute "optimized".
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  tools/include/nolibc/compiler.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
> index fe3701863634..f77bb7d3e1a8 100644
> --- a/tools/include/nolibc/compiler.h
> +++ b/tools/include/nolibc/compiler.h
> @@ -9,6 +9,15 @@
>  #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
>  #define __entrypoint_epilogue() __builtin_unreachable()
>  
> +#if defined(__has_attribute)
> +#  if __has_attribute(naked)
> +#    undef  __entrypoint
> +#    define __entrypoint __attribute__((naked))
> +#    undef __entrypoint_epilogue
> +#    define __entrypoint_epilogue()
> +#  endif
> +#endif /* defined(__has_attribute) */

I would find it cleaner to enclose the previous declaration with the #if
rather than #undef everything just after it has been defined. Also it's
not very common to undo declarations just after they've been done, and
it makes quick code analysis harder.

I think that it can resolve to roughly this:

#if defined(__has_attribute) && __has_attribute(naked)
#  define __entrypoint __attribute__((naked))
#  define __entrypoint_epilogue()
#else
#  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
#  define __entrypoint_epilogue() __builtin_unreachable()
#endif
 
What do you think ?

Willy

Re: [PATCH 06/12] selftests/nolibc: avoid passing NULL to printf("%s")

[Willy Tarreau]

On Sun, Jul 28, 2024 at 12:10:00PM +0200, Thomas Weißschuh wrote:
> Clang on higher optimization levels detects that NULL is passed to
> printf("%s") and warns about it.
> Avoid the warning.

I don't see why this would be a problem, we do explicitly check for
NULL in our printf implementation and print "(null)". Or maybe it is
upset due to the attribute(printf) ? I don't know what the standard
says regarding %s and NULL, though. If it says that NULL is forbidden
then I can understand the warning and your fix is indeed correct. In
any case it's not worth fighting with a compiler for nolibc-test, but
it's probably worth mentioning in the commit message that it warns
despite the check being already done.

Willy

Re: [PATCH 07/12] selftests/nolibc: determine $(srctree) first

[Willy Tarreau]

On Sun, Jul 28, 2024 at 12:10:01PM +0200, Thomas Weißschuh wrote:
> Avoid needing relative includes.

I'm not opposed, but what's the benefit ? IMHO relative paths are
generally more flexible and robust. you could imagine a completely
made up example in which you have a symlink to selftests/nolibc in
your home dir, which works perfectly with relative paths when you
cd into it while it would not anymore with absolute paths (unless
you use cd -P).

Thus if we are decided to lose that flexibility at least it should
be argumented in the commit message.

Thanks,
Willy

Re: [PATCH 09/12] selftests/nolibc: add support for LLVM= parameter

[Willy Tarreau]

On Sun, Jul 28, 2024 at 12:10:03PM +0200, Thomas Weißschuh wrote:
> Makefile.include can modify CC and CFLAGS for usage with clang.
> Make use of it.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  tools/testing/selftests/nolibc/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 8000bc3c408b..cdff317c35f2 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -155,6 +155,9 @@ CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wex
>  		$(CFLAGS_$(XARCH)) $(CFLAGS_STACKPROTECTOR) $(CFLAGS_EXTRA)
>  LDFLAGS :=
>  
> +# Modify CFLAGS based on LLVM=
> +include $(srctree)/tools/scripts/Makefile.include

I'm confused, doesn't it precisely undo the previous patch, which
said that we ought not to include Makefile.include as it makes it
harder for LLVM ?

If so, I suspect that both commits should be squashed with a better
explanation for both operations at once (e.g. maybe "move makefile
inclusion later to benefit from LLVM=..." etc).

Thanks,
Willy

Re: [PATCH 00/12] tools/nolibc: improve LLVM/clang support

[Willy Tarreau]

On Sun, Jul 28, 2024 at 12:09:54PM +0200, Thomas Weißschuh wrote:
> The current support for LLVM and clang in nolibc and its testsuite is
> very limited.
> 
> * Various architectures plain do not compile
> * The user *has* to specify "-Os" otherwise the program crashes
> * Cross-compilation of the tests does not work
> * Using clang is not wired up in run-tests.sh
> 
> This series extends this support.

Overall Ack on the whole series once the minor comments are addressed
or discussed: Acked-by: Willy Tarreau <w@1wt.eu>

thanks!
Willy

Re: [PATCH 03/12] tools/nolibc: move entrypoint specifics to compiler.h

[Thomas Weißschuh]

Aug 3, 2024 11:22:23 Willy Tarreau <w@1wt.eu>:

> Hi Thomas,
>
> On Sun, Jul 28, 2024 at 12:09:57PM +0200, Thomas Weißschuh wrote:
>> The specific attributes for the _start entrypoint are duplicated for
>> each architecture.
>> Deduplicate it into a dedicated #define into compiler.h.
>> This make the code shorter and will make it easier to adapt for clang
>> compatibility.
>>
>> For clang compatibility, the epilogue will also need to be adapted, so
>> move that one, too.
>
> I'm fine with the general approach, however I think that if we start to
> add specific attributes and macros like this, we should prefix them with
> "nolibc" to make sure they won't collide with userland.

Ack.

FYI for v2 I intend to rename the macros to
__nolibc_naked, as I have a followup series that needs
them also for the non-entrypoint asm functions in
arch-x86_64.

> Thanks,
> willy

Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

[Thomas Weißschuh]

Aug 3, 2024 11:26:07 Willy Tarreau <w@1wt.eu>:

> On Sun, Jul 28, 2024 at 12:09:58PM +0200, Thomas Weißschuh wrote:
>> The current entrypoint attributes optimize("Os", "omit-frame-pointer")
>> are intended to avoid all compiler generated code, like function
>> porologue and epilogue.
>> This is the exact usecase implemented by the attribute "naked".
>>
>> Unfortunately this is not implemented by GCC for all targets,
>> so only use it where available.
>> This also provides compatibility with clang, which recognizes the
>> "naked" attribute but not the previously used attribute "optimized".
>>
>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>> ---
>> tools/include/nolibc/compiler.h | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
>> index fe3701863634..f77bb7d3e1a8 100644
>> --- a/tools/include/nolibc/compiler.h
>> +++ b/tools/include/nolibc/compiler.h
>> @@ -9,6 +9,15 @@
>> #define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
>> #define __entrypoint_epilogue() __builtin_unreachable()
>>
>> +#if defined(__has_attribute)
>> +#  if __has_attribute(naked)
>> +#    undef  __entrypoint
>> +#    define __entrypoint __attribute__((naked))
>> +#    undef __entrypoint_epilogue
>> +#    define __entrypoint_epilogue()
>> +#  endif
>> +#endif /* defined(__has_attribute) */
>
> I would find it cleaner to enclose the previous declaration with the #if
> rather than #undef everything just after it has been defined. Also it's
> not very common to undo declarations just after they've been done, and
> it makes quick code analysis harder.
>
> I think that it can resolve to roughly this:
>
> #if defined(__has_attribute) && __has_attribute(naked)
> #  define __entrypoint __attribute__((naked))
> #  define __entrypoint_epilogue()
> #else
> #  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
> #  define __entrypoint_epilogue() __builtin_unreachable()
> #endif

We would need to duplicate the define for the
!defined(__has_attribute) case.
I wanted to avoid that duplication.

> What do you think ?

With the reasoning above I'll let you choose.

Re: [PATCH 06/12] selftests/nolibc: avoid passing NULL to printf("%s")

[Thomas Weißschuh]

Aug 3, 2024 11:34:03 Willy Tarreau <w@1wt.eu>:

> On Sun, Jul 28, 2024 at 12:10:00PM +0200, Thomas Weißschuh wrote:
>> Clang on higher optimization levels detects that NULL is passed to
>> printf("%s") and warns about it.
>> Avoid the warning.
>
> I don't see why this would be a problem, we do explicitly check for
> NULL in our printf implementation and print "(null)". Or maybe it is
> upset due to the attribute(printf) ? I don't know what the standard
> says regarding %s and NULL, though. If it says that NULL is forbidden
> then I can understand the warning and your fix is indeed correct. In
> any case it's not worth fighting with a compiler for nolibc-test, but
> it's probably worth mentioning in the commit message that it warns
> despite the check being already done.

It's undefined as per POSIX.
I'll update the commit message.

Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

[Willy Tarreau]

On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh  wrote:
> > I think that it can resolve to roughly this:
> >
> > #if defined(__has_attribute) && __has_attribute(naked)
> > #  define __entrypoint __attribute__((naked))
> > #  define __entrypoint_epilogue()
> > #else
> > #  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
> > #  define __entrypoint_epilogue() __builtin_unreachable()
> > #endif
> 
> We would need to duplicate the define for the
> !defined(__has_attribute) case.

I don't understand why. Above both are tested on the first line.
Am I missing something ?

> I wanted to avoid that duplication.
> > What do you think ?
> 
> With the reasoning above I'll let you choose.

I'm fine with avoiding duplication, I just don't understand why there
should be.

Willy

Re: [PATCH 06/12] selftests/nolibc: avoid passing NULL to printf("%s")

[Willy Tarreau]

On Sat, Aug 03, 2024 at 08:29:14PM +0200, Thomas Weißschuh  wrote:
> Aug 3, 2024 11:34:03 Willy Tarreau <w@1wt.eu>:
> 
> > On Sun, Jul 28, 2024 at 12:10:00PM +0200, Thomas Weißschuh wrote:
> >> Clang on higher optimization levels detects that NULL is passed to
> >> printf("%s") and warns about it.
> >> Avoid the warning.
> >
> > I don't see why this would be a problem, we do explicitly check for
> > NULL in our printf implementation and print "(null)". Or maybe it is
> > upset due to the attribute(printf) ? I don't know what the standard
> > says regarding %s and NULL, though. If it says that NULL is forbidden
> > then I can understand the warning and your fix is indeed correct. In
> > any case it's not worth fighting with a compiler for nolibc-test, but
> > it's probably worth mentioning in the commit message that it warns
> > despite the check being already done.
> 
> It's undefined as per POSIX.
> I'll update the commit message.

OK, works for me!

Thanks,
Willy

Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

[Thomas Weißschuh]

Aug 3, 2024 20:33:11 Willy Tarreau <w@1wt.eu>:

> On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh  wrote:
>>> I think that it can resolve to roughly this:
>>>
>>> #if defined(__has_attribute) && __has_attribute(naked)
>>> #  define __entrypoint __attribute__((naked))
>>> #  define __entrypoint_epilogue()
>>> #else
>>> #  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
>>> #  define __entrypoint_epilogue() __builtin_unreachable()
>>> #endif
>>
>> We would need to duplicate the define for the
>> !defined(__has_attribute) case.
>
> I don't understand why. Above both are tested on the first line.
> Am I missing something ?

This specifically does not work [0]:

    a result, combining the two tests into a single expression as shown below would only be valid with a compiler that supports the operator but not with others that don’t.


>
>> I wanted to avoid that duplication.
>>> What do you think ?
>>
>> With the reasoning above I'll let you choose.
>
> I'm fine with avoiding duplication, I just don't understand why there
> should be.
>
> Willy

[0] https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fattribute.html

Re: [PATCH 04/12] tools/nolibc: use attribute((naked)) if available

[Willy Tarreau]

On Sat, Aug 03, 2024 at 10:55:07PM +0200, Thomas Weißschuh  wrote:
> Aug 3, 2024 20:33:11 Willy Tarreau <w@1wt.eu>:
> 
> > On Sat, Aug 03, 2024 at 08:28:08PM +0200, Thomas Weißschuh  wrote:
> >>> I think that it can resolve to roughly this:
> >>>
> >>> #if defined(__has_attribute) && __has_attribute(naked)
> >>> #  define __entrypoint __attribute__((naked))
> >>> #  define __entrypoint_epilogue()
> >>> #else
> >>> #  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
> >>> #  define __entrypoint_epilogue() __builtin_unreachable()
> >>> #endif
> >>
> >> We would need to duplicate the define for the
> >> !defined(__has_attribute) case.
> >
> > I don't understand why. Above both are tested on the first line.
> > Am I missing something ?
> 
> This specifically does not work [0]:
> 
> a result, combining the two tests into a single expression as shown
> below would only be valid with a compiler that supports the operator but not
> with others that don't.

Ah I didn't remember about that one, thanks for the reference. Indeed
it's annoying then.

We have a similar construct in compiler.h:

  #if defined(__has_attribute)
  #  if __has_attribute(no_stack_protector)
  #    define __no_stack_protector __attribute__((no_stack_protector))
  #  else
  #    define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
  #  endif
  #else
  #  define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
  #endif /* defined(__has_attribute) */

Maybe it would be a good opportunity to have our own macro so as to
simplify such tests:

  #if defined(__has_attribute)
  #  define nolibc_has_attribute(x) __has_attribute(x)
  #else
  #  define nolibc_has_attribute(x) 0
  #endif

  #if nolibc_has_attribute(no_stack_protector)
  #  define __no_stack_protector __attribute__((no_stack_protector))
  #else
  #  define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector"
  #endif

Then:

  #if nolibc_has_attribute(naked)
  #  define __entrypoint __attribute__((naked))
  #  define __entrypoint_epilogue()
  #else
  #  define __entrypoint __attribute__((optimize("Os", "omit-frame-pointer")))
  #  define __entrypoint_epilogue() __builtin_unreachable()
  #endif

It's as you want. Either we take your #undef-based solution or we take
this opportunity to clean up as above. I'm fine with both.

Thanks!
Willy

Re: [PATCH 07/12] selftests/nolibc: determine $(srctree) first

[Thomas Weißschuh]

On 2024-08-03 11:40:24+0000, Willy Tarreau wrote:
> On Sun, Jul 28, 2024 at 12:10:01PM +0200, Thomas Weißschuh wrote:
> > Avoid needing relative includes.
> 
> I'm not opposed, but what's the benefit ? IMHO relative paths are
> generally more flexible and robust. you could imagine a completely
> made up example in which you have a symlink to selftests/nolibc in
> your home dir, which works perfectly with relative paths when you
> cd into it while it would not anymore with absolute paths (unless
> you use cd -P).

This commit is solely about relative includes in the nolibc-test
Makefile.
The actual code is unaffected.

include ../../../scripts/utilities.mak
->
include $(srctree)/tools/scripts/utilities.mak

This commit is not necessary, just a cleanup.
IMO consistently using $(srctree) is nicer.

And yes the message for this commit is really not great.

> Thus if we are decided to lose that flexibility at least it should
> be argumented in the commit message.
> 
> Thanks,
> Willy

Re: [PATCH 09/12] selftests/nolibc: add support for LLVM= parameter

[Thomas Weißschuh]

On 2024-08-03 11:45:30+0000, Willy Tarreau wrote:
> On Sun, Jul 28, 2024 at 12:10:03PM +0200, Thomas Weißschuh wrote:
> > Makefile.include can modify CC and CFLAGS for usage with clang.
> > Make use of it.
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 8000bc3c408b..cdff317c35f2 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -155,6 +155,9 @@ CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wex
> >  		$(CFLAGS_$(XARCH)) $(CFLAGS_STACKPROTECTOR) $(CFLAGS_EXTRA)
> >  LDFLAGS :=
> >  
> > +# Modify CFLAGS based on LLVM=
> > +include $(srctree)/tools/scripts/Makefile.include
> 
> I'm confused, doesn't it precisely undo the previous patch, which
> said that we ought not to include Makefile.include as it makes it
> harder for LLVM ?

The previous inclusion doesn't make it harder.
The problem is that Makefile.include does two things
1) objtree setup and 2) LLVM handling.

For 1) we want to include it as early as possible, necessarily before
using $(objtree).
For 2) we need to include it after "CFLAGS ?=".
Reshuffling the Makefile to satisfy both requirements will look bad.
So the first commit removed the usage of Makefile.include for 1) because
that is easy to do and the second commit includes it later to satisfy
2).

> If so, I suspect that both commits should be squashed with a better
> explanation for both operations at once (e.g. maybe "move makefile
> inclusion later to benefit from LLVM=..." etc).

Ack.