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,
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 */ );
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")))
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
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
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
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
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
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.
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
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
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
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:
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);
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
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.
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
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)
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
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
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
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");; \
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
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.
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"
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
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
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
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
linux-kselftest-mirror@lists.linaro.org