* [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
@ 2023-07-12 9:16 ` Zhangjin Wu
2023-07-15 8:59 ` Willy Tarreau
2023-07-12 9:17 ` [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c Zhangjin Wu
` (10 subsequent siblings)
11 siblings, 1 reply; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:16 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
__NR_statx has been added from v4.10:
commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")
It has been supported by all of the platforms since at least from v4.20
and glibc 2.28.
Let's remove the old arch related and dependent sys_stat support
completely.
This is friendly to the future new architecture porting.
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-aarch64.h | 28 -------------
tools/include/nolibc/arch-arm.h | 37 -----------------
tools/include/nolibc/arch-i386.h | 26 ------------
tools/include/nolibc/arch-mips.h | 28 -------------
tools/include/nolibc/arch-riscv.h | 23 -----------
tools/include/nolibc/arch-s390.h | 25 ------------
tools/include/nolibc/arch-x86_64.h | 27 -------------
tools/include/nolibc/sys.h | 63 +++++------------------------
tools/include/nolibc/types.h | 4 +-
9 files changed, 13 insertions(+), 248 deletions(-)
diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index 6227b77a4a09..1bf122cd5966 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -9,34 +9,6 @@
#include "compiler.h"
-/* The struct returned by the newfstatat() syscall. Differs slightly from the
- * x86_64's stat one by field ordering, so be careful.
- */
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned int st_mode;
- unsigned int st_nlink;
- unsigned int st_uid;
- unsigned int st_gid;
-
- unsigned long st_rdev;
- unsigned long __pad1;
- long st_size;
- int st_blksize;
- int __pad2;
-
- long st_blocks;
- long st_atime;
- unsigned long st_atime_nsec;
- long st_mtime;
-
- unsigned long st_mtime_nsec;
- long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned int __unused[2];
-};
-
/* Syscalls for AARCH64 :
* - registers are 64-bit
* - stack is 16-byte aligned
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index 4d4887a5f04b..ea723596ed23 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -9,43 +9,6 @@
#include "compiler.h"
-/* The struct returned by the stat() syscall, 32-bit only, the syscall returns
- * exactly 56 bytes (stops before the unused array). In big endian, the format
- * differs as devices are returned as short only.
- */
-struct sys_stat_struct {
-#if defined(__ARMEB__)
- unsigned short st_dev;
- unsigned short __pad1;
-#else
- unsigned long st_dev;
-#endif
- unsigned long st_ino;
- unsigned short st_mode;
- unsigned short st_nlink;
- unsigned short st_uid;
- unsigned short st_gid;
-
-#if defined(__ARMEB__)
- unsigned short st_rdev;
- unsigned short __pad2;
-#else
- unsigned long st_rdev;
-#endif
- unsigned long st_size;
- unsigned long st_blksize;
- unsigned long st_blocks;
-
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
- unsigned long st_mtime_nsec;
-
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned long __unused[2];
-};
-
/* Syscalls for ARM in ARM or Thumb modes :
* - registers are 32-bit
* - stack is 8-byte aligned
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index 4c6b7c04e2e7..1d4b683bc2cd 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -9,32 +9,6 @@
#include "compiler.h"
-/* The struct returned by the stat() syscall, 32-bit only, the syscall returns
- * exactly 56 bytes (stops before the unused array).
- */
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned short st_mode;
- unsigned short st_nlink;
- unsigned short st_uid;
- unsigned short st_gid;
-
- unsigned long st_rdev;
- unsigned long st_size;
- unsigned long st_blksize;
- unsigned long st_blocks;
-
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
- unsigned long st_mtime_nsec;
-
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned long __unused[2];
-};
-
/* Syscalls for i386 :
* - mostly similar to x86_64
* - registers are 32-bit
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index a2bfdf57b957..ae40d5268793 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -9,34 +9,6 @@
#include "compiler.h"
-/* The struct returned by the stat() syscall. 88 bytes are returned by the
- * syscall.
- */
-struct sys_stat_struct {
- unsigned int st_dev;
- long st_pad1[3];
- unsigned long st_ino;
- unsigned int st_mode;
- unsigned int st_nlink;
- unsigned int st_uid;
- unsigned int st_gid;
- unsigned int st_rdev;
- long st_pad2[2];
- long st_size;
- long st_pad3;
-
- long st_atime;
- long st_atime_nsec;
- long st_mtime;
- long st_mtime_nsec;
-
- long st_ctime;
- long st_ctime_nsec;
- long st_blksize;
- long st_blocks;
- long st_pad4[14];
-};
-
/* Syscalls for MIPS ABI O32 :
* - WARNING! there's always a delayed slot!
* - WARNING again, the syntax is different, registers take a '$' and numbers
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index cd958b2f4b1b..2b89ea59c5e4 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -9,29 +9,6 @@
#include "compiler.h"
-struct sys_stat_struct {
- unsigned long st_dev; /* Device. */
- unsigned long st_ino; /* File serial number. */
- unsigned int st_mode; /* File mode. */
- unsigned int st_nlink; /* Link count. */
- unsigned int st_uid; /* User ID of the file's owner. */
- unsigned int st_gid; /* Group ID of the file's group. */
- unsigned long st_rdev; /* Device number, if device. */
- unsigned long __pad1;
- long st_size; /* Size of file, in bytes. */
- int st_blksize; /* Optimal block size for I/O. */
- int __pad2;
- long st_blocks; /* Number 512-byte blocks allocated. */
- long st_atime; /* Time of last access. */
- unsigned long st_atime_nsec;
- long st_mtime; /* Time of last modification. */
- unsigned long st_mtime_nsec;
- long st_ctime; /* Time of last status change. */
- unsigned long st_ctime_nsec;
- unsigned int __unused4;
- unsigned int __unused5;
-};
-
#if __riscv_xlen == 64
#define PTRLOG "3"
#define SZREG "8"
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index a644ecd361c0..a40424ba043e 100644
--- a/tools/include/nolibc/arch-s390.h
+++ b/tools/include/nolibc/arch-s390.h
@@ -10,31 +10,6 @@
#include "compiler.h"
-/* The struct returned by the stat() syscall, equivalent to stat64(). The
- * syscall returns 116 bytes and stops in the middle of __unused.
- */
-
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned long st_nlink;
- unsigned int st_mode;
- unsigned int st_uid;
- unsigned int st_gid;
- unsigned int __pad1;
- unsigned long st_rdev;
- unsigned long st_size;
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
- unsigned long st_mtime_nsec;
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned long st_blksize;
- long st_blocks;
- unsigned long __unused[3];
-};
-
/* Syscalls for s390:
* - registers are 64-bit
* - syscall number is passed in r1
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index e69113742a99..e980ca5417d0 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -9,33 +9,6 @@
#include "compiler.h"
-/* The struct returned by the stat() syscall, equivalent to stat64(). The
- * syscall returns 116 bytes and stops in the middle of __unused.
- */
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned long st_nlink;
- unsigned int st_mode;
- unsigned int st_uid;
-
- unsigned int st_gid;
- unsigned int __pad0;
- unsigned long st_rdev;
- long st_size;
- long st_blksize;
-
- long st_blocks;
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
-
- unsigned long st_mtime_nsec;
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- long __unused[3];
-};
-
/* Syscalls for x86_64 :
* - registers are 64-bit
* - syscall number is passed in rax
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index dee56894a811..8bfe7db20b80 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -943,15 +943,19 @@ pid_t setsid(void)
return __sysret(sys_setsid());
}
-#if defined(__NR_statx)
/*
* int statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf);
+ * int stat(const char *path, struct stat *buf);
*/
static __attribute__((unused))
int sys_statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf)
{
+#ifdef __NR_statx
return my_syscall5(__NR_statx, fd, path, flags, mask, buf);
+#else
+ return -ENOSYS;
+#endif
}
static __attribute__((unused))
@@ -959,24 +963,18 @@ int statx(int fd, const char *path, int flags, unsigned int mask, struct statx *
{
return __sysret(sys_statx(fd, path, flags, mask, buf));
}
-#endif
-/*
- * int stat(const char *path, struct stat *buf);
- * Warning: the struct stat's layout is arch-dependent.
- */
-#if defined(__NR_statx) && !defined(__NR_newfstatat) && !defined(__NR_stat)
-/*
- * Maybe we can just use statx() when available for all architectures?
- */
static __attribute__((unused))
-int sys_stat(const char *path, struct stat *buf)
+int stat(const char *path, struct stat *buf)
{
struct statx statx;
long ret;
- ret = sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &statx);
+ ret = __sysret(sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &statx));
+ if (ret == -1)
+ return ret;
+
buf->st_dev = ((statx.stx_dev_minor & 0xff)
| (statx.stx_dev_major << 8)
| ((statx.stx_dev_minor & ~0xff) << 12));
@@ -997,47 +995,8 @@ int sys_stat(const char *path, struct stat *buf)
buf->st_mtim.tv_nsec = statx.stx_mtime.tv_nsec;
buf->st_ctim.tv_sec = statx.stx_ctime.tv_sec;
buf->st_ctim.tv_nsec = statx.stx_ctime.tv_nsec;
- return ret;
-}
-#else
-static __attribute__((unused))
-int sys_stat(const char *path, struct stat *buf)
-{
- struct sys_stat_struct stat;
- long ret;
-
-#ifdef __NR_newfstatat
- /* only solution for arm64 */
- ret = my_syscall4(__NR_newfstatat, AT_FDCWD, path, &stat, 0);
-#elif defined(__NR_stat)
- ret = my_syscall2(__NR_stat, path, &stat);
-#else
- return -ENOSYS;
-#endif
- buf->st_dev = stat.st_dev;
- buf->st_ino = stat.st_ino;
- buf->st_mode = stat.st_mode;
- buf->st_nlink = stat.st_nlink;
- buf->st_uid = stat.st_uid;
- buf->st_gid = stat.st_gid;
- buf->st_rdev = stat.st_rdev;
- buf->st_size = stat.st_size;
- buf->st_blksize = stat.st_blksize;
- buf->st_blocks = stat.st_blocks;
- buf->st_atim.tv_sec = stat.st_atime;
- buf->st_atim.tv_nsec = stat.st_atime_nsec;
- buf->st_mtim.tv_sec = stat.st_mtime;
- buf->st_mtim.tv_nsec = stat.st_mtime_nsec;
- buf->st_ctim.tv_sec = stat.st_ctime;
- buf->st_ctim.tv_nsec = stat.st_ctime_nsec;
- return ret;
-}
-#endif
-static __attribute__((unused))
-int stat(const char *path, struct stat *buf)
-{
- return __sysret(sys_stat(path, buf));
+ return 0;
}
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 23963e48d8ee..8cfc4c860fa4 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -15,8 +15,8 @@
/* Only the generic macros and types may be defined here. The arch-specific
- * ones such as the O_RDONLY and related macros used by fcntl() and open(), or
- * the layout of sys_stat_struct must not be defined here.
+ * ones such as the O_RDONLY and related macros used by fcntl() and open()
+ * must not be defined here.
*/
/* stat flags (WARNING, octal here). We need to check for an existing
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support
2023-07-12 9:16 ` [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support Zhangjin Wu
@ 2023-07-15 8:59 ` Willy Tarreau
2023-07-15 10:39 ` Zhangjin Wu
0 siblings, 1 reply; 27+ messages in thread
From: Willy Tarreau @ 2023-07-15 8:59 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: arnd, linux-kernel, linux-kselftest, thomas
Hi Zhangjin,
On Wed, Jul 12, 2023 at 05:16:34PM +0800, Zhangjin Wu wrote:
> __NR_statx has been added from v4.10:
>
> commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")
>
> It has been supported by all of the platforms since at least from v4.20
> and glibc 2.28.
>
> Let's remove the old arch related and dependent sys_stat support
> completely.
>
> This is friendly to the future new architecture porting.
As I previously said, I'd like that we at least preserve compatibility
with supported stable branches. 4.14 and 4.19 are still supported, so
does this mean that if I rebuild my preinit against the updated nolibc
it will fail to boot on such older systems for the archs that we support?
Because if it means that in order to support all currently active
kernels, one must only build from an older copy of the lib, that becomes
a disservice to its development and usage. Thus if you checked that aarch64,
arm, i386, mips, riscv, s390 and x86_64 had already adopted statx by 4.14,
then I'm fine and we can drop stat(), but then it must be mentioned in
the commit message, because here it's not explicit.
Thanks!
Willy
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support
2023-07-15 8:59 ` Willy Tarreau
@ 2023-07-15 10:39 ` Zhangjin Wu
2023-07-15 11:45 ` Willy Tarreau
0 siblings, 1 reply; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-15 10:39 UTC (permalink / raw)
To: w; +Cc: arnd, falcon, linux-kernel, linux-kselftest, thomas
> Hi Zhangjin,
>
> On Wed, Jul 12, 2023 at 05:16:34PM +0800, Zhangjin Wu wrote:
> > __NR_statx has been added from v4.10:
> >
> > commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")
> >
> > It has been supported by all of the platforms since at least from v4.20
> > and glibc 2.28.
> >
> > Let's remove the old arch related and dependent sys_stat support
> > completely.
> >
> > This is friendly to the future new architecture porting.
>
> As I previously said, I'd like that we at least preserve compatibility
> with supported stable branches. 4.14 and 4.19 are still supported, so
> does this mean that if I rebuild my preinit against the updated nolibc
> it will fail to boot on such older systems for the archs that we support?
>
> Because if it means that in order to support all currently active
> kernels, one must only build from an older copy of the lib, that becomes
> a disservice to its development and usage. Thus if you checked that aarch64,
> arm, i386, mips, riscv, s390 and x86_64 had already adopted statx by 4.14,
> then I'm fine and we can drop stat(), but then it must be mentioned in
> the commit message, because here it's not explicit.
>
Yeah, we used 'git blame' before and found the last related change is
v4.20, but it is not really the first change.
Just read the statx manpage again:
https://man7.org/linux/man-pages/man2/statx.2.html
It shows something about "Linux 4.11, glibc 2.28.
And 'git grep' shows it is true:
$ git grep -r statx v4.11 arch/ include/uapi/asm-generic/unistd.h | grep -E "aarch64|arm|mips|s390|x86|:include/uapi"
v4.11:arch/arm/tools/syscall.tbl:397 common statx sys_statx
v4.11:arch/arm64/include/asm/unistd32.h:#define __NR_statx 397
v4.11:arch/arm64/include/asm/unistd32.h:__SYSCALL(__NR_statx, sys_statx)
v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 366)
v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 326)
v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 330)
v4.11:arch/mips/kernel/scall32-o32.S: PTR sys_statx
v4.11:arch/mips/kernel/scall64-64.S: PTR sys_statx
v4.11:arch/mips/kernel/scall64-n32.S: PTR sys_statx /* 6330 */
v4.11:arch/mips/kernel/scall64-o32.S: PTR sys_statx
v4.11:arch/s390/include/uapi/asm/unistd.h:#define __NR_statx 379
v4.11:arch/s390/kernel/compat_wrapper.c:COMPAT_SYSCALL_WRAP5(statx, int, dfd, const char __user *, path, unsigned, flags, unsigned, mask, struct statx __user *, buffer);
v4.11:arch/s390/kernel/syscalls.S:SYSCALL(sys_statx,compat_sys_statx)
v4.11:arch/x86/entry/syscalls/syscall_32.tbl:383 i386 statx sys_statx
v4.11:arch/x86/entry/syscalls/syscall_64.tbl:332 common statx sys_statx
v4.11:include/uapi/asm-generic/unistd.h:#define __NR_statx 291
v4.11:include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_statx, sys_statx)
both riscv and loongarch use the generic unistd.h, so, all of our
supported archs should work as-is. riscv itself is added from v4.15,
loongarch itself is added from v5.19.
The powerpc we plan to support is from v4.11:
$ git grep -r statx v4.11 arch/powerpc/
v4.11:arch/powerpc/include/asm/systbl.h:SYSCALL(statx)
v4.11:arch/powerpc/include/uapi/asm/unistd.h:#define __NR_statx 383
So, let's simply correct the commit message (the old v4.10 has a wrong -1
offset, I wrongly used 'git show a528d35e8bfc:Makefile' before).
sys_statx has been supported from Linux v4.11 and glibc 2.28:
$ git grep -r statx v4.11 arch/ include/uapi/asm-generic/unistd.h
Both riscv (firstly added from v4.15) and loongarch (firstly added from
v5.19) use the generic unistd.h, the others are supported from arch/.
Let's remove the old arch related and dependent sys_stat support
completely.
Since the current oldest stable branch is v4.14, so, using statx
instead of sys_stat preserves compatibility with all of the
supported stable branches.
This is friendly to the future new architecture porting.
Best regards,
Zhangjin
> Thanks!
> Willy
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support
2023-07-15 10:39 ` Zhangjin Wu
@ 2023-07-15 11:45 ` Willy Tarreau
0 siblings, 0 replies; 27+ messages in thread
From: Willy Tarreau @ 2023-07-15 11:45 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: arnd, linux-kernel, linux-kselftest, thomas
On Sat, Jul 15, 2023 at 06:39:41PM +0800, Zhangjin Wu wrote:
> Just read the statx manpage again:
>
> https://man7.org/linux/man-pages/man2/statx.2.html
>
> It shows something about "Linux 4.11, glibc 2.28.
>
> And 'git grep' shows it is true:
>
> $ git grep -r statx v4.11 arch/ include/uapi/asm-generic/unistd.h | grep -E "aarch64|arm|mips|s390|x86|:include/uapi"
> v4.11:arch/arm/tools/syscall.tbl:397 common statx sys_statx
> v4.11:arch/arm64/include/asm/unistd32.h:#define __NR_statx 397
> v4.11:arch/arm64/include/asm/unistd32.h:__SYSCALL(__NR_statx, sys_statx)
> v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 366)
> v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 326)
> v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 330)
> v4.11:arch/mips/kernel/scall32-o32.S: PTR sys_statx
> v4.11:arch/mips/kernel/scall64-64.S: PTR sys_statx
> v4.11:arch/mips/kernel/scall64-n32.S: PTR sys_statx /* 6330 */
> v4.11:arch/mips/kernel/scall64-o32.S: PTR sys_statx
> v4.11:arch/s390/include/uapi/asm/unistd.h:#define __NR_statx 379
> v4.11:arch/s390/kernel/compat_wrapper.c:COMPAT_SYSCALL_WRAP5(statx, int, dfd, const char __user *, path, unsigned, flags, unsigned, mask, struct statx __user *, buffer);
> v4.11:arch/s390/kernel/syscalls.S:SYSCALL(sys_statx,compat_sys_statx)
> v4.11:arch/x86/entry/syscalls/syscall_32.tbl:383 i386 statx sys_statx
> v4.11:arch/x86/entry/syscalls/syscall_64.tbl:332 common statx sys_statx
> v4.11:include/uapi/asm-generic/unistd.h:#define __NR_statx 291
> v4.11:include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_statx, sys_statx)
>
> both riscv and loongarch use the generic unistd.h, so, all of our
> supported archs should work as-is. riscv itself is added from v4.15,
> loongarch itself is added from v5.19.
So that's perfect, thank you!
Willy
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
2023-07-12 9:16 ` [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support Zhangjin Wu
@ 2023-07-12 9:17 ` Zhangjin Wu
2023-07-12 20:39 ` Thomas Weißschuh
2023-07-12 9:18 ` [PATCH v3 03/11] tools/nolibc: arm: shrink _start " Zhangjin Wu
` (9 subsequent siblings)
11 siblings, 1 reply; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:17 UTC (permalink / raw)
To: w
Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas,
Thomas Weißschuh
As the environ and _auxv support added for nolibc, the assembly _start
function becomes more and more complex and therefore makes the porting
of nolibc to new architectures harder and harder.
To simplify portability, this C version of _start_c() is added to do
most of the assembly start operations in C, which reduces the complexity
a lot and will eventually simplify the porting of nolibc to the new
architectures.
The new _start_c() only requires a stack pointer argument, it will find
argv, envp and _auxv for us, and then call main(), finally, it exit()
with main's return status. With this new _start_c(), the future new
architectures only require to add very few assembly instructions.
As suggested by Thomas, users may use a different signature of main
(e.g. void main(void)), a _nolibc_main alias is added for main to
silence the warning about potential conflicting types.
Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
Link: https://lore.kernel.org/lkml/90fdd255-32f4-4caf-90ff-06456b53dac3@t-8ch.de/
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/Makefile | 1 +
tools/include/nolibc/crt.h | 59 +++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
create mode 100644 tools/include/nolibc/crt.h
diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index 64d67b080744..909b6eb500fe 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -27,6 +27,7 @@ nolibc_arch := $(patsubst arm64,aarch64,$(ARCH))
arch_file := arch-$(nolibc_arch).h
all_files := \
compiler.h \
+ crt.h \
ctype.h \
errno.h \
nolibc.h \
diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h
new file mode 100644
index 000000000000..f9db2389acd2
--- /dev/null
+++ b/tools/include/nolibc/crt.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+/*
+ * C Run Time support for NOLIBC
+ * Copyright (C) 2023 Zhangjin Wu <falcon@tinylab.org>
+ */
+
+#ifndef _NOLIBC_CRT_H
+#define _NOLIBC_CRT_H
+
+char **environ __attribute__((weak));
+const unsigned long *_auxv __attribute__((weak));
+
+typedef int (_nolibc_main_fn)(int, char **, char **);
+static void exit(int);
+
+void _start_c(long *sp)
+{
+ int argc, i;
+ char **argv;
+ char **envp;
+ /* silence potential warning: conflicting types for 'main' */
+ _nolibc_main_fn _nolibc_main __asm__ ("main");
+
+ /*
+ * sp : argc <-- argument count, required by main()
+ * argv: argv[0] <-- argument vector, required by main()
+ * argv[1]
+ * ...
+ * argv[argc-1]
+ * null
+ * envp: envp[0] <-- environment variables, required by main() and getenv()
+ * envp[1]
+ * ...
+ * null
+ * _auxv: auxv[0] <-- auxiliary vector, required by getauxval()
+ * auxv[1]
+ * ...
+ * null
+ */
+
+ /* assign argc and argv */
+ argc = sp[0];
+ argv = (void *)(sp + 1);
+
+ /* find envp */
+ envp = argv + argc + 1;
+ environ = envp;
+
+ /* find auxv */
+ i = 0;
+ while (envp[i])
+ i++;
+ _auxv = (void *)(envp + i + 1);
+
+ /* go to application */
+ exit(_nolibc_main(argc, argv, envp));
+}
+
+#endif /* _NOLIBC_CRT_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-12 9:17 ` [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c Zhangjin Wu
@ 2023-07-12 20:39 ` Thomas Weißschuh
2023-07-13 6:12 ` Zhangjin Wu
0 siblings, 1 reply; 27+ messages in thread
From: Thomas Weißschuh @ 2023-07-12 20:39 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: w, arnd, linux-kernel, linux-kselftest
On 2023-07-12 17:17:39+0800, Zhangjin Wu wrote:
> As the environ and _auxv support added for nolibc, the assembly _start
> function becomes more and more complex and therefore makes the porting
> of nolibc to new architectures harder and harder.
>
> To simplify portability, this C version of _start_c() is added to do
> most of the assembly start operations in C, which reduces the complexity
> a lot and will eventually simplify the porting of nolibc to the new
> architectures.
>
> The new _start_c() only requires a stack pointer argument, it will find
> argv, envp and _auxv for us, and then call main(), finally, it exit()
> with main's return status. With this new _start_c(), the future new
> architectures only require to add very few assembly instructions.
>
> As suggested by Thomas, users may use a different signature of main
> (e.g. void main(void)), a _nolibc_main alias is added for main to
> silence the warning about potential conflicting types.
>
> Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
> Link: https://lore.kernel.org/lkml/90fdd255-32f4-4caf-90ff-06456b53dac3@t-8ch.de/
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
> tools/include/nolibc/Makefile | 1 +
> tools/include/nolibc/crt.h | 59 +++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
> create mode 100644 tools/include/nolibc/crt.h
>
> diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
> index 64d67b080744..909b6eb500fe 100644
> --- a/tools/include/nolibc/Makefile
> +++ b/tools/include/nolibc/Makefile
> @@ -27,6 +27,7 @@ nolibc_arch := $(patsubst arm64,aarch64,$(ARCH))
> arch_file := arch-$(nolibc_arch).h
> all_files := \
> compiler.h \
> + crt.h \
> ctype.h \
> errno.h \
> nolibc.h \
> diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h
> new file mode 100644
> index 000000000000..f9db2389acd2
> --- /dev/null
> +++ b/tools/include/nolibc/crt.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
> +/*
> + * C Run Time support for NOLIBC
> + * Copyright (C) 2023 Zhangjin Wu <falcon@tinylab.org>
> + */
> +
> +#ifndef _NOLIBC_CRT_H
> +#define _NOLIBC_CRT_H
> +
> +char **environ __attribute__((weak));
> +const unsigned long *_auxv __attribute__((weak));
> +
> +typedef int (_nolibc_main_fn)(int, char **, char **);
What's the advantage of the typedef over using the pointer type inline?
> +static void exit(int);
> +
> +void _start_c(long *sp)
> +{
> + int argc, i;
> + char **argv;
> + char **envp;
> + /* silence potential warning: conflicting types for 'main' */
> + _nolibc_main_fn _nolibc_main __asm__ ("main");
What about the stackprotector initialization?
It would really fit great into this series.
> +
> + /*
> + * sp : argc <-- argument count, required by main()
> + * argv: argv[0] <-- argument vector, required by main()
> + * argv[1]
> + * ...
> + * argv[argc-1]
> + * null
> + * envp: envp[0] <-- environment variables, required by main() and getenv()
> + * envp[1]
> + * ...
> + * null
> + * _auxv: auxv[0] <-- auxiliary vector, required by getauxval()
> + * auxv[1]
> + * ...
> + * null
> + */
> +
> + /* assign argc and argv */
> + argc = sp[0];
> + argv = (void *)(sp + 1);
Bit of a weird mismatch between array syntax and pointer arithmetic.
> +
> + /* find envp */
> + envp = argv + argc + 1;
> + environ = envp;
Is envp really needed? Could just be assigned directly to environ.
> +
> + /* find auxv */
> + i = 0;
> + while (envp[i])
> + i++;
> + _auxv = (void *)(envp + i + 1);
Could be simplified a bit:
_auxv = (void *) envp;
while (_auxv)
_auxv++;
> +
> + /* go to application */
> + exit(_nolibc_main(argc, argv, envp));
> +}
> +
> +#endif /* _NOLIBC_CRT_H */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-12 20:39 ` Thomas Weißschuh
@ 2023-07-13 6:12 ` Zhangjin Wu
2023-07-13 6:23 ` Zhangjin Wu
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-13 6:12 UTC (permalink / raw)
To: thomas; +Cc: arnd, falcon, linux-kernel, linux-kselftest, w
Hi, Thomas
> On 2023-07-12 17:17:39+0800, Zhangjin Wu wrote:
> > As the environ and _auxv support added for nolibc, the assembly _start
> > function becomes more and more complex and therefore makes the porting
> > of nolibc to new architectures harder and harder.
> >
> > To simplify portability, this C version of _start_c() is added to do
> > most of the assembly start operations in C, which reduces the complexity
> > a lot and will eventually simplify the porting of nolibc to the new
> > architectures.
> >
> > The new _start_c() only requires a stack pointer argument, it will find
> > argv, envp and _auxv for us, and then call main(), finally, it exit()
> > with main's return status. With this new _start_c(), the future new
> > architectures only require to add very few assembly instructions.
> >
> > As suggested by Thomas, users may use a different signature of main
> > (e.g. void main(void)), a _nolibc_main alias is added for main to
> > silence the warning about potential conflicting types.
> >
> > Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
> > Link: https://lore.kernel.org/lkml/90fdd255-32f4-4caf-90ff-06456b53dac3@t-8ch.de/
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> > tools/include/nolibc/Makefile | 1 +
> > tools/include/nolibc/crt.h | 59 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 60 insertions(+)
> > create mode 100644 tools/include/nolibc/crt.h
> >
> > diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
> > index 64d67b080744..909b6eb500fe 100644
> > --- a/tools/include/nolibc/Makefile
> > +++ b/tools/include/nolibc/Makefile
> > @@ -27,6 +27,7 @@ nolibc_arch := $(patsubst arm64,aarch64,$(ARCH))
> > arch_file := arch-$(nolibc_arch).h
> > all_files := \
> > compiler.h \
> > + crt.h \
> > ctype.h \
> > errno.h \
> > nolibc.h \
> > diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h
> > new file mode 100644
> > index 000000000000..f9db2389acd2
> > --- /dev/null
> > +++ b/tools/include/nolibc/crt.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
> > +/*
> > + * C Run Time support for NOLIBC
> > + * Copyright (C) 2023 Zhangjin Wu <falcon@tinylab.org>
> > + */
> > +
> > +#ifndef _NOLIBC_CRT_H
> > +#define _NOLIBC_CRT_H
> > +
> > +char **environ __attribute__((weak));
> > +const unsigned long *_auxv __attribute__((weak));
> > +
> > +typedef int (_nolibc_main_fn)(int, char **, char **);
>
> What's the advantage of the typedef over using the pointer type inline?
>
With the extra comment added, this is not required anymore, will remove
this typedef.
> > +static void exit(int);
> > +
> > +void _start_c(long *sp)
> > +{
> > + int argc, i;
> > + char **argv;
> > + char **envp;
> > + /* silence potential warning: conflicting types for 'main' */
> > + _nolibc_main_fn _nolibc_main __asm__ ("main");
>
> What about the stackprotector initialization?
> It would really fit great into this series.
>
Ok, which gcc version supports stackprotector? seems the test even skip
on gcc 10, I will find one to verify the code change.
> > +
> > + /*
> > + * sp : argc <-- argument count, required by main()
> > + * argv: argv[0] <-- argument vector, required by main()
> > + * argv[1]
> > + * ...
> > + * argv[argc-1]
> > + * null
> > + * envp: envp[0] <-- environment variables, required by main() and getenv()
> > + * envp[1]
> > + * ...
> > + * null
> > + * _auxv: auxv[0] <-- auxiliary vector, required by getauxval()
> > + * auxv[1]
> > + * ...
> > + * null
> > + */
> > +
> > + /* assign argc and argv */
> > + argc = sp[0];
> > + argv = (void *)(sp + 1);
>
> Bit of a weird mismatch between array syntax and pointer arithmetic.
>
This 'argc = *sp;' may be better ;-)
> > +
> > + /* find envp */
> > + envp = argv + argc + 1;
> > + environ = envp;
>
> Is envp really needed? Could just be assigned directly to environ.
>
Ok, let's save one variable, envp is used to be consistent with the
frequenty declaration of main().
> > +
> > + /* find auxv */
> > + i = 0;
> > + while (envp[i])
> > + i++;
> > + _auxv = (void *)(envp + i + 1);
>
> Could be simplified a bit:
>
> _auxv = (void *) envp;
> while (_auxv)
> _auxv++;
>
Yeah, it is better, but needs a little change.
_auxv = (void *) envp;
while (*_auxv)
_auxv++;
_auxv++;
Thanks,
Zhangjin
> > +
> > + /* go to application */
> > + exit(_nolibc_main(argc, argv, envp));
> > +}
> > +
> > +#endif /* _NOLIBC_CRT_H */
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-13 6:12 ` Zhangjin Wu
@ 2023-07-13 6:23 ` Zhangjin Wu
2023-07-13 6:24 ` Thomas Weißschuh
2023-07-13 6:34 ` Willy Tarreau
2 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-13 6:23 UTC (permalink / raw)
To: falcon; +Cc: arnd, linux-kernel, linux-kselftest, thomas, w
Hi, Thomas
> > On 2023-07-12 17:17:39+0800, Zhangjin Wu wrote:
> [...]
> > > +static void exit(int);
> > > +
> > > +void _start_c(long *sp)
> > > +{
> > > + int argc, i;
> > > + char **argv;
> > > + char **envp;
> > > + /* silence potential warning: conflicting types for 'main' */
> > > + _nolibc_main_fn _nolibc_main __asm__ ("main");
> >
> > What about the stackprotector initialization?
> > It would really fit great into this series.
> >
>
> Ok, which gcc version supports stackprotector? seems the test even skip
> on gcc 10, I will find one to verify the code change.
>
Thomas, please ignore this question, I forgot some options in my own script.
Best regards,
Zhangjin
> > > +
> > > + /*
> > > + * sp : argc <-- argument count, required by main()
> > > + * argv: argv[0] <-- argument vector, required by main()
> > > + * argv[1]
> > > + * ...
> > > + * argv[argc-1]
> > > + * null
> > > + * envp: envp[0] <-- environment variables, required by main() and getenv()
> > > + * envp[1]
> > > + * ...
> > > + * null
> > > + * _auxv: auxv[0] <-- auxiliary vector, required by getauxval()
> > > + * auxv[1]
> > > + * ...
> > > + * null
> > > + */
> > > +
> > > + /* assign argc and argv */
> > > + argc = sp[0];
> > > + argv = (void *)(sp + 1);
> >
> > Bit of a weird mismatch between array syntax and pointer arithmetic.
> >
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-13 6:12 ` Zhangjin Wu
2023-07-13 6:23 ` Zhangjin Wu
@ 2023-07-13 6:24 ` Thomas Weißschuh
2023-07-13 6:34 ` Willy Tarreau
2 siblings, 0 replies; 27+ messages in thread
From: Thomas Weißschuh @ 2023-07-13 6:24 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: arnd, linux-kernel, linux-kselftest, w
On 2023-07-13 14:12:27+0800, Zhangjin Wu wrote:
> > On 2023-07-12 17:17:39+0800, Zhangjin Wu wrote:
> [..]
> > > diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h
> > > new file mode 100644
> > > index 000000000000..f9db2389acd2
> > > --- /dev/null
> > > +++ b/tools/include/nolibc/crt.h
> > > @@ -0,0 +1,59 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
> > > +/*
> > > + * C Run Time support for NOLIBC
> > > + * Copyright (C) 2023 Zhangjin Wu <falcon@tinylab.org>
> > > + */
> > > +
> > > +#ifndef _NOLIBC_CRT_H
> > > +#define _NOLIBC_CRT_H
> > > +
> > > +char **environ __attribute__((weak));
> > > +const unsigned long *_auxv __attribute__((weak));
> > > +
> > > +typedef int (_nolibc_main_fn)(int, char **, char **);
> >
> > What's the advantage of the typedef over using the pointer type inline?
> >
>
> With the extra comment added, this is not required anymore, will remove
> this typedef.
>
> > > +static void exit(int);
> > > +
> > > +void _start_c(long *sp)
> > > +{
> > > + int argc, i;
> > > + char **argv;
> > > + char **envp;
> > > + /* silence potential warning: conflicting types for 'main' */
> > > + _nolibc_main_fn _nolibc_main __asm__ ("main");
> >
> > What about the stackprotector initialization?
> > It would really fit great into this series.
> >
>
> Ok, which gcc version supports stackprotector? seems the test even skip
> on gcc 10, I will find one to verify the code change.
For crosscompilation I use the crosstools from kernel.org at
https://mirrors.edge.kernel.org/pub/tools/crosstool/
It seems to be at least in their gcc 9.5.0.
And also in their 11.2 which I use mostly at the moment.
> > > + /*
> > > + * sp : argc <-- argument count, required by main()
> > > + * argv: argv[0] <-- argument vector, required by main()
> > > + * argv[1]
> > > + * ...
> > > + * argv[argc-1]
> > > + * null
> > > + * envp: envp[0] <-- environment variables, required by main() and getenv()
> > > + * envp[1]
> > > + * ...
> > > + * null
> > > + * _auxv: auxv[0] <-- auxiliary vector, required by getauxval()
> > > + * auxv[1]
> > > + * ...
> > > + * null
> > > + */
> > > +
> > > + /* assign argc and argv */
> > > + argc = sp[0];
> > > + argv = (void *)(sp + 1);
> >
> > Bit of a weird mismatch between array syntax and pointer arithmetic.
> >
>
> This 'argc = *sp;' may be better ;-)
>
> > > +
> > > + /* find envp */
> > > + envp = argv + argc + 1;
> > > + environ = envp;
> >
> > Is envp really needed? Could just be assigned directly to environ.
> >
>
> Ok, let's save one variable, envp is used to be consistent with the
> frequenty declaration of main().
>
> > > +
> > > + /* find auxv */
> > > + i = 0;
> > > + while (envp[i])
> > > + i++;
> > > + _auxv = (void *)(envp + i + 1);
> >
> > Could be simplified a bit:
> >
> > _auxv = (void *) envp;
> > while (_auxv)
> > _auxv++;
> >
>
> Yeah, it is better, but needs a little change.
>
> _auxv = (void *) envp;
> while (*_auxv)
> _auxv++;
> _auxv++;
Good catch, thanks.
> [..]
Thomas
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-13 6:12 ` Zhangjin Wu
2023-07-13 6:23 ` Zhangjin Wu
2023-07-13 6:24 ` Thomas Weißschuh
@ 2023-07-13 6:34 ` Willy Tarreau
2023-07-14 5:58 ` Zhangjin Wu
2 siblings, 1 reply; 27+ messages in thread
From: Willy Tarreau @ 2023-07-13 6:34 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: thomas, arnd, linux-kernel, linux-kselftest
Hi Zhangjin,
I haven't reviewed the rest yet but regarding this point:
On Thu, Jul 13, 2023 at 02:12:27PM +0800, Zhangjin Wu wrote:
> > > + /* find auxv */
> > > + i = 0;
> > > + while (envp[i])
> > > + i++;
> > > + _auxv = (void *)(envp + i + 1);
> >
> > Could be simplified a bit:
> >
> > _auxv = (void *) envp;
> > while (_auxv)
> > _auxv++;
> >
>
> Yeah, it is better, but needs a little change.
>
> _auxv = (void *) envp;
> while (*_auxv)
> _auxv++;
> _auxv++;
Or just:
_auxv = (void*)environ;
while (*_auxv++)
;
or:
for (_auxv = (void*)environ; *_auxv++; )
;
Please also have a look at the output code, because at low optimization
levels, compilers sometimes produce a better code with a local variable
than with a global variable in a loop. Thus I wouldn't be that much
surprised if at -O0 or -O1 you'd see slightly more compact code using:
/* find envp */
environ = argv + argc + 1;
/* find auxv */
for (auxv = environ; *auxv++)
;
_auxv = auxv;
than:
/* find envp */
envp = argv + argc + 1;
environ = envp;
/* find auxv */
for (_auxv = environ; *_auxv++)
;
Since it's going to become generic code, it's worth running a few tests
to see how to best polish it.
Thanks,
Willy
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-13 6:34 ` Willy Tarreau
@ 2023-07-14 5:58 ` Zhangjin Wu
2023-07-14 6:34 ` Thomas Weißschuh
2023-07-15 9:57 ` Willy Tarreau
0 siblings, 2 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-14 5:58 UTC (permalink / raw)
To: w; +Cc: arnd, falcon, linux-kernel, linux-kselftest, thomas
Hi, Willy, Thomas
> Hi Zhangjin,
>
> I haven't reviewed the rest yet but regarding this point:
>
> On Thu, Jul 13, 2023 at 02:12:27PM +0800, Zhangjin Wu wrote:
> > > > + /* find auxv */
> > > > + i = 0;
> > > > + while (envp[i])
> > > > + i++;
> > > > + _auxv = (void *)(envp + i + 1);
> > >
> > > Could be simplified a bit:
> > >
> > > _auxv = (void *) envp;
> > > while (_auxv)
> > > _auxv++;
> > >
> >
> > Yeah, it is better, but needs a little change.
> >
> > _auxv = (void *) envp;
> > while (*_auxv)
> > _auxv++;
> > _auxv++;
>
> Or just:
>
> _auxv = (void*)environ;
> while (*_auxv++)
> ;
>
> or:
>
> for (_auxv = (void*)environ; *_auxv++; )
> ;
>
> Please also have a look at the output code, because at low optimization
> levels, compilers sometimes produce a better code with a local variable
> than with a global variable in a loop. Thus I wouldn't be that much
> surprised if at -O0 or -O1 you'd see slightly more compact code using:
>
Very good suggestion, I did find some interesting results, after
removing some local variables, although the text size shrinks, but the
total size is the same with -O0/1/2/3/s.
Here is the diff with -O0/1/2/3/s (a REPORT_SIZE macro may be required for
this):
$ diff -Nubr startup.old.txt startup.new.txt
--- startup.old.txt 2023-07-14 10:22:45.990413661 +0800
+++ startup.new.txt 2023-07-14 10:22:52.278869146 +0800
@@ -2,34 +2,34 @@
sudo strip -s nolibc-test
size nolibc-test
text data bss dec hex filename
- 46872 88 80 47040 b7c0 nolibc-test
+ 46836 88 80 47004 b79c nolibc-test
wc -c nolibc-test
58016 nolibc-test
O1:
sudo strip -s nolibc-test
size nolibc-test
text data bss dec hex filename
- 27298 88 72 27458 6b42 nolibc-test
+ 27287 88 72 27447 6b37 nolibc-test
wc -c nolibc-test
37536 nolibc-test
O2:
sudo strip -s nolibc-test
size nolibc-test
text data bss dec hex filename
- 25688 88 72 25848 64f8 nolibc-test
+ 25672 88 72 25832 64e8 nolibc-test
wc -c nolibc-test
37536 nolibc-test
O3:
sudo strip -s nolibc-test
size nolibc-test
text data bss dec hex filename
- 44020 88 72 44180 ac94 nolibc-test
+ 44004 88 72 44164 ac84 nolibc-test
wc -c nolibc-test
53920 nolibc-test
Os:
sudo strip -s nolibc-test
size nolibc-test
text data bss dec hex filename
- 20887 88 72 21047 5237 nolibc-test
+ 20884 88 72 21044 5234 nolibc-test
wc -c nolibc-test
33440 nolibc-test
The code with local variables:
void _start_c(long *sp)
{
int argc, i;
char **argv;
char **envp;
/* silence potential warning: conflicting types for 'main' */
int _nolibc_main(int, char **, char **) __asm__ ("main");
/*
* sp : argc <-- argument count, required by main()
* argv: argv[0] <-- argument vector, required by main()
* argv[1]
* ...
* argv[argc-1]
* null
* environ: environ[0] <-- environment variables, required by main() and getenv()
* environ[1]
* ...
* null
* _auxv: _auxv[0] <-- auxiliary vector, required by getauxval()
* _auxv[1]
* ...
* null
*/
/* assign argc and argv */
argc = *sp;
argv = (void *)(sp + 1);
/* find environ */
environ = envp = argv + argc + 1;
/* find _auxv */
for (i = 0; *(envp + i); i++)
;
_auxv = (void *)(envp + i + 1);
/* go to application */
exit(_nolibc_main(argc, argv, envp));
}
The code with global variables:
void _start_c(long *sp)
{
int argc;
char **argv;
/* silence potential warning: conflicting types for 'main' */
int _nolibc_main(int, char **, char **) __asm__ ("main");
/*
* sp : argc <-- argument count, required by main()
* argv: argv[0] <-- argument vector, required by main()
* argv[1]
* ...
* argv[argc-1]
* null
* environ: environ[0] <-- environment variables, required by main() and getenv()
* environ[1]
* ...
* null
* _auxv: _auxv[0] <-- auxiliary vector, required by getauxval()
* _auxv[1]
* ...
* null
*/
/* assign argc and argv */
argc = *sp;
argv = (void *)(sp + 1);
/* find environ */
environ = argv + argc + 1;
/* find _auxv */
for (_auxv = (void *)environ; *_auxv++;)
;
/* go to application */
exit(_nolibc_main(argc, argv, environ));
}
Which one do you prefer? the one with local variables may be more readable (not
that much), the one with global variables has smaller text size (and therefore
smaller memory footprint).
BTW, just found an arch-<ARCH>.h bug with -O0, seems the
'optimize("omit-frame-pointer")' attribute not really work as expected with
-O0. It uses frame pointer for _start eventually and breaks the stack pointer
variable (a push 'rbp' inserted at the begging of _start, so, the real rsp has
an offset), so, therefore, it is not able to get the right argc, argv, environ
and _auxv with -O0 currently. A solution is reverting _start to pure assembly.
For the above tests, I manually reverted the arch-x86_64.h to:
__asm__(
".text\n"
".weak _start\n"
"_start:\n"
#ifdef _NOLIBC_STACKPROTECTOR
"call __stack_chk_init\n" /* initialize stack protector */
#endif
"xor %ebp, %ebp\n" /* zero the stack frame */
"mov %rsp, %rdi\n" /* save stack pointer to %rdi, as arg1 of _start_c */
"and $-16, %rsp\n" /* %rsp must be 16-byte aligned before call */
"call _start_c\n" /* transfer to c runtime */
"hlt\n" /* ensure it does not return */
);
'man gcc' shows:
Most optimizations are completely disabled at -O0 or if an -O level is not set on the command line, even if individual optimization flags are specified.
To want -O0 work again, since now we have C _start_c, is it ok for us to revert
the commit 7f8548589661 ("tools/nolibc: make compiler and assembler agree on
the section around _start") and the later __no_stack_protector changes?
At the same time, to verify such issues, as Thomas suggested, in this series,
we may need to add more startup tests to verify argc, argv, environ, _auxv, do
we need to add a standalone run_startup (or run_crt) test entry just like
run_syscall? or, let's simply add more in the run_stdlib, like the environ test
added by Thomas. seems, the argc test is necessary one currently missing (argc
>= 1):
CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
As we summarized before, the related test cases are:
argv0:
CASE_TEST(chmod_argv0); EXPECT_SYSZR(1, chmod(test_argv0, 0555)); break;
CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(test_argv0), -1, ENOTDIR); break;
environ:
CASE_TEST(chdir_root); EXPECT_SYSZR(1, chdir("/")); chdir(getenv("PWD")); break;
CASE_TEST(environ); EXPECT_PTREQ(1, environ, test_envp); break;
CASE_TEST(getenv_TERM); EXPECT_STRNZ(1, getenv("TERM")); break;
CASE_TEST(getenv_blah); EXPECT_STRZR(1, getenv("blah")); break;
auxv:
CASE_TEST(getpagesize); EXPECT_SYSZR(1, test_getpagesize()); break;
The above tests are in different test group and are not aimed to startup test,
we'd better add a run_startup (or run_crt) test group before any other tests,
it is a requiremnt of the others, we at least have these ones:
+int run_startup(int min, int max)
+{
+ int test;
+ int tmp;
+ int ret = 0;
+
+ for (test = min; test >= 0 && test <= max; test++) {
+ int llen = 0; /* line length */
+
+ /* avoid leaving empty lines below, this will insert holes into
+ * test numbers.
+ */
+ switch (test + __LINE__ + 1) {
+ CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
+ CASE_TEST(argv_addr); EXPECT_PTRNZ(1, test_argv); break;
+ CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc); break;
+ CASE_TEST(argv0_addr); EXPECT_PTRNZ(1, argv0); break;
+ CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0); break;
+ CASE_TEST(argv0_len); EXPECT_GE(1, strlen(argv0), 1); break;
+ CASE_TEST(environ_addr); EXPECT_PTRNZ(1, environ); break;
+ CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
+ CASE_TEST(environ_total); EXPECT_GE(1, (void *)_auxv - (void *)environ - 1, 1); break;
+ CASE_TEST(_auxv_addr); EXPECT_PTRNZ(1, _auxv); break;
+ case __LINE__:
+ return ret; /* must be last */
+ /* note: do not set any defaults so as to permit holes above */
+ }
+ }
+ return ret;
+}
Any more?
> /* find envp */
> environ = argv + argc + 1;
>
> /* find auxv */
> for (auxv = environ; *auxv++)
> ;
> _auxv = auxv;
>
> than:
> /* find envp */
> envp = argv + argc + 1;
> environ = envp;
>
> /* find auxv */
> for (_auxv = environ; *_auxv++)
> ;
>
Great, `*_auxv++` is a very good trick to avoid duplicated _auxv++, although it
is a little hard to read (not that hard in reality) ;-)
> Since it's going to become generic code, it's worth running a few tests
> to see how to best polish it.
>
Yes, I focused on the big shrinking itself but forgot to polish the _start_c
itself ;-)
Best regards,
Zhangjin
> Thanks,
> Willy
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-14 5:58 ` Zhangjin Wu
@ 2023-07-14 6:34 ` Thomas Weißschuh
2023-07-14 9:47 ` Zhangjin Wu
2023-07-15 9:57 ` Willy Tarreau
1 sibling, 1 reply; 27+ messages in thread
From: Thomas Weißschuh @ 2023-07-14 6:34 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: w, arnd, linux-kernel, linux-kselftest
On 2023-07-14 13:58:13+0800, Zhangjin Wu wrote:
> [..]
> Which one do you prefer? the one with local variables may be more readable (not
> that much), the one with global variables has smaller text size (and therefore
> smaller memory footprint).
The one with local variables. But not by much.
> BTW, just found an arch-<ARCH>.h bug with -O0, seems the
> 'optimize("omit-frame-pointer")' attribute not really work as expected with
> -O0. It uses frame pointer for _start eventually and breaks the stack pointer
> variable (a push 'rbp' inserted at the begging of _start, so, the real rsp has
> an offset), so, therefore, it is not able to get the right argc, argv, environ
> and _auxv with -O0 currently. A solution is reverting _start to pure assembly.
>
> For the above tests, I manually reverted the arch-x86_64.h to:
>
> __asm__(
> ".text\n"
> ".weak _start\n"
> "_start:\n"
> #ifdef _NOLIBC_STACKPROTECTOR
> "call __stack_chk_init\n" /* initialize stack protector */
> #endif
> "xor %ebp, %ebp\n" /* zero the stack frame */
> "mov %rsp, %rdi\n" /* save stack pointer to %rdi, as arg1 of _start_c */
> "and $-16, %rsp\n" /* %rsp must be 16-byte aligned before call */
> "call _start_c\n" /* transfer to c runtime */
> "hlt\n" /* ensure it does not return */
> );
>
>
> 'man gcc' shows:
>
> Most optimizations are completely disabled at -O0 or if an -O level is not set on the command line, even if individual optimization flags are specified.
>
> To want -O0 work again, since now we have C _start_c, is it ok for us to revert
> the commit 7f8548589661 ("tools/nolibc: make compiler and assembler agree on
> the section around _start") and the later __no_stack_protector changes?
This commit explicitly mentions being tested with -O0 on x86_64.
I was also not able to reproduce the issue.
Before doing any reverts I think some more investigation is in order.
Can you provide exact reproduction steps?
> At the same time, to verify such issues, as Thomas suggested, in this series,
> we may need to add more startup tests to verify argc, argv, environ, _auxv, do
> we need to add a standalone run_startup (or run_crt) test entry just like
> run_syscall? or, let's simply add more in the run_stdlib, like the environ test
> added by Thomas. seems, the argc test is necessary one currently missing (argc
> >= 1):
>
> CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
>
> As we summarized before, the related test cases are:
>
> argv0:
>
> CASE_TEST(chmod_argv0); EXPECT_SYSZR(1, chmod(test_argv0, 0555)); break;
> CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(test_argv0), -1, ENOTDIR); break;
>
> environ:
>
> CASE_TEST(chdir_root); EXPECT_SYSZR(1, chdir("/")); chdir(getenv("PWD")); break;
> CASE_TEST(environ); EXPECT_PTREQ(1, environ, test_envp); break;
> CASE_TEST(getenv_TERM); EXPECT_STRNZ(1, getenv("TERM")); break;
> CASE_TEST(getenv_blah); EXPECT_STRZR(1, getenv("blah")); break;
>
> auxv:
>
> CASE_TEST(getpagesize); EXPECT_SYSZR(1, test_getpagesize()); break;
>
> The above tests are in different test group and are not aimed to startup test,
> we'd better add a run_startup (or run_crt) test group before any other tests,
> it is a requiremnt of the others, we at least have these ones:
>
> +int run_startup(int min, int max)
> +{
> + int test;
> + int tmp;
> + int ret = 0;
> +
> + for (test = min; test >= 0 && test <= max; test++) {
> + int llen = 0; /* line length */
> +
> + /* avoid leaving empty lines below, this will insert holes into
> + * test numbers.
> + */
> + switch (test + __LINE__ + 1) {
> + CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
> + CASE_TEST(argv_addr); EXPECT_PTRNZ(1, test_argv); break;
> + CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc); break;
> + CASE_TEST(argv0_addr); EXPECT_PTRNZ(1, argv0); break;
> + CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0); break;
> + CASE_TEST(argv0_len); EXPECT_GE(1, strlen(argv0), 1); break;
> + CASE_TEST(environ_addr); EXPECT_PTRNZ(1, environ); break;
> + CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
> + CASE_TEST(environ_total); EXPECT_GE(1, (void *)_auxv - (void *)environ - 1, 1); break;
> + CASE_TEST(_auxv_addr); EXPECT_PTRNZ(1, _auxv); break;
> + case __LINE__:
> + return ret; /* must be last */
> + /* note: do not set any defaults so as to permit holes above */
> + }
> + }
> + return ret;
> +}
>
> Any more?
My original idea was to have tests that exec /proc/self/exe or argv0.
This way we can actually pass and validate arbitrary argc, argv and
environ values.
But looking at your list, that should be enough.
> [..]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-14 6:34 ` Thomas Weißschuh
@ 2023-07-14 9:47 ` Zhangjin Wu
2023-07-15 7:24 ` Thomas Weißschuh
0 siblings, 1 reply; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-14 9:47 UTC (permalink / raw)
To: thomas; +Cc: arnd, falcon, linux-kernel, linux-kselftest, w
> On 2023-07-14 13:58:13+0800, Zhangjin Wu wrote:
>
> > [..]
>
> > Which one do you prefer? the one with local variables may be more readable (not
> > that much), the one with global variables has smaller text size (and therefore
> > smaller memory footprint).
>
> The one with local variables. But not by much.
>
> > BTW, just found an arch-<ARCH>.h bug with -O0, seems the
> > 'optimize("omit-frame-pointer")' attribute not really work as expected with
> > -O0. It uses frame pointer for _start eventually and breaks the stack pointer
> > variable (a push 'rbp' inserted at the begging of _start, so, the real rsp has
> > an offset), so, therefore, it is not able to get the right argc, argv, environ
> > and _auxv with -O0 currently. A solution is reverting _start to pure assembly.
> >
> > For the above tests, I manually reverted the arch-x86_64.h to:
> >
> > __asm__(
> > ".text\n"
> > ".weak _start\n"
> > "_start:\n"
> > #ifdef _NOLIBC_STACKPROTECTOR
> > "call __stack_chk_init\n" /* initialize stack protector */
> > #endif
> > "xor %ebp, %ebp\n" /* zero the stack frame */
> > "mov %rsp, %rdi\n" /* save stack pointer to %rdi, as arg1 of _start_c */
> > "and $-16, %rsp\n" /* %rsp must be 16-byte aligned before call */
> > "call _start_c\n" /* transfer to c runtime */
> > "hlt\n" /* ensure it does not return */
> > );
> >
> >
> > 'man gcc' shows:
> >
> > Most optimizations are completely disabled at -O0 or if an -O level is not set on the command line, even if individual optimization flags are specified.
> >
> > To want -O0 work again, since now we have C _start_c, is it ok for us to revert
> > the commit 7f8548589661 ("tools/nolibc: make compiler and assembler agree on
> > the section around _start") and the later __no_stack_protector changes?
>
> This commit explicitly mentions being tested with -O0 on x86_64.
Yeah, I was worried about that the old tests didn't use any of the startup
variables, but the getpagesize test may be a very old test, It uses _auxv and
should fail If -O0 not work.
> I was also not able to reproduce the issue.
>
Thanks very much for your 'reproduce' result, It is so weird, just
rechecked the toolchain, 13.1.0 from https://mirrors.edge.kernel.org/ is
ok, gcc 9, gcc 10.3 not work.
But even in the page of 13.1.0 [1], we still see this line:
Most optimizations are completely disabled at -O0 or if an -O level is not set on the command line, even if individual optimization flags are specified.
Not sure if "individual optimization flags" also means the optimize()
flags in gcc attributes. or the doc is not updated yet?
And further found gcc 11.1.0 is ok, gcc 10.4 still not work, so, gcc
11.1.0 may changed something to let the "individual optimization flags"
work with -O0.
We may need to at least document this issue in some files, -O0 is not such a
frequently-used option, not sure if we still need -O0 work with the older gcc <
11.1.0 ;-)
Willy, I'm not sure if the issues solved by the commit 7f8548589661
("tools/nolibc: make compiler and assembler agree on the section around
_start") still exist after we using _start_c()?
Thomas, because we plan to move the stackprotector init to _start_c(), If using
pure assembly _start, we may also not need the __no_stack_protector macro too?
Welcome more discussion.
[1]: https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/Optimize-Options.html
> Before doing any reverts I think some more investigation is in order.
> Can you provide exact reproduction steps?
>
some startup variables related tests failed with gcc 9 and gcc 10 (even with
gcc 10.4.0):
$ x86_64-linux-gcc --version
x86_64-linux-gcc (GCC) 10.4.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ make run-user CROSS_COMPILE=x86_64-linux-
0 argc = 0 [FAIL]
3 argv_total = 0 [FAIL]
4 argv0_addr = <0x1> [FAIL]
5 argv0_str = <(null)> [FAIL]
6 argv0_len = 0 [FAIL]
14 chmod_argv0 = -1 EFAULT [FAIL]
19 chroot_exe = -1 EFAULT != (-1 ENOTDIR) [FAIL]
0 getenv_TERM = <(null)> [FAIL]
> > At the same time, to verify such issues, as Thomas suggested, in this series,
> > we may need to add more startup tests to verify argc, argv, environ, _auxv, do
> > we need to add a standalone run_startup (or run_crt) test entry just like
> > run_syscall? or, let's simply add more in the run_stdlib, like the environ test
> > added by Thomas. seems, the argc test is necessary one currently missing (argc
> > >= 1):
> >
> > CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
> >
> > As we summarized before, the related test cases are:
> >
> > argv0:
> >
> > CASE_TEST(chmod_argv0); EXPECT_SYSZR(1, chmod(test_argv0, 0555)); break;
> > CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(test_argv0), -1, ENOTDIR); break;
> >
> > environ:
> >
> > CASE_TEST(chdir_root); EXPECT_SYSZR(1, chdir("/")); chdir(getenv("PWD")); break;
> > CASE_TEST(environ); EXPECT_PTREQ(1, environ, test_envp); break;
> > CASE_TEST(getenv_TERM); EXPECT_STRNZ(1, getenv("TERM")); break;
> > CASE_TEST(getenv_blah); EXPECT_STRZR(1, getenv("blah")); break;
> >
> > auxv:
> >
> > CASE_TEST(getpagesize); EXPECT_SYSZR(1, test_getpagesize()); break;
> >
> > The above tests are in different test group and are not aimed to startup test,
> > we'd better add a run_startup (or run_crt) test group before any other tests,
> > it is a requiremnt of the others, we at least have these ones:
> >
> > +int run_startup(int min, int max)
> > +{
> > + int test;
> > + int tmp;
> > + int ret = 0;
> > +
> > + for (test = min; test >= 0 && test <= max; test++) {
> > + int llen = 0; /* line length */
> > +
> > + /* avoid leaving empty lines below, this will insert holes into
> > + * test numbers.
> > + */
> > + switch (test + __LINE__ + 1) {
> > + CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
> > + CASE_TEST(argv_addr); EXPECT_PTRNZ(1, test_argv); break;
> > + CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc); break;
> > + CASE_TEST(argv0_addr); EXPECT_PTRNZ(1, argv0); break;
> > + CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0); break;
> > + CASE_TEST(argv0_len); EXPECT_GE(1, strlen(argv0), 1); break;
> > + CASE_TEST(environ_addr); EXPECT_PTRNZ(1, environ); break;
> > + CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
> > + CASE_TEST(environ_total); EXPECT_GE(1, (void *)_auxv - (void *)environ - 1, 1); break;
> > + CASE_TEST(_auxv_addr); EXPECT_PTRNZ(1, _auxv); break;
> > + case __LINE__:
> > + return ret; /* must be last */
> > + /* note: do not set any defaults so as to permit holes above */
> > + }
> > + }
> > + return ret;
> > +}
> >
> > Any more?
>
> My original idea was to have tests that exec /proc/self/exe or argv0.
> This way we can actually pass and validate arbitrary argc, argv and
> environ values.
>
> But looking at your list, that should be enough.
>
Ok.
Best regards,
Zhangjin
> > [..]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-14 9:47 ` Zhangjin Wu
@ 2023-07-15 7:24 ` Thomas Weißschuh
2023-07-15 9:23 ` Zhangjin Wu
0 siblings, 1 reply; 27+ messages in thread
From: Thomas Weißschuh @ 2023-07-15 7:24 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: arnd, linux-kernel, linux-kselftest, w
On 2023-07-14 17:47:23+0800, Zhangjin Wu wrote:
> > On 2023-07-14 13:58:13+0800, Zhangjin Wu wrote:
> [..]
> > I was also not able to reproduce the issue.
> >
>
> Thanks very much for your 'reproduce' result, It is so weird, just
> rechecked the toolchain, 13.1.0 from https://mirrors.edge.kernel.org/ is
> ok, gcc 9, gcc 10.3 not work.
>
> But even in the page of 13.1.0 [1], we still see this line:
>
> Most optimizations are completely disabled at -O0 or if an -O level is not set on the command line, even if individual optimization flags are specified.
>
> Not sure if "individual optimization flags" also means the optimize()
> flags in gcc attributes. or the doc is not updated yet?
>
> And further found gcc 11.1.0 is ok, gcc 10.4 still not work, so, gcc
> 11.1.0 may changed something to let the "individual optimization flags"
> work with -O0.
>
> We may need to at least document this issue in some files, -O0 is not such a
> frequently-used option, not sure if we still need -O0 work with the older gcc <
> 11.1.0 ;-)
It seems we can avoid the issue by enforcing optimizations for _start:
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index f5614a67f05a..b9d8b8861dc4 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -161,12 +161,9 @@
* 2) The deepest stack frame should be zero (the %rbp).
*
*/
-void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
>
> Willy, I'm not sure if the issues solved by the commit 7f8548589661
> ("tools/nolibc: make compiler and assembler agree on the section around
> _start") still exist after we using _start_c()?
>
> Thomas, because we plan to move the stackprotector init to _start_c(), If using
> pure assembly _start, we may also not need the __no_stack_protector macro too?
It would probably not needed anymore in this case.
Thomas
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-15 7:24 ` Thomas Weißschuh
@ 2023-07-15 9:23 ` Zhangjin Wu
0 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-15 9:23 UTC (permalink / raw)
To: thomas; +Cc: arnd, falcon, linux-kernel, linux-kselftest, w
> On 2023-07-14 17:47:23+0800, Zhangjin Wu wrote:
> > > On 2023-07-14 13:58:13+0800, Zhangjin Wu wrote:
>
> > [..]
>
> > > I was also not able to reproduce the issue.
> > >
> >
> > Thanks very much for your 'reproduce' result, It is so weird, just
> > rechecked the toolchain, 13.1.0 from https://mirrors.edge.kernel.org/ is
> > ok, gcc 9, gcc 10.3 not work.
> >
> > But even in the page of 13.1.0 [1], we still see this line:
> >
> > Most optimizations are completely disabled at -O0 or if an -O level is not set on the command line, even if individual optimization flags are specified.
> >
> > Not sure if "individual optimization flags" also means the optimize()
> > flags in gcc attributes. or the doc is not updated yet?
> >
> > And further found gcc 11.1.0 is ok, gcc 10.4 still not work, so, gcc
> > 11.1.0 may changed something to let the "individual optimization flags"
> > work with -O0.
> >
> > We may need to at least document this issue in some files, -O0 is not such a
> > frequently-used option, not sure if we still need -O0 work with the older gcc <
> > 11.1.0 ;-)
>
> It seems we can avoid the issue by enforcing optimizations for _start:
>
> diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
> index f5614a67f05a..b9d8b8861dc4 100644
> --- a/tools/include/nolibc/arch-x86_64.h
> +++ b/tools/include/nolibc/arch-x86_64.h
> @@ -161,12 +161,9 @@
> * 2) The deepest stack frame should be zero (the %rbp).
> *
> */
> -void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
> +void __attribute__((weak, noreturn, optimize("Os", "omit-frame-pointer"))) __no_stack_protector _start(void)
>
Great, it works and it is minimal enough ;-)
Thanks very much.
> >
> > Willy, I'm not sure if the issues solved by the commit 7f8548589661
> > ("tools/nolibc: make compiler and assembler agree on the section around
> > _start") still exist after we using _start_c()?
> >
> > Thomas, because we plan to move the stackprotector init to _start_c(), If using
> > pure assembly _start, we may also not need the __no_stack_protector macro too?
>
> It would probably not needed anymore in this case.
>
Yeah, but let's reserve it as-is for we have the working
omit-frame-pointer now.
Best regards,
Zhangjin
> Thomas
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c
2023-07-14 5:58 ` Zhangjin Wu
2023-07-14 6:34 ` Thomas Weißschuh
@ 2023-07-15 9:57 ` Willy Tarreau
1 sibling, 0 replies; 27+ messages in thread
From: Willy Tarreau @ 2023-07-15 9:57 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: arnd, linux-kernel, linux-kselftest, thomas
Hi Zhangjin,
On Fri, Jul 14, 2023 at 01:58:13PM +0800, Zhangjin Wu wrote:
> > or:
> >
> > for (_auxv = (void*)environ; *_auxv++; )
> > ;
> >
> > Please also have a look at the output code, because at low optimization
> > levels, compilers sometimes produce a better code with a local variable
> > than with a global variable in a loop. Thus I wouldn't be that much
> > surprised if at -O0 or -O1 you'd see slightly more compact code using:
> >
>
> Very good suggestion, I did find some interesting results, after
> removing some local variables, although the text size shrinks, but the
> total size is the same with -O0/1/2/3/s.
>
> Here is the diff with -O0/1/2/3/s (a REPORT_SIZE macro may be required for
> this):
>
> $ diff -Nubr startup.old.txt startup.new.txt
> --- startup.old.txt 2023-07-14 10:22:45.990413661 +0800
> +++ startup.new.txt 2023-07-14 10:22:52.278869146 +0800
> @@ -2,34 +2,34 @@
> sudo strip -s nolibc-test
> size nolibc-test
> text data bss dec hex filename
> - 46872 88 80 47040 b7c0 nolibc-test
> + 46836 88 80 47004 b79c nolibc-test
(...)
I meant only checking the function's size, not the whole program :-)
Here's what I'm having for the function:
$ for opt in O0 O1 O2 Os O3; do
echo "## $opt"
for file in global local local2; do
gcc -$opt -c startc-$file.c
done
nm -o --size startc-*.o | grep _start_c
echo
done
## O0
startc-global.o:0000000000000089 T _start_c
startc-local.o:00000000000000ad T _start_c
startc-local2.o:0000000000000090 T _start_c
## O1
startc-global.o:0000000000000048 T _start_c
startc-local.o:0000000000000054 T _start_c
startc-local2.o:000000000000003a T _start_c
## O2
startc-global.o:0000000000000044 T _start_c
startc-local.o:000000000000004d T _start_c
startc-local2.o:0000000000000040 T _start_c
## Os
startc-global.o:0000000000000041 T _start_c
startc-local.o:0000000000000040 T _start_c
startc-local2.o:000000000000003a T _start_c
## O3
startc-global.o:0000000000000044 T _start_c
startc-local.o:000000000000004d T _start_c
startc-local2.o:0000000000000040 T _start_c
In local2 that's always smaller I've just used a local auxv variable
instead of iterating over the global one, and it's cheaper than using
an index since 1) the instructions are shorter, and 2) the value is
already available, no need to initialize one register to zero:
void _start_c(long *sp)
{
long argc;
char **argv;
char **envp;
const unsigned long *auxv;
/* silence potential warning: conflicting types for 'main' */
int _nolibc_main(int, char **, char **) __asm__ ("main");
/*
* sp : argc <-- argument count, required by main()
* argv: argv[0] <-- argument vector, required by main()
* argv[1]
* ...
* argv[argc-1]
* null
* environ: environ[0] <-- environment variables, required by main() and getenv()
* environ[1]
* ...
* null
* _auxv: _auxv[0] <-- auxiliary vector, required by getauxval()
* _auxv[1]
* ...
* null
*/
/* assign argc and argv */
argc = *sp;
argv = (void *)(sp + 1);
/* find environ */
environ = envp = argv + argc + 1;
/* find _auxv */
for (auxv = (void*)envp; *auxv++; )
;
_auxv = auxv;
/* go to application */
exit(_nolibc_main(argc, argv, envp));
}
I'm not showing how ugly it is at -O1, as I suspected the global version
does indeed perform a write to _auxv for each turn. At -Os it's
interesting:
In the global version it looks like this (rdx has envp):
13: 48 8d 42 08 lea 0x8(%rdx),%rax
17: 48 89 15 00 00 00 00 mov %rdx,0x0(%rip) # environ
1e: 48 89 c7 mov %rax,%rdi
21: 48 83 c0 08 add $0x8,%rax
25: 48 83 78 f0 00 cmpq $0x0,-0x10(%rax)
2a: 75 f2 jne 1e <_start_c+0x1e>
In your local version with the index, it looks like this (rdx has envp):
13: 31 c0 xor %eax,%eax
15: 48 89 15 00 00 00 00 mov %rdx,0x0(%rip) # environ
1c: 48 ff c0 inc %rax
1f: 48 83 7c c2 f8 00 cmpq $0x0,-0x8(%rdx,%rax,8)
25: 75 f5 jne 1c <_start_c+0x1c>
27: 48 8d 04 c2 lea (%rdx,%rax,8),%rax # rax now has auxv
In the one without index it's like this:
13: 48 89 15 00 00 00 00 mov %rdx,0x0(%rip) # environ
1a: 48 89 d0 mov %rdx,%rax
1d: 48 83 c0 08 add $0x8,%rax
21: 48 83 78 f8 00 cmpq $0x0,-0x8(%rax)
26: 75 f5 jne 1d <_start_c+0x1d>
Finally, since argc is used in pointer computation while being taken from
a long*, it experiences a double conversion while doing so. Storing it as
a long avoids this saving 3 extra bytes.
> Which one do you prefer? the one with local variables may be more readable (not
> that much), the one with global variables has smaller text size (and therefore
> smaller memory footprint).
The smaller one with local variables and no index ;-)
> BTW, just found an arch-<ARCH>.h bug with -O0, seems the
> 'optimize("omit-frame-pointer")' attribute not really work as expected with
> -O0. It uses frame pointer for _start eventually and breaks the stack pointer
> variable (a push 'rbp' inserted at the begging of _start, so, the real rsp has
> an offset), so, therefore, it is not able to get the right argc, argv, environ
> and _auxv with -O0 currently. A solution is reverting _start to pure assembly.
I didn't notice this one.
> 'man gcc' shows:
>
> Most optimizations are completely disabled at -O0 or if an -O level is
> not set on the command line, even if individual optimization flags are
> specified.
Indeed, that's pretty clear!
> To want -O0 work again, since now we have C _start_c, is it ok for us to revert
> the commit 7f8548589661 ("tools/nolibc: make compiler and assembler agree on
> the section around _start") and the later __no_stack_protector changes?
As Thomas said in somewhere else in the thread, let's analyze deeper first.
Maybe passing optimize("O") in addition will be sufficient.
> At the same time, to verify such issues, as Thomas suggested, in this series,
> we may need to add more startup tests to verify argc, argv, environ, _auxv, do
Yep!
> we need to add a standalone run_startup (or run_crt) test entry just like
> run_syscall? or, let's simply add more in the run_stdlib, like the environ test
> added by Thomas. seems, the argc test is necessary one currently missing (argc
> >= 1):
Yes it could be a good idea to add a startup test category. Some of the
stuff we've placed into "stdlib" are more about startup code (provided
by the lib) that tends to be very sensitive to the architecture and
optimizations.
> CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
>
> As we summarized before, the related test cases are:
>
> argv0:
>
> CASE_TEST(chmod_argv0); EXPECT_SYSZR(1, chmod(test_argv0, 0555)); break;
> CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(test_argv0), -1, ENOTDIR); break;
>
> environ:
>
> CASE_TEST(chdir_root); EXPECT_SYSZR(1, chdir("/")); chdir(getenv("PWD")); break;
> CASE_TEST(environ); EXPECT_PTREQ(1, environ, test_envp); break;
> CASE_TEST(getenv_TERM); EXPECT_STRNZ(1, getenv("TERM")); break;
> CASE_TEST(getenv_blah); EXPECT_STRZR(1, getenv("blah")); break;
>
> auxv:
>
> CASE_TEST(getpagesize); EXPECT_SYSZR(1, test_getpagesize()); break;
>
> The above tests are in different test group and are not aimed to startup test,
> we'd better add a run_startup (or run_crt) test group before any other tests,
> it is a requiremnt of the others, we at least have these ones:
>
> +int run_startup(int min, int max)
> +{
> + int test;
> + int tmp;
> + int ret = 0;
> +
> + for (test = min; test >= 0 && test <= max; test++) {
> + int llen = 0; /* line length */
> +
> + /* avoid leaving empty lines below, this will insert holes into
> + * test numbers.
> + */
> + switch (test + __LINE__ + 1) {
> + CASE_TEST(argc); EXPECT_GE(1, test_argc, 1); break;
> + CASE_TEST(argv_addr); EXPECT_PTRNZ(1, test_argv); break;
> + CASE_TEST(argv_total); EXPECT_EQ(1, environ - test_argv - 1, test_argc); break;
> + CASE_TEST(argv0_addr); EXPECT_PTRNZ(1, argv0); break;
> + CASE_TEST(argv0_str); EXPECT_STRNZ(1, argv0); break;
> + CASE_TEST(argv0_len); EXPECT_GE(1, strlen(argv0), 1); break;
> + CASE_TEST(environ_addr); EXPECT_PTRNZ(1, environ); break;
> + CASE_TEST(environ_envp); EXPECT_PTREQ(1, environ, test_envp); break;
> + CASE_TEST(environ_total); EXPECT_GE(1, (void *)_auxv - (void *)environ - 1, 1); break;
> + CASE_TEST(_auxv_addr); EXPECT_PTRNZ(1, _auxv); break;
> + case __LINE__:
> + return ret; /* must be last */
> + /* note: do not set any defaults so as to permit holes above */
> + }
> + }
> + return ret;
> +}
>
> Any more?
I quickly glanced over this but I tend to like it, indeed.
Thanks!
Willy
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 03/11] tools/nolibc: arm: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
2023-07-12 9:16 ` [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support Zhangjin Wu
2023-07-12 9:17 ` [PATCH v3 02/11] tools/nolibc: add new crt.h with _start_c Zhangjin Wu
@ 2023-07-12 9:18 ` Zhangjin Wu
2023-07-12 9:19 ` [PATCH v3 04/11] tools/nolibc: aarch64: " Zhangjin Wu
` (8 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:18 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-arm.h | 43 +++++----------------------------
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index ea723596ed23..74773ddcf4ca 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -8,6 +8,7 @@
#define _NOLIBC_ARCH_ARM_H
#include "compiler.h"
+#include "crt.h"
/* Syscalls for ARM in ARM or Thumb modes :
* - registers are 32-bit
@@ -183,49 +184,17 @@
_arg1; \
})
-
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code */
void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
- "bl __stack_chk_init\n" /* initialize stack protector */
+ "bl __stack_chk_init\n" /* initialize stack protector */
#endif
- "pop {%r0}\n" /* argc was in the stack */
- "mov %r1, %sp\n" /* argv = sp */
-
- "add %r2, %r0, $1\n" /* envp = (argc + 1) ... */
- "lsl %r2, %r2, $2\n" /* * 4 ... */
- "add %r2, %r2, %r1\n" /* + argv */
- "ldr %r3, 1f\n" /* r3 = &environ (see below) */
- "str %r2, [r3]\n" /* store envp into environ */
-
- "mov r4, r2\n" /* search for auxv (follows NULL after last env) */
- "0:\n"
- "mov r5, r4\n" /* r5 = r4 */
- "add r4, r4, #4\n" /* r4 += 4 */
- "ldr r5,[r5]\n" /* r5 = *r5 = *(r4-4) */
- "cmp r5, #0\n" /* and stop at NULL after last env */
- "bne 0b\n"
- "ldr %r3, 2f\n" /* r3 = &_auxv (low bits) */
- "str r4, [r3]\n" /* store r4 into _auxv */
-
- "mov %r3, $8\n" /* AAPCS : sp must be 8-byte aligned in the */
- "neg %r3, %r3\n" /* callee, and bl doesn't push (lr=pc) */
- "and %r3, %r3, %r1\n" /* so we do sp = r1(=sp) & r3(=-8); */
- "mov %sp, %r3\n"
-
- "bl main\n" /* main() returns the status code, we'll exit with it. */
- "movs r7, $1\n" /* NR_exit == 1 */
- "svc $0x00\n"
- ".align 2\n" /* below are the pointers to a few variables */
- "1:\n"
- ".word environ\n"
- "2:\n"
- ".word _auxv\n"
+ "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 */
);
__builtin_unreachable();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 04/11] tools/nolibc: aarch64: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (2 preceding siblings ...)
2023-07-12 9:18 ` [PATCH v3 03/11] tools/nolibc: arm: shrink _start " Zhangjin Wu
@ 2023-07-12 9:19 ` Zhangjin Wu
2023-07-12 9:20 ` [PATCH v3 05/11] tools/nolibc: i386: " Zhangjin Wu
` (7 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:19 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-aarch64.h | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index 1bf122cd5966..e52fa5a20d71 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -8,6 +8,7 @@
#define _NOLIBC_ARCH_AARCH64_H
#include "compiler.h"
+#include "crt.h"
/* Syscalls for AARCH64 :
* - registers are 64-bit
@@ -143,33 +144,16 @@
_arg1; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code */
void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
- "bl __stack_chk_init\n" /* initialize stack protector */
+ "bl __stack_chk_init\n" /* initialize stack protector */
#endif
- "ldr x0, [sp]\n" /* argc (x0) was in the stack */
- "add x1, sp, 8\n" /* argv (x1) = sp */
- "lsl x2, x0, 3\n" /* envp (x2) = 8*argc ... */
- "add x2, x2, 8\n" /* + 8 (skip null) */
- "add x2, x2, x1\n" /* + argv */
- "adrp x3, environ\n" /* x3 = &environ (high bits) */
- "str x2, [x3, #:lo12:environ]\n" /* store envp into environ */
- "mov x4, x2\n" /* search for auxv (follows NULL after last env) */
- "0:\n"
- "ldr x5, [x4], 8\n" /* x5 = *x4; x4 += 8 */
- "cbnz x5, 0b\n" /* and stop at NULL after last env */
- "adrp x3, _auxv\n" /* x3 = &_auxv (high bits) */
- "str x4, [x3, #:lo12:_auxv]\n" /* store x4 into _auxv */
- "and sp, x1, -16\n" /* sp must be 16-byte aligned in the callee */
- "bl main\n" /* main() returns the status code, we'll exit with it. */
- "mov x8, 93\n" /* NR_exit == 93 */
- "svc #0\n"
+ "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();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 05/11] tools/nolibc: i386: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (3 preceding siblings ...)
2023-07-12 9:19 ` [PATCH v3 04/11] tools/nolibc: aarch64: " Zhangjin Wu
@ 2023-07-12 9:20 ` Zhangjin Wu
2023-07-12 9:22 ` [PATCH v3 06/11] tools/nolibc: x86_64: " Zhangjin Wu
` (6 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:20 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-i386.h | 33 ++++++++------------------------
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index 1d4b683bc2cd..f0d0f5c364b8 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -8,6 +8,7 @@
#define _NOLIBC_ARCH_I386_H
#include "compiler.h"
+#include "crt.h"
/* Syscalls for i386 :
* - mostly similar to x86_64
@@ -154,9 +155,6 @@
_eax; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code */
/*
* i386 System V ABI mandates:
@@ -168,29 +166,14 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
- "call __stack_chk_init\n" /* initialize stack protector */
+ "call __stack_chk_init\n" /* initialize stack protector */
#endif
- "pop %eax\n" /* argc (first arg, %eax) */
- "mov %esp, %ebx\n" /* argv[] (second arg, %ebx) */
- "lea 4(%ebx,%eax,4),%ecx\n" /* then a NULL then envp (third arg, %ecx) */
- "mov %ecx, environ\n" /* save environ */
- "xor %ebp, %ebp\n" /* zero the stack frame */
- "mov %ecx, %edx\n" /* search for auxv (follows NULL after last env) */
- "0:\n"
- "add $4, %edx\n" /* search for auxv using edx, it follows the */
- "cmp -4(%edx), %ebp\n" /* ... NULL after last env (ebp is zero here) */
- "jnz 0b\n"
- "mov %edx, _auxv\n" /* save it into _auxv */
- "and $-16, %esp\n" /* x86 ABI : esp must be 16-byte aligned before */
- "sub $4, %esp\n" /* the call instruction (args are aligned) */
- "push %ecx\n" /* push all registers on the stack so that we */
- "push %ebx\n" /* support both regparm and plain stack modes */
- "push %eax\n"
- "call main\n" /* main() returns the status code in %eax */
- "mov %eax, %ebx\n" /* retrieve exit code (32-bit int) */
- "movl $1, %eax\n" /* NR_exit == 1 */
- "int $0x80\n" /* exit now */
- "hlt\n" /* ensure it does not */
+ "xor %ebp, %ebp\n" /* zero the stack frame */
+ "mov %esp, %eax\n" /* save stack pointer to %eax, as arg1 of _start_c */
+ "and $-16, %esp\n" /* last pushed argument must be 16-byte aligned */
+ "push %eax\n" /* push arg1 on stack to support plain stack modes too */
+ "call _start_c\n" /* transfer to c runtime */
+ "hlt\n" /* ensure it does not return */
);
__builtin_unreachable();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 06/11] tools/nolibc: x86_64: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (4 preceding siblings ...)
2023-07-12 9:20 ` [PATCH v3 05/11] tools/nolibc: i386: " Zhangjin Wu
@ 2023-07-12 9:22 ` Zhangjin Wu
2023-07-12 9:23 ` [PATCH v3 07/11] tools/nolibc: mips: " Zhangjin Wu
` (5 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:22 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-x86_64.h | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index e980ca5417d0..0048adf6b11f 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -8,6 +8,7 @@
#define _NOLIBC_ARCH_X86_64_H
#include "compiler.h"
+#include "crt.h"
/* Syscalls for x86_64 :
* - registers are 64-bit
@@ -153,9 +154,6 @@
_ret; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code */
/*
* x86-64 System V ABI mandates:
@@ -167,25 +165,13 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
- "call __stack_chk_init\n" /* initialize stack protector */
+ "call __stack_chk_init\n" /* initialize stack protector */
#endif
- "pop %rdi\n" /* argc (first arg, %rdi) */
- "mov %rsp, %rsi\n" /* argv[] (second arg, %rsi) */
- "lea 8(%rsi,%rdi,8),%rdx\n" /* then a NULL then envp (third arg, %rdx) */
- "mov %rdx, environ\n" /* save environ */
- "xor %ebp, %ebp\n" /* zero the stack frame */
- "mov %rdx, %rax\n" /* search for auxv (follows NULL after last env) */
- "0:\n"
- "add $8, %rax\n" /* search for auxv using rax, it follows the */
- "cmp -8(%rax), %rbp\n" /* ... NULL after last env (rbp is zero here) */
- "jnz 0b\n"
- "mov %rax, _auxv\n" /* save it into _auxv */
- "and $-16, %rsp\n" /* x86 ABI : esp must be 16-byte aligned before call */
- "call main\n" /* main() returns the status code, we'll exit with it. */
- "mov %eax, %edi\n" /* retrieve exit code (32 bit) */
- "mov $60, %eax\n" /* NR_exit == 60 */
- "syscall\n" /* really exit */
- "hlt\n" /* ensure it does not return */
+ "xor %ebp, %ebp\n" /* zero the stack frame */
+ "mov %rsp, %rdi\n" /* save stack pointer to %rdi, as arg1 of _start_c */
+ "and $-16, %rsp\n" /* %rsp must be 16-byte aligned before call */
+ "call _start_c\n" /* transfer to c runtime */
+ "hlt\n" /* ensure it does not return */
);
__builtin_unreachable();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 07/11] tools/nolibc: mips: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (5 preceding siblings ...)
2023-07-12 9:22 ` [PATCH v3 06/11] tools/nolibc: x86_64: " Zhangjin Wu
@ 2023-07-12 9:23 ` Zhangjin Wu
2023-07-12 9:24 ` [PATCH v3 08/11] tools/nolibc: loongarch: " Zhangjin Wu
` (4 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:23 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Also clean up the instructions in delayed slots.
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-mips.h | 46 +++++++-------------------------
1 file changed, 10 insertions(+), 36 deletions(-)
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index ae40d5268793..b1c070c5b24a 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -8,6 +8,7 @@
#define _NOLIBC_ARCH_MIPS_H
#include "compiler.h"
+#include "crt.h"
/* Syscalls for MIPS ABI O32 :
* - WARNING! there's always a delayed slot!
@@ -173,50 +174,23 @@
_arg4 ? -_num : _num; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code, note that it's called __start on MIPS */
void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector __start(void)
{
__asm__ volatile (
- /*".set nomips16\n"*/
".set push\n"
- ".set noreorder\n"
+ ".set noreorder\n"
".option pic0\n"
#ifdef _NOLIBC_STACKPROTECTOR
- "jal __stack_chk_init\n" /* initialize stack protector */
- "nop\n" /* delayed slot */
+ "jal __stack_chk_init\n" /* initialize stack protector */
+ " nop\n" /* delayed slot */
#endif
- /*".ent __start\n"*/
- /*"__start:\n"*/
- "lw $a0,($sp)\n" /* argc was in the stack */
- "addiu $a1, $sp, 4\n" /* argv = sp + 4 */
- "sll $a2, $a0, 2\n" /* a2 = argc * 4 */
- "add $a2, $a2, $a1\n" /* envp = argv + 4*argc ... */
- "addiu $a2, $a2, 4\n" /* ... + 4 */
- "lui $a3, %hi(environ)\n" /* load environ into a3 (hi) */
- "addiu $a3, %lo(environ)\n" /* load environ into a3 (lo) */
- "sw $a2,($a3)\n" /* store envp(a2) into environ */
-
- "move $t0, $a2\n" /* iterate t0 over envp, look for NULL */
- "0:" /* do { */
- "lw $a3, ($t0)\n" /* a3=*(t0); */
- "bne $a3, $0, 0b\n" /* } while (a3); */
- "addiu $t0, $t0, 4\n" /* delayed slot: t0+=4; */
- "lui $a3, %hi(_auxv)\n" /* load _auxv into a3 (hi) */
- "addiu $a3, %lo(_auxv)\n" /* load _auxv into a3 (lo) */
- "sw $t0, ($a3)\n" /* store t0 into _auxv */
-
- "li $t0, -8\n"
- "and $sp, $sp, $t0\n" /* sp must be 8-byte aligned */
- "addiu $sp,$sp,-16\n" /* the callee expects to save a0..a3 there! */
- "jal main\n" /* main() returns the status code, we'll exit with it. */
- "nop\n" /* delayed slot */
- "move $a0, $v0\n" /* retrieve 32-bit exit code from v0 */
- "li $v0, 4001\n" /* NR_exit == 4001 */
- "syscall\n"
- /*".end __start\n"*/
+ "move $a0, $sp\n" /* save stack pointer to $a0, as arg1 of _start_c */
+ "li $t0, -8\n"
+ "and $sp, $sp, $t0\n" /* $sp must be 8-byte aligned */
+ "addiu $sp, $sp, -16\n" /* the callee expects to save a0..a3 there */
+ "jal _start_c\n" /* transfer to c runtime */
+ " nop\n" /* delayed slot */
".set pop\n"
);
__builtin_unreachable();
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 08/11] tools/nolibc: loongarch: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (6 preceding siblings ...)
2023-07-12 9:23 ` [PATCH v3 07/11] tools/nolibc: mips: " Zhangjin Wu
@ 2023-07-12 9:24 ` Zhangjin Wu
2023-07-12 9:25 ` [PATCH v3 09/11] tools/nolibc: riscv: " Zhangjin Wu
` (3 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:24 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-loongarch.h | 43 ++++-----------------------
1 file changed, 5 insertions(+), 38 deletions(-)
diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
index 8aa7724fe38e..9db70d6f2c31 100644
--- a/tools/include/nolibc/arch-loongarch.h
+++ b/tools/include/nolibc/arch-loongarch.h
@@ -8,6 +8,7 @@
#define _NOLIBC_ARCH_LOONGARCH_H
#include "compiler.h"
+#include "crt.h"
/* Syscalls for LoongArch :
* - stack is 16-byte aligned
@@ -143,26 +144,9 @@
_arg1; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
#if __loongarch_grlen == 32
-#define LONGLOG "2"
-#define SZREG "4"
-#define REG_L "ld.w"
-#define LONG_S "st.w"
-#define LONG_ADD "add.w"
-#define LONG_ADDI "addi.w"
-#define LONG_SLL "slli.w"
#define LONG_BSTRINS "bstrins.w"
#else /* __loongarch_grlen == 64 */
-#define LONGLOG "3"
-#define SZREG "8"
-#define REG_L "ld.d"
-#define LONG_S "st.d"
-#define LONG_ADD "add.d"
-#define LONG_ADDI "addi.d"
-#define LONG_SLL "slli.d"
#define LONG_BSTRINS "bstrins.d"
#endif
@@ -171,28 +155,11 @@ void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_pr
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
- "bl __stack_chk_init\n" /* initialize stack protector */
+ "bl __stack_chk_init\n" /* initialize stack protector */
#endif
- REG_L " $a0, $sp, 0\n" /* argc (a0) was in the stack */
- LONG_ADDI " $a1, $sp, "SZREG"\n" /* argv (a1) = sp + SZREG */
- LONG_SLL " $a2, $a0, "LONGLOG"\n" /* envp (a2) = SZREG*argc ... */
- LONG_ADDI " $a2, $a2, "SZREG"\n" /* + SZREG (skip null) */
- LONG_ADD " $a2, $a2, $a1\n" /* + argv */
-
- "move $a3, $a2\n" /* iterate a3 over envp to find auxv (after NULL) */
- "0:\n" /* do { */
- REG_L " $a4, $a3, 0\n" /* a4 = *a3; */
- LONG_ADDI " $a3, $a3, "SZREG"\n" /* a3 += sizeof(void*); */
- "bne $a4, $zero, 0b\n" /* } while (a4); */
- "la.pcrel $a4, _auxv\n" /* a4 = &_auxv */
- LONG_S " $a3, $a4, 0\n" /* store a3 into _auxv */
-
- "la.pcrel $a3, environ\n" /* a3 = &environ */
- LONG_S " $a2, $a3, 0\n" /* store envp(a2) into environ */
- LONG_BSTRINS " $sp, $zero, 3, 0\n" /* sp must be 16-byte aligned */
- "bl main\n" /* main() returns the status code, we'll exit with it. */
- "li.w $a7, 93\n" /* NR_exit == 93 */
- "syscall 0\n"
+ "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();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 09/11] tools/nolibc: riscv: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (7 preceding siblings ...)
2023-07-12 9:24 ` [PATCH v3 08/11] tools/nolibc: loongarch: " Zhangjin Wu
@ 2023-07-12 9:25 ` Zhangjin Wu
2023-07-12 9:26 ` [PATCH v3 10/11] tools/nolibc: s390: " Zhangjin Wu
` (2 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:25 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-riscv.h | 43 +++++--------------------------
1 file changed, 6 insertions(+), 37 deletions(-)
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index 2b89ea59c5e4..2e3fcf925ae9 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -8,18 +8,7 @@
#define _NOLIBC_ARCH_RISCV_H
#include "compiler.h"
-
-#if __riscv_xlen == 64
-#define PTRLOG "3"
-#define SZREG "8"
-#define REG_L "ld"
-#define REG_S "sd"
-#elif __riscv_xlen == 32
-#define PTRLOG "2"
-#define SZREG "4"
-#define REG_L "lw"
-#define REG_S "sw"
-#endif
+#include "crt.h"
/* Syscalls for RISCV :
* - stack is 16-byte aligned
@@ -153,40 +142,20 @@
_arg1; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code */
void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
".option push\n"
".option norelax\n"
- "lla gp, __global_pointer$\n"
+ "lla gp, __global_pointer$\n"
".option pop\n"
#ifdef _NOLIBC_STACKPROTECTOR
- "call __stack_chk_init\n" /* initialize stack protector */
+ "call __stack_chk_init\n" /* initialize stack protector */
#endif
- REG_L" a0, 0(sp)\n" /* argc (a0) was in the stack */
- "add a1, sp, "SZREG"\n" /* argv (a1) = sp */
- "slli a2, a0, "PTRLOG"\n" /* envp (a2) = SZREG*argc ... */
- "add a2, a2, "SZREG"\n" /* + SZREG (skip null) */
- "add a2,a2,a1\n" /* + argv */
-
- "add a3, a2, zero\n" /* iterate a3 over envp to find auxv (after NULL) */
- "0:\n" /* do { */
- REG_L" a4, 0(a3)\n" /* a4 = *a3; */
- "add a3, a3, "SZREG"\n" /* a3 += sizeof(void*); */
- "bne a4, zero, 0b\n" /* } while (a4); */
- "lui a4, %hi(_auxv)\n" /* a4 = &_auxv (high bits) */
- REG_S" a3, %lo(_auxv)(a4)\n" /* store a3 into _auxv */
-
- "lui a3, %hi(environ)\n" /* a3 = &environ (high bits) */
- REG_S" a2,%lo(environ)(a3)\n"/* store envp(a2) into environ */
- "andi sp,a1,-16\n" /* sp must be 16-byte aligned */
- "call main\n" /* main() returns the status code, we'll exit with it. */
- "li a7, 93\n" /* NR_exit == 93 */
- "ecall\n"
+ "mv a0, sp\n" /* save stack pointer to a0, as arg1 of _start_c */
+ "andi sp, a0, -16\n" /* sp must be 16-byte aligned */
+ "call _start_c\n" /* transfer to c runtime */
);
__builtin_unreachable();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 10/11] tools/nolibc: s390: shrink _start with _start_c
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (8 preceding siblings ...)
2023-07-12 9:25 ` [PATCH v3 09/11] tools/nolibc: riscv: " Zhangjin Wu
@ 2023-07-12 9:26 ` Zhangjin Wu
2023-07-12 9:27 ` [PATCH v3 11/11] tools/nolibc: arch-*.h: add missing space after ',' Zhangjin Wu
2023-07-15 10:01 ` [PATCH v3 00/11] tools/nolibc: shrink arch support Willy Tarreau
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:26 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
move most of the _start operations to _start_c().
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-s390.h | 36 +++++---------------------------
1 file changed, 5 insertions(+), 31 deletions(-)
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index a40424ba043e..051f3f4ed19b 100644
--- a/tools/include/nolibc/arch-s390.h
+++ b/tools/include/nolibc/arch-s390.h
@@ -9,6 +9,7 @@
#include <asm/unistd.h>
#include "compiler.h"
+#include "crt.h"
/* Syscalls for s390:
* - registers are 64-bit
@@ -137,41 +138,14 @@
_arg1; \
})
-char **environ __attribute__((weak));
-const unsigned long *_auxv __attribute__((weak));
-
/* startup code */
void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
- "lg %r2,0(%r15)\n" /* argument count */
- "la %r3,8(%r15)\n" /* argument pointers */
-
- "xgr %r0,%r0\n" /* r0 will be our NULL value */
- /* search for envp */
- "lgr %r4,%r3\n" /* start at argv */
- "0:\n"
- "clg %r0,0(%r4)\n" /* entry zero? */
- "la %r4,8(%r4)\n" /* advance pointer */
- "jnz 0b\n" /* no -> test next pointer */
- /* yes -> r4 now contains start of envp */
- "larl %r1,environ\n"
- "stg %r4,0(%r1)\n"
-
- /* search for auxv */
- "lgr %r5,%r4\n" /* start at envp */
- "1:\n"
- "clg %r0,0(%r5)\n" /* entry zero? */
- "la %r5,8(%r5)\n" /* advance pointer */
- "jnz 1b\n" /* no -> test next pointer */
- "larl %r1,_auxv\n" /* yes -> store value in _auxv */
- "stg %r5,0(%r1)\n"
-
- "aghi %r15,-160\n" /* allocate new stackframe */
- "xc 0(8,%r15),0(%r15)\n" /* clear backchain */
- "brasl %r14,main\n" /* ret value of main is arg to exit */
- "lghi %r1,1\n" /* __NR_exit */
- "svc 0\n"
+ "lgr %r2, %r15\n" /* save stack pointer to %r2, as arg1 of _start_c */
+ "aghi %r15, -160\n" /* allocate new stackframe */
+ "xc 0(8,%r15), 0(%r15)\n" /* clear backchain */
+ "brasl %r14, _start_c\n" /* transfer to c runtime */
);
__builtin_unreachable();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 11/11] tools/nolibc: arch-*.h: add missing space after ','
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (9 preceding siblings ...)
2023-07-12 9:26 ` [PATCH v3 10/11] tools/nolibc: s390: " Zhangjin Wu
@ 2023-07-12 9:27 ` Zhangjin Wu
2023-07-15 10:01 ` [PATCH v3 00/11] tools/nolibc: shrink arch support Willy Tarreau
11 siblings, 0 replies; 27+ messages in thread
From: Zhangjin Wu @ 2023-07-12 9:27 UTC (permalink / raw)
To: w; +Cc: falcon, arnd, linux-kernel, linux-kselftest, thomas
Fix up such errors reported by scripts/checkpatch.pl:
ERROR: space required after that ',' (ctx:VxV)
#148: FILE: tools/include/nolibc/arch-aarch64.h:148:
+void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
^
ERROR: space required after that ',' (ctx:VxV)
#148: FILE: tools/include/nolibc/arch-aarch64.h:148:
+void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
^
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
tools/include/nolibc/arch-aarch64.h | 2 +-
tools/include/nolibc/arch-arm.h | 2 +-
tools/include/nolibc/arch-i386.h | 2 +-
tools/include/nolibc/arch-loongarch.h | 2 +-
tools/include/nolibc/arch-mips.h | 2 +-
tools/include/nolibc/arch-riscv.h | 2 +-
tools/include/nolibc/arch-s390.h | 2 +-
tools/include/nolibc/arch-x86_64.h | 2 +-
8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index e52fa5a20d71..c94fdca9ace6 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -145,7 +145,7 @@
})
/* startup code */
-void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index 74773ddcf4ca..5f8bfc24e9c7 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("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index f0d0f5c364b8..915f0e77629e 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("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
index 9db70d6f2c31..6edec94538e0 100644
--- a/tools/include/nolibc/arch-loongarch.h
+++ b/tools/include/nolibc/arch-loongarch.h
@@ -151,7 +151,7 @@
#endif
/* startup code */
-void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index b1c070c5b24a..36d4bf0c6aaa 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -175,7 +175,7 @@
})
/* startup code, note that it's called __start on MIPS */
-void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector __start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector __start(void)
{
__asm__ volatile (
".set push\n"
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index 2e3fcf925ae9..043e2fd85ab0 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -143,7 +143,7 @@
})
/* startup code */
-void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
".option push\n"
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index 051f3f4ed19b..705576d8fd15 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("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
"lgr %r2, %r15\n" /* save stack pointer to %r2, as arg1 of _start_c */
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index 0048adf6b11f..f5614a67f05a 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("omit-frame-pointer"))) __no_stack_protector _start(void)
+void __attribute__((weak, noreturn, optimize("omit-frame-pointer"))) __no_stack_protector _start(void)
{
__asm__ volatile (
#ifdef _NOLIBC_STACKPROTECTOR
--
2.25.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 00/11] tools/nolibc: shrink arch support
2023-07-12 9:15 [PATCH v3 00/11] tools/nolibc: shrink arch support Zhangjin Wu
` (10 preceding siblings ...)
2023-07-12 9:27 ` [PATCH v3 11/11] tools/nolibc: arch-*.h: add missing space after ',' Zhangjin Wu
@ 2023-07-15 10:01 ` Willy Tarreau
11 siblings, 0 replies; 27+ messages in thread
From: Willy Tarreau @ 2023-07-15 10:01 UTC (permalink / raw)
To: Zhangjin Wu; +Cc: arnd, linux-kernel, linux-kselftest, thomas
On Wed, Jul 12, 2023 at 05:15:28PM +0800, Zhangjin Wu wrote:
> Hi, Willy, Thomas
>
> Here is the revision of the v2 arch support shrink patchset [1], it
> mainly applies suggestions from you.
>
> It is based on the 20230710-nolibc-ser2-tom-syscall-configv4-report
> branch of nolibc repo.
>
> Tested for all of the supported archs:
>
> arch/board | result
> ------------|------------
> arm/versatilepb | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> arm/vexpress-a9 | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> arm/virt | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> aarch64/virt | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> i386/pc | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> x86_64/pc | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> mipsel/malta | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> loongarch64/virt | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> riscv64/virt | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
> riscv32/virt | 151 test(s): 122 passed, 7 skipped, 22 failed => status: failure.
> s390x/s390-ccw-virtio | 151 test(s): 144 passed, 7 skipped, 0 failed => status: warning.
Thanks for all this work. So I've checked a few random archs and trust
your tests above to confirm they're correct ;-) I'd just like to get
your confirmation regarding statx() support in 4.14 and 4.19, and likely
adjust _start_c() according to the last few tests. Also please do prepend
to the list the patch that adds the optimize("-Os") to fix _start, and we
should be good.
Thanks!
Willy
^ permalink raw reply [flat|nested] 27+ messages in thread