Hi, all
Thanks very much for your review suggestions of the v1 series [1], we just sent out the generic part1 [2], and here is the part2 of the whole v2 revision.
Changes from v1 -> v2:
* Don't emulate the return values in the new syscalls path, fix up or support the new syscalls in the side of the related test cases (1-3)
selftests/nolibc: remove gettimeofday_bad1/2 completely selftests/nolibc: support two errnos with EXPECT_SYSER2() selftests/nolibc: waitpid_min: add waitid syscall support
(Review suggestions from Willy and Thomas)
* Fix up new failure of the state_timestamps test case (4, new)
tools/nolibc: add missing nanoseconds support for __NR_statx
(Fixes for the commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
* Add new waitstatus macros as a standalone patch for the waitid support (5)
tools/nolibc: add more wait status related types
(Split and Cleanup for the waitid syscall based sys_wait4)
* Pure 64bit lseek and time64 select/poll/gettimeofday support (6-11)
tools/nolibc: add pure 64bit off_t, time_t and blkcnt_t tools/nolibc: sys_lseek: add pure 64bit lseek tools/nolibc: add pure 64bit time structs tools/nolibc: sys_select: add pure 64bit select tools/nolibc: sys_poll: add pure 64bit poll tools/nolibc: sys_gettimeofday: add pure 64bit gettimeofday
(Review suggestions from Arnd, Thomas and Willy, time32 variants have been removed completely and some fixups)
* waitid syscall support cleanup (12)
tools/nolibc: sys_wait4: add waitid syscall support
(Sync with the waitstatus macros update and Removal of emulated code)
* rv32 nolibc-test support, commit message update (13)
selftests/nolibc: riscv: customize makefile for rv32
(Review suggestions from Thomas, explain more about the change logic in commit message)
Best regards, Zhangjin ---
[1]: https://lore.kernel.org/linux-riscv/20230529113143.GB2762@1wt.eu/T/#t [2]: https://lore.kernel.org/linux-riscv/cover.1685362482.git.falcon@tinylab.org/
Zhangjin Wu (13): selftests/nolibc: remove gettimeofday_bad1/2 completely selftests/nolibc: support two errnos with EXPECT_SYSER2() selftests/nolibc: waitpid_min: add waitid syscall support tools/nolibc: add missing nanoseconds support for __NR_statx tools/nolibc: add more wait status related types tools/nolibc: add pure 64bit off_t, time_t and blkcnt_t tools/nolibc: sys_lseek: add pure 64bit lseek tools/nolibc: add pure 64bit time structs tools/nolibc: sys_select: add pure 64bit select tools/nolibc: sys_poll: add pure 64bit poll tools/nolibc: sys_gettimeofday: add pure 64bit gettimeofday tools/nolibc: sys_wait4: add waitid syscall support selftests/nolibc: riscv: customize makefile for rv32
tools/include/nolibc/arch-aarch64.h | 3 - tools/include/nolibc/arch-loongarch.h | 3 - tools/include/nolibc/arch-riscv.h | 3 - tools/include/nolibc/std.h | 28 ++-- tools/include/nolibc/sys.h | 134 +++++++++++++++---- tools/include/nolibc/types.h | 58 +++++++- tools/testing/selftests/nolibc/Makefile | 11 +- tools/testing/selftests/nolibc/nolibc-test.c | 20 +-- 8 files changed, 202 insertions(+), 58 deletions(-)
In the clock_gettime / clock_gettime64 syscalls based gettimeofday(), there is no way to let kernel space 'fixup' the invalid data pointer of 'struct timeval' and 'struct timezone' for us for we need to read timespec from kernel space and then convert to timeval in user-space ourselves and also we need to simply ignore and reset timezone in user-space.
Without this removal, the invalid (void *)1 address will trigger a sigsegv (signum = 11) signal and stop the whole test.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index a8fcad801cf2..7be2625f952d 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -582,10 +582,6 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break; -#ifdef NOLIBC - CASE_TEST(gettimeofday_bad1); EXPECT_SYSER(1, gettimeofday((void *)1, NULL), -1, EFAULT); break; - CASE_TEST(gettimeofday_bad2); EXPECT_SYSER(1, gettimeofday(NULL, (void *)1), -1, EFAULT); break; -#endif CASE_TEST(getpagesize); EXPECT_SYSZR(1, test_getpagesize()); break; CASE_TEST(ioctl_tiocinq); EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break; CASE_TEST(ioctl_tiocinq); EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break;
Some functions may be implemented with different syscalls in different platforms, these syscalls may set different errnos for the same arguments, let's support such cases.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 7be2625f952d..bf63fc66e486 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -300,18 +300,24 @@ static int expect_sysne(int expr, int llen, int val) }
+#define EXPECT_SYSER2(cond, expr, expret, experr1, experr2) \ + do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syserr2(expr, expret, experr1, experr2, llen); } while (0) + #define EXPECT_SYSER(cond, expr, expret, experr) \ - do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syserr(expr, expret, experr, llen); } while (0) + EXPECT_SYSER2(cond, expr, expret, experr, 0)
-static int expect_syserr(int expr, int expret, int experr, int llen) +static int expect_syserr2(int expr, int expret, int experr1, int experr2, int llen) { int ret = 0; int _errno = errno;
llen += printf(" = %d %s ", expr, errorname(_errno)); - if (expr != expret || _errno != experr) { + if (expr != expret || (_errno != experr1 && _errno != experr2)) { ret = 1; - llen += printf(" != (%d %s) ", expret, errorname(experr)); + if (experr2 == 0) + llen += printf(" != (%d %s) ", expret, errorname(experr1)); + else + llen += printf(" != (%d %s %s) ", expret, errorname(experr1), errorname(experr2)); llen += pad_spc(llen, 64, "[FAIL]\n"); } else { llen += pad_spc(llen, 64, " [OK]\n");
waitpid() is based on sys_wait4().
When the first argument is INT_MIN, the wait4 syscall based sys_wait4() return EFAULT by default, but the waitid syscall based sys_wait4() return EINVAL in rv32 platform, let's support such case.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index bf63fc66e486..8ba8c2fc71a0 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -616,7 +616,7 @@ int run_syscall(int min, int max) CASE_TEST(unlink_root); EXPECT_SYSER(1, unlink("/"), -1, EISDIR); break; CASE_TEST(unlink_blah); EXPECT_SYSER(1, unlink("/proc/self/blah"), -1, ENOENT); break; CASE_TEST(wait_child); EXPECT_SYSER(1, wait(&tmp), -1, ECHILD); break; - CASE_TEST(waitpid_min); EXPECT_SYSER(1, waitpid(INT_MIN, &tmp, WNOHANG), -1, ESRCH); break; + CASE_TEST(waitpid_min); EXPECT_SYSER2(1, waitpid(INT_MIN, &tmp, WNOHANG), -1, ESRCH, EINVAL); break; CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break; CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break; CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()") added nanoseconds for stat() but missed the statx case, this adds it.
The stx_atime, stx_mtime, stx_ctime are in type of 'struct statx_timestamp', which is incompatible with 'struct timespec', should convert explicitly.
/* include/uapi/linux/stat.h */
struct statx_timestamp { __s64 tv_sec; __u32 tv_nsec; __s32 __reserved; };
/* include/uapi/linux/time_types.h */ struct __kernel_timespec { __kernel_time64_t tv_sec; /* seconds */ long long tv_nsec; /* nanoseconds */ };
/* tools/include/nolibc/types.h */ #define timespec __kernel_timespec
Without this patch, the stat_timestamps test case would fail on rv32.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 154194056962..98cfa2f6d021 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -1175,9 +1175,9 @@ int sys_stat(const char *path, struct stat *buf) buf->st_size = statx.stx_size; buf->st_blksize = statx.stx_blksize; buf->st_blocks = statx.stx_blocks; - buf->st_atime = statx.stx_atime.tv_sec; - buf->st_mtime = statx.stx_mtime.tv_sec; - buf->st_ctime = statx.stx_ctime.tv_sec; + buf->st_atim = (struct timespec){ .tv_sec = statx.stx_atime.tv_sec, .tv_nsec = statx.stx_atime.tv_nsec }; + buf->st_mtim = (struct timespec){ .tv_sec = statx.stx_mtime.tv_sec, .tv_nsec = statx.stx_mtime.tv_nsec }; + buf->st_ctim = (struct timespec){ .tv_sec = statx.stx_ctime.tv_sec, .tv_nsec = statx.stx_ctime.tv_nsec }; return ret; } #else
On 2023-05-30 03:50:34+0800, Zhangjin Wu wrote:
Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()") added nanoseconds for stat() but missed the statx case, this adds it.
Welp, I should have thought of that. At least the testcase seems to have been useful.
Thanks for the fix!
The stx_atime, stx_mtime, stx_ctime are in type of 'struct statx_timestamp', which is incompatible with 'struct timespec', should convert explicitly.
/* include/uapi/linux/stat.h */ struct statx_timestamp { __s64 tv_sec; __u32 tv_nsec; __s32 __reserved; }; /* include/uapi/linux/time_types.h */ struct __kernel_timespec { __kernel_time64_t tv_sec; /* seconds */ long long tv_nsec; /* nanoseconds */ }; /* tools/include/nolibc/types.h */ #define timespec __kernel_timespecWithout this patch, the stat_timestamps test case would fail on rv32.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/include/nolibc/sys.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 154194056962..98cfa2f6d021 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -1175,9 +1175,9 @@ int sys_stat(const char *path, struct stat *buf) buf->st_size = statx.stx_size; buf->st_blksize = statx.stx_blksize; buf->st_blocks = statx.stx_blocks;
- buf->st_atime = statx.stx_atime.tv_sec;
- buf->st_mtime = statx.stx_mtime.tv_sec;
- buf->st_ctime = statx.stx_ctime.tv_sec;
- buf->st_atim = (struct timespec){ .tv_sec = statx.stx_atime.tv_sec, .tv_nsec = statx.stx_atime.tv_nsec };
- buf->st_mtim = (struct timespec){ .tv_sec = statx.stx_mtime.tv_sec, .tv_nsec = statx.stx_mtime.tv_nsec };
- buf->st_ctim = (struct timespec){ .tv_sec = statx.stx_ctime.tv_sec, .tv_nsec = statx.stx_ctime.tv_nsec };
I would prefer to split the compound assignment into two single assignments, though.
buf->st_ctim.tv_sec = statx.stx_ctime.tv_sec; buf->st_ctim.tv_nsec = statx.stx_ctime.tv_nsec;
return ret; }
#else
2.25.1
Thomas, Arnd, Willy
On 2023-05-30 03:50:34+0800, Zhangjin Wu wrote:
Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()") added nanoseconds for stat() but missed the statx case, this adds it.
Welp, I should have thought of that. At least the testcase seems to have been useful.
yeah, your testcase telled me this issue.
Thanks for the fix!
The stx_atime, stx_mtime, stx_ctime are in type of 'struct statx_timestamp', which is incompatible with 'struct timespec', should convert explicitly.
/* include/uapi/linux/stat.h */ struct statx_timestamp { __s64 tv_sec; __u32 tv_nsec; __s32 __reserved; }; /* include/uapi/linux/time_types.h */ struct __kernel_timespec { __kernel_time64_t tv_sec; /* seconds */ long long tv_nsec; /* nanoseconds */ }; /* tools/include/nolibc/types.h */ #define timespec __kernel_timespecWithout this patch, the stat_timestamps test case would fail on rv32.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/include/nolibc/sys.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 154194056962..98cfa2f6d021 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -1175,9 +1175,9 @@ int sys_stat(const char *path, struct stat *buf) buf->st_size = statx.stx_size; buf->st_blksize = statx.stx_blksize; buf->st_blocks = statx.stx_blocks;
- buf->st_atime = statx.stx_atime.tv_sec;
- buf->st_mtime = statx.stx_mtime.tv_sec;
- buf->st_ctime = statx.stx_ctime.tv_sec;
- buf->st_atim = (struct timespec){ .tv_sec = statx.stx_atime.tv_sec, .tv_nsec = statx.stx_atime.tv_nsec };
- buf->st_mtim = (struct timespec){ .tv_sec = statx.stx_mtime.tv_sec, .tv_nsec = statx.stx_mtime.tv_nsec };
- buf->st_ctim = (struct timespec){ .tv_sec = statx.stx_ctime.tv_sec, .tv_nsec = statx.stx_ctime.tv_nsec };
I would prefer to split the compound assignment into two single assignments, though.
buf->st_ctim.tv_sec = statx.stx_ctime.tv_sec; buf->st_ctim.tv_nsec = statx.stx_ctime.tv_nsec;
Ok, will update it in the v3 revision.
And further, what about removing the other !statx parts (__NR_newfstatat, __NR_stat)? just like we are doing for the other 64bit syscalls (llseek and time65).
Best regards, Zhangjin
return ret; } #else
More wait status related types are added for the coming waitid syscall based wait4() support.
Resue the ones from <linux/wait.h>, add the missing ones from sys/wait.h and bits/waitstatus.h of glibc.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/types.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h index f96e28bff4ba..698d859fc6e2 100644 --- a/tools/include/nolibc/types.h +++ b/tools/include/nolibc/types.h @@ -10,6 +10,7 @@ #include "std.h" #include <linux/time.h> #include <linux/stat.h> +#include <linux/wait.h>
/* Only the generic macros and types may be defined here. The arch-specific @@ -91,9 +92,13 @@ #define WIFEXITED(status) (((status) & 0x7f) == 0) #define WTERMSIG(status) ((status) & 0x7f) #define WIFSIGNALED(status) ((status) - 1 < 0xff) +#define WIFSTOPPED(status) (((status) & 0xff) == 0x7f)
-/* waitpid() flags */ -#define WNOHANG 1 +/* Macros for constructing status values. */ +#define W_EXITCODE(ret, sig) ((ret) << 8 | (sig)) +#define W_STOPCODE(sig) ((sig) << 8 | 0x7f) +#define W_CONTINUED 0xffff +#define WCOREFLAG 0x80
/* standard exit() codes */ #define EXIT_SUCCESS 0
clean up std.h with include/uapi/linux/posix_types.h
convert time_t to 64bit even in 32bit platforms, for y2038 issue.
align off_t and blkcnt_t with 'struct statx' in include/uapi/linux/stat.h
Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/83ab9f47-e1ed-463c-a717-26aad6bf2b71@app... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/std.h | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/tools/include/nolibc/std.h b/tools/include/nolibc/std.h index 933bc0be7e1c..0f458c1c24de 100644 --- a/tools/include/nolibc/std.h +++ b/tools/include/nolibc/std.h @@ -20,17 +20,21 @@
#include "stdint.h"
-/* those are commonly provided by sys/types.h */ -typedef unsigned int dev_t; -typedef unsigned long ino_t; -typedef unsigned int mode_t; -typedef signed int pid_t; -typedef unsigned int uid_t; -typedef unsigned int gid_t; -typedef unsigned long nlink_t; -typedef signed long off_t; -typedef signed long blksize_t; -typedef signed long blkcnt_t; -typedef signed long time_t; +#include <linux/posix_types.h> + +/* based on linux/types.h */ +typedef uint32_t __kernel_dev_t; + +typedef __kernel_dev_t dev_t; +typedef __kernel_ulong_t ino_t; +typedef __kernel_mode_t mode_t; +typedef __kernel_pid_t pid_t; +typedef __kernel_uid32_t uid_t; +typedef __kernel_gid32_t gid_t; +typedef __kernel_loff_t off_t; +typedef __kernel_time64_t time_t; +typedef uint32_t nlink_t; +typedef uint32_t blksize_t; +typedef uint64_t blkcnt_t;
#endif /* _NOLIBC_STD_H */
use sys_llseek instead of sys_lseek to add 64bit seek even in 32bit platforms.
This code is based on sysdeps/unix/sysv/linux/lseek.c of glibc and src/unistd/lseek.c of musl.
Signed-off-by: Zhangjin Wu falcon@tinylab.org Signed-off-by: Willy Tarreau w@1wt.eu --- tools/include/nolibc/sys.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 98cfa2f6d021..d0720af84b6d 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -672,7 +672,17 @@ int link(const char *old, const char *new) static __attribute__((unused)) off_t sys_lseek(int fd, off_t offset, int whence) { +#if defined(__NR_llseek) || defined(__NR__llseek) +#ifndef __NR__llseek +#define __NR__llseek __NR_llseek +#endif + off_t result; + return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, whence) ?: result; +#elif defined(__NR_lseek) return my_syscall3(__NR_lseek, fd, offset, whence); +#else +#error None of __NR_lseek, __NR_llseek nor __NR__llseek defined, cannot implement sys_lseek() +#endif }
static __attribute__((unused))
On Mon, May 29, 2023, at 21:54, Zhangjin Wu wrote:
use sys_llseek instead of sys_lseek to add 64bit seek even in 32bit platforms.
This code is based on sysdeps/unix/sysv/linux/lseek.c of glibc and src/unistd/lseek.c of musl.
Signed-off-by: Zhangjin Wu falcon@tinylab.org Signed-off-by: Willy Tarreau w@1wt.eu
tools/include/nolibc/sys.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 98cfa2f6d021..d0720af84b6d 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -672,7 +672,17 @@ int link(const char *old, const char *new) static __attribute__((unused)) off_t sys_lseek(int fd, off_t offset, int whence) { +#if defined(__NR_llseek) || defined(__NR__llseek) +#ifndef __NR__llseek +#define __NR__llseek __NR_llseek +#endif
- off_t result;
- return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result,
whence) ?: result; +#elif defined(__NR_lseek) return my_syscall3(__NR_lseek, fd, offset, whence); +#else +#error None of __NR_lseek, __NR_llseek nor __NR__llseek defined, cannot implement sys_lseek() +#endif }
This is not technically wrong, but I think a different approach would be clearer: Instead of having a sys_lseek() that works differently depending on the macros, why not define the low-level helpers to match the kernel arguments like
static inline __attribute__((unused)) __kernel_loff_t sys_lseek(int fd, __kernel_loff_t offset, int whence) { #ifdef __NR__llseek __kernel_loff_t result; return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, whence) ?: result; #else
#endif }
static inline __attribute__((unused)) __kernel_off_t sys_lseek(int fd, __kernel_off_t offset, int whence) { #ifdef __NR_lseek return my_syscall3(__NR_lseek, fd, offset, whence); #else return -ENOSYS; #endif }
And then do the selection inside of the actual lseek, something like
static __attribute__((unused)) off_t lseek(int fd, off_t offset, int whence) { off_t ret = -ENOSYS;
if (BITS_PER_LONG == 32) ret = sys_llseek(fd, offset, whence);
if (ret == -ENOSYS) ret = sys_lseek(fd, offset, whence);
if (ret < 0) { SET_ERRNO(-ret); ret = -1; } return ret;
}
For the loff_t selection, there is no real need to handle the fallback, so this could just be an if()/else to select 32-bit or 64-bit, but for the time_t ones the fallback is required for pre-5.6 kernels.
Arnd
Hi, Arnd, Willy
On Mon, May 29, 2023, at 21:54, Zhangjin Wu wrote:
use sys_llseek instead of sys_lseek to add 64bit seek even in 32bit platforms.
This code is based on sysdeps/unix/sysv/linux/lseek.c of glibc and src/unistd/lseek.c of musl.
Signed-off-by: Zhangjin Wu falcon@tinylab.org Signed-off-by: Willy Tarreau w@1wt.eu
tools/include/nolibc/sys.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 98cfa2f6d021..d0720af84b6d 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -672,7 +672,17 @@ int link(const char *old, const char *new) static __attribute__((unused)) off_t sys_lseek(int fd, off_t offset, int whence) { +#if defined(__NR_llseek) || defined(__NR__llseek) +#ifndef __NR__llseek +#define __NR__llseek __NR_llseek +#endif
- off_t result;
- return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result,
whence) ?: result; +#elif defined(__NR_lseek) return my_syscall3(__NR_lseek, fd, offset, whence); +#else +#error None of __NR_lseek, __NR_llseek nor __NR__llseek defined, cannot implement sys_lseek() +#endif }
This is not technically wrong, but I think a different approach would be clearer: Instead of having a sys_lseek() that works differently depending on the macros, why not define the low-level helpers to match the kernel arguments like
static inline __attribute__((unused)) __kernel_loff_t sys_lseek(int fd, __kernel_loff_t offset, int whence) { #ifdef __NR__llseek __kernel_loff_t result; return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, whence) ?: result; #else #endif }
static inline __attribute__((unused)) __kernel_off_t sys_lseek(int fd, __kernel_off_t offset, int whence) { #ifdef __NR_lseek return my_syscall3(__NR_lseek, fd, offset, whence); #else return -ENOSYS; #endif }
And then do the selection inside of the actual lseek, something like
static __attribute__((unused)) off_t lseek(int fd, off_t offset, int whence) { off_t ret = -ENOSYS;
if (BITS_PER_LONG == 32) ret = sys_llseek(fd, offset, whence); if (ret == -ENOSYS) ret = sys_lseek(fd, offset, whence); if (ret < 0) { SET_ERRNO(-ret); ret = -1; } return ret;}
Yes, It is clearer, thanks. will learn carefully about the kernel types.
For the loff_t selection, there is no real need to handle the fallback, so this could just be an if()/else to select 32-bit or 64-bit, but for the time_t ones the fallback is required for pre-5.6 kernels.
Ok, will test it on the pre-5.6 versions too.
Hi, Willy, what's your suggestion about the oldest kernel versions we plan to support? ;-)
Best regards, Zhangjin
Arnd
Hi Zhangjin, Arnd,
On Tue, May 30, 2023 at 09:54:33PM +0800, Zhangjin Wu wrote:
And then do the selection inside of the actual lseek, something like
static __attribute__((unused)) off_t lseek(int fd, off_t offset, int whence) { off_t ret = -ENOSYS;
if (BITS_PER_LONG == 32) ret = sys_llseek(fd, offset, whence); if (ret == -ENOSYS) ret = sys_lseek(fd, offset, whence); if (ret < 0) { SET_ERRNO(-ret); ret = -1; } return ret;}
Yes, It is clearer, thanks. will learn carefully about the kernel types.
I, too, like Arnd's proposal here. I tend to use a similar approach in other projects when possible. Often the limit is the types definition, which is necessary to define even empty static inline functions. The only thing is that due to the reliance on -ENOSYS above, the compiler cannot fully optimize the code away, particularly when both syscalls are defined, which may result in the compiler emitting the code for both calls on 32-bit platforms. But the idea is there anyway, and it may possibly just need a few adjustments based on BITS_PER_LONG after checking the emitted code.
For the loff_t selection, there is no real need to handle the fallback, so this could just be an if()/else to select 32-bit or 64-bit, but for the time_t ones the fallback is required for pre-5.6 kernels.
Ok, will test it on the pre-5.6 versions too.
Hi, Willy, what's your suggestion about the oldest kernel versions we plan to support? ;-)
Like I said last time, since the code is included in the kernel, we expect userland developers to use this one to build their code, even if it's meant to work on older kernels. At the very least I want that supported kernels continue to work, and then as long as it does not require particular efforts, it's nice to continue to work on older ones (think LTS distros, late upgraders of legacy systems etc).
Thanks, Willy
It's time to provide 64bit time structs for all platforms, for y2038 is near.
There are still old "struct timeval" and "struct itimerval" in include/uapi/linux/time.h, remove "#include <linux/time.h>" and add our own pure 64bit ones.
Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/9e4064fc-f0c5-4dd3-941f-344d2150e1cd@app... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 2 -- tools/include/nolibc/types.h | 49 +++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index d0720af84b6d..1b3675d4c5fc 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -17,7 +17,6 @@ #include <asm/mman.h> #include <linux/fs.h> #include <linux/loop.h> -#include <linux/time.h> #include <linux/auxvec.h> #include <linux/fcntl.h> /* for O_* and AT_* */ #include <linux/stat.h> /* for statx() */ @@ -28,7 +27,6 @@ #include "errno.h" #include "types.h"
- /* Functions in this file only describe syscalls. They're declared static so * that the compiler usually decides to inline them while still being allowed * to pass a pointer to one of their instances. Each syscall exists in two diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h index 698d859fc6e2..4ff35b7ea2bb 100644 --- a/tools/include/nolibc/types.h +++ b/tools/include/nolibc/types.h @@ -8,10 +8,57 @@ #define _NOLIBC_TYPES_H
#include "std.h" -#include <linux/time.h> +#include <linux/time_types.h> #include <linux/stat.h> #include <linux/wait.h>
+/* based on linux/time.h but with pure 64bit time structs */ +#define timespec __kernel_timespec +#define itimerspec __kernel_itimerspec + +/* timeval is only provided for users, not compatible with syscalls */ +struct timeval { + __kernel_time64_t tv_sec; /* seconds */ + __s64 tv_usec; /* microseconds */ +}; + +struct timezone { + int tz_minuteswest; /* minutes west of Greenwich */ + int tz_dsttime; /* type of dst correction */ +}; + +/* itimerval is only provided for users, not compatible with syscalls */ +struct itimerval { + struct timeval it_interval; /* timer interval */ + struct timeval it_value; /* current value */ +}; + +/* + * Names of the interval timers, and structure + * defining a timer setting: + */ +#define ITIMER_REAL 0 +#define ITIMER_VIRTUAL 1 +#define ITIMER_PROF 2 + +/* + * The IDs of the various system clocks (for POSIX.1b interval timers): + */ +#define CLOCK_REALTIME 0 +#define CLOCK_MONOTONIC 1 +#define CLOCK_PROCESS_CPUTIME_ID 2 +#define CLOCK_THREAD_CPUTIME_ID 3 +#define CLOCK_MONOTONIC_RAW 4 +#define CLOCK_REALTIME_COARSE 5 +#define CLOCK_MONOTONIC_COARSE 6 +#define CLOCK_BOOTTIME 7 +#define CLOCK_REALTIME_ALARM 8 +#define CLOCK_BOOTTIME_ALARM 9 + +/* + * The various flags for setting POSIX.1b interval timers: + */ +#define TIMER_ABSTIME 0x01
/* 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
It's time to provide 64bit time structs for all platforms, for y2038 is near.
pselect6_time64 has been added from v4.20 and the last arch support is at least from v5.0.0.
Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/76a5f9a0-eec4-415a-9c5d-ac3bca4d4b0e@t-8... Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/bf3e07c1-75f5-425b-9124-f3f2b230e63a@app... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/arch-aarch64.h | 3 --- tools/include/nolibc/arch-loongarch.h | 3 --- tools/include/nolibc/arch-riscv.h | 3 --- tools/include/nolibc/sys.h | 25 ++++++++++--------------- 4 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h index 11f294a406b7..53255a3a326f 100644 --- a/tools/include/nolibc/arch-aarch64.h +++ b/tools/include/nolibc/arch-aarch64.h @@ -47,10 +47,7 @@ struct sys_stat_struct { * - the arguments are cast to long and assigned into the target registers * which are then simply passed as registers to the asm code, so that we * don't have to experience issues with register constraints. - * - * On aarch64, select() is not implemented so we have to use pselect6(). */ -#define __ARCH_WANT_SYS_PSELECT6
#define my_syscall0(num) \ ({ \ diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h index ad3f266e7093..973fdca96651 100644 --- a/tools/include/nolibc/arch-loongarch.h +++ b/tools/include/nolibc/arch-loongarch.h @@ -18,10 +18,7 @@ * - the arguments are cast to long and assigned into the target * registers which are then simply passed as registers to the asm code, * so that we don't have to experience issues with register constraints. - * - * On LoongArch, select() is not implemented so we have to use pselect6(). */ -#define __ARCH_WANT_SYS_PSELECT6
#define my_syscall0(num) \ ({ \ diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h index a2e8564e66d6..713eb9d2c91d 100644 --- a/tools/include/nolibc/arch-riscv.h +++ b/tools/include/nolibc/arch-riscv.h @@ -53,10 +53,7 @@ struct sys_stat_struct { * - the arguments are cast to long and assigned into the target * registers which are then simply passed as registers to the asm code, * so that we don't have to experience issues with register constraints. - * - * On riscv, select() is not implemented so we have to use pselect6(). */ -#define __ARCH_WANT_SYS_PSELECT6
#define my_syscall0(num) \ ({ \ diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 1b3675d4c5fc..db648b5b9a1c 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -1046,28 +1046,23 @@ int sched_yield(void) static __attribute__((unused)) int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeval *timeout) { -#if defined(__ARCH_WANT_SYS_OLD_SELECT) && !defined(__NR__newselect) - struct sel_arg_struct { - unsigned long n; - fd_set *r, *w, *e; - struct timeval *t; - } arg = { .n = nfds, .r = rfds, .w = wfds, .e = efds, .t = timeout }; - return my_syscall1(__NR_select, &arg); -#elif defined(__ARCH_WANT_SYS_PSELECT6) && defined(__NR_pselect6) +#if defined(__NR_pselect6) || defined(__NR_pselect6_time64) +#ifdef __NR_pselect6_time64 + const long nr_pselect = __NR_pselect6_time64; +#elif __SIZEOF_LONG__ == 8 + const long nr_pselect = __NR_pselect6; +#else +#error No __NR_pselect6_time64 defined, cannot implement time64 sys_select() +#endif struct timespec t;
if (timeout) { t.tv_sec = timeout->tv_sec; t.tv_nsec = timeout->tv_usec * 1000; } - return my_syscall6(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL); -#elif defined(__NR__newselect) || defined(__NR_select) -#ifndef __NR__newselect -#define __NR__newselect __NR_select -#endif - return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout); + return my_syscall6(nr_pselect, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL); #else -#error None of __NR_select, __NR_pselect6, nor __NR__newselect defined, cannot implement sys_select() +#error Neither __NR_pselect6 nor __NR_pselect6_time64 defined, cannot implement sys_select() #endif }
It's time to provide 64bit time structs for all platforms, for y2038 is near.
ppoll_time64 has been added from v4.20 and the last arch support is at least from v5.0.0
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index db648b5b9a1c..ca802627e88f 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -940,14 +940,21 @@ int pivot_root(const char *new, const char *old) static __attribute__((unused)) int sys_poll(struct pollfd *fds, int nfds, int timeout) { -#if defined(__NR_ppoll) +#if defined(__NR_ppoll) || defined(__NR_ppoll_time64) +#ifdef __NR_ppoll_time64 + const long nr_ppoll = __NR_ppoll_time64; +#elif __SIZEOF_LONG__ == 8 + const long nr_ppoll = __NR_ppoll; +#else +#error No __NR_ppoll_time64 defined, cannot implement time64 sys_poll() +#endif struct timespec t;
if (timeout >= 0) { t.tv_sec = timeout / 1000; t.tv_nsec = (timeout % 1000) * 1000000; } - return my_syscall5(__NR_ppoll, fds, nfds, (timeout >= 0) ? &t : NULL, NULL, 0); + return my_syscall5(nr_ppoll, fds, nfds, (timeout >= 0) ? &t : NULL, NULL, 0); #elif defined(__NR_poll) return my_syscall3(__NR_poll, fds, nfds, timeout); #else
It's time to provide 64bit time structs for all platforms, for y2038 is near.
clock_gettime64 has been added from at least v5.0.0.
Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/afc4944f-9494-4367-906d-06ac47648ab7@app... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index ca802627e88f..533233094733 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -25,6 +25,7 @@
#include "arch.h" #include "errno.h" +#include "string.h" #include "types.h"
/* Functions in this file only describe syscalls. They're declared static so @@ -552,7 +553,34 @@ long getpagesize(void) static __attribute__((unused)) int sys_gettimeofday(struct timeval *tv, struct timezone *tz) { - return my_syscall2(__NR_gettimeofday, tv, tz); +#if defined(__NR_clock_gettime) || defined(__NR_clock_gettime64) +#ifdef __NR_clock_gettime64 + const long nr_clock_gettime = __NR_clock_gettime64; +#elif __SIZEOF_LONG__ == 8 + const long nr_clock_gettime = __NR_clock_gettime; +#else +#error No __NR_clock_gettime64 defined, cannot implement time64 sys_gettimeofday() +#endif + struct timespec ts; + int ret; + + /* set tz to zero to avoid random number */ + if (tz != NULL) + memset(tz, 0, sizeof(struct timezone)); + + if (tv == NULL) + return 0; + + ret = my_syscall2(nr_clock_gettime, CLOCK_REALTIME, &ts); + if (ret) + return ret; + + tv->tv_sec = ts.tv_sec; + tv->tv_usec = (unsigned int)ts.tv_nsec / 1000; + return 0; +#else +#error Neither __NR_clock_gettime nor __NR_clock_gettime64 defined, cannot implement sys_gettimeofday() +#endif }
static __attribute__((unused))
rv32 uses the generic include/uapi/asm-generic/unistd.h and it has no __NR_wait4 after kernel commit d4c08b9776b3 ("riscv: Use latest system call ABI"), use __NR_waitid instead.
This code is based on sysdeps/unix/sysv/linux/wait4.c of glibc.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 533233094733..a32b90b1fd3b 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -12,6 +12,7 @@
/* system includes */ #include <asm/unistd.h> +#include <asm/siginfo.h> /* for siginfo_t */ #include <asm/signal.h> /* for SIGCHLD */ #include <asm/ioctls.h> #include <asm/mman.h> @@ -1373,7 +1374,56 @@ int unlink(const char *path) static __attribute__((unused)) pid_t sys_wait4(pid_t pid, int *status, int options, struct rusage *rusage) { +#ifdef __NR_wait4 return my_syscall4(__NR_wait4, pid, status, options, rusage); +#elif defined(__NR_waitid) + siginfo_t infop; + int idtype = P_PID; + int ret; + + if (pid < -1) { + idtype = P_PGID; + pid *= -1; + } else if (pid == -1) { + idtype = P_ALL; + } else if (pid == 0) { + idtype = P_PGID; + } + + options |= WEXITED; + + ret = my_syscall5(__NR_waitid, idtype, pid, &infop, options, rusage); + if (ret < 0) + return ret; + + if (status) { + switch (infop.si_code) { + case CLD_EXITED: + *status = W_EXITCODE(infop.si_status, 0); + break; + case CLD_DUMPED: + *status = WCOREFLAG | infop.si_status; + break; + case CLD_KILLED: + *status = infop.si_status; + break; + case CLD_TRAPPED: + case CLD_STOPPED: + *status = W_STOPCODE(infop.si_status); + break; + case CLD_CONTINUED: + *status = W_CONTINUED; + break; + default: + *status = 0; + break; + } + } + + return infop.si_pid; +#else +#error Neither __NR_wait4 nor __NR_waitid defined, cannot implement sys_wait4() +#endif }
static __attribute__((unused))
Both riscv64 and riscv32 have:
* the same ARCH value, it is riscv * the same arch/riscv source code tree
The only differences are:
* riscv64 uses defconfig, riscv32 uses rv32_defconfig * riscv64 uses qemu-system-riscv64, riscv32 uses qemu-system-riscv32 * riscv32 has different compiler options (-march= and -mabi=)
So, riscv32 can share most of the settings with riscv64, there is no need to add it as a whole new architecture but just need a flag to record and reflect the difference.
The 32bit mips and loongarch may be able to use the same method, so, let's use a meaningful flag: CONFIG_32BIT. If required in the future, this flag can also be automatically loaded from include/config/auto.conf.
With this patch, it is able to run nolibc test for rv32 like this:
$ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/Makefile | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 44088535682e..ea434a0acdc1 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -14,6 +14,12 @@ include $(srctree)/scripts/subarch.include ARCH = $(SUBARCH) endif
+# Allow pass ARCH=riscv|riscv32|riscv64, riscv implies riscv64 +ifneq ($(findstring xriscv,x$(ARCH)),) + CONFIG_32BIT := $(if $(findstring 32x,$(ARCH)x),1) + override ARCH := riscv +endif + # kernel image names by architecture IMAGE_i386 = arch/x86/boot/bzImage IMAGE_x86_64 = arch/x86/boot/bzImage @@ -34,7 +40,7 @@ DEFCONFIG_x86 = defconfig DEFCONFIG_arm64 = defconfig DEFCONFIG_arm = multi_v7_defconfig DEFCONFIG_mips = malta_defconfig -DEFCONFIG_riscv = defconfig +DEFCONFIG_riscv = $(if $(CONFIG_32BIT),rv32_defconfig,defconfig) DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig DEFCONFIG = $(DEFCONFIG_$(ARCH)) @@ -49,7 +55,7 @@ QEMU_ARCH_x86 = x86_64 QEMU_ARCH_arm64 = aarch64 QEMU_ARCH_arm = arm QEMU_ARCH_mips = mipsel # works with malta_defconfig -QEMU_ARCH_riscv = riscv64 +QEMU_ARCH_riscv = $(if $(CONFIG_32BIT),riscv32,riscv64) QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 QEMU_ARCH = $(QEMU_ARCH_$(ARCH)) @@ -76,6 +82,7 @@ else Q=@ endif
+CFLAGS_riscv = $(if $(CONFIG_32BIT),-march=rv32i -mabi=ilp32) CFLAGS_s390 = -m64 CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \
Willy, Arnd and Thomas
Based on your suggestions, in the comming v3, I plan to split the whole rv32 support to something like this:
1. Generic part1
(The old feedbacks are applied with the new Suggested-by lines, welcome your additional feedbacks if there are ;-))
selftests/nolibc: syscall_args: use generic __NR_statx tools/nolibc: add missing nanoseconds support for __NR_statx selftests/nolibc: allow specify extra arguments for qemu selftests/nolibc: fix up compile warning with glibc on x86_64 selftests/nolibc: not include limits.h for nolibc selftests/nolibc: use INT_MAX instead of __INT_MAX__ tools/nolibc: arm: add missing my_syscall6 tools/nolibc: open: fix up compile warning for arm selftests/nolibc: support two errnos with EXPECT_SYSER2() selftests/nolibc: remove gettimeofday_bad1/2 completely selftests/nolibc: add new gettimeofday test cases
2. Add Compile support for rv32
(Convert all of the unsupported syscalls to a return of -ENOSYS, this allows us to fix up the test failures one by one not that urgently later)
tools/nolibc: fix up #error compile failures with -ENOSYS tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS selftests/nolibc: riscv: customize makefile for rv32
(The first two are new but clear enough, based on the idea of suggestion from Arnd [1])
3. Fix up the left test failures one by one
(Plan to add everyone as a standalone patchset, which will easier the review and merge progress)
wait4 -> waitid lseek -> llseek gettimeofday -> clock_gettime/clock_gettime64 select -> pselect6/pselect6_time64 ppoll -> ppoll_time64
4. Clean up some old test cases one by one
Like statx ...
Best regards, Zhangjin
[1]: https://lore.kernel.org/linux-riscv/5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app...
Both riscv64 and riscv32 have:
- the same ARCH value, it is riscv
- the same arch/riscv source code tree
The only differences are:
- riscv64 uses defconfig, riscv32 uses rv32_defconfig
- riscv64 uses qemu-system-riscv64, riscv32 uses qemu-system-riscv32
- riscv32 has different compiler options (-march= and -mabi=)
So, riscv32 can share most of the settings with riscv64, there is no need to add it as a whole new architecture but just need a flag to record and reflect the difference.
The 32bit mips and loongarch may be able to use the same method, so, let's use a meaningful flag: CONFIG_32BIT. If required in the future, this flag can also be automatically loaded from include/config/auto.conf.
With this patch, it is able to run nolibc test for rv32 like this:
$ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/Makefile | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 44088535682e..ea434a0acdc1 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -14,6 +14,12 @@ include $(srctree)/scripts/subarch.include ARCH = $(SUBARCH) endif +# Allow pass ARCH=riscv|riscv32|riscv64, riscv implies riscv64 +ifneq ($(findstring xriscv,x$(ARCH)),)
- CONFIG_32BIT := $(if $(findstring 32x,$(ARCH)x),1)
- override ARCH := riscv
+endif
# kernel image names by architecture IMAGE_i386 = arch/x86/boot/bzImage IMAGE_x86_64 = arch/x86/boot/bzImage @@ -34,7 +40,7 @@ DEFCONFIG_x86 = defconfig DEFCONFIG_arm64 = defconfig DEFCONFIG_arm = multi_v7_defconfig DEFCONFIG_mips = malta_defconfig -DEFCONFIG_riscv = defconfig +DEFCONFIG_riscv = $(if $(CONFIG_32BIT),rv32_defconfig,defconfig) DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig DEFCONFIG = $(DEFCONFIG_$(ARCH)) @@ -49,7 +55,7 @@ QEMU_ARCH_x86 = x86_64 QEMU_ARCH_arm64 = aarch64 QEMU_ARCH_arm = arm QEMU_ARCH_mips = mipsel # works with malta_defconfig -QEMU_ARCH_riscv = riscv64 +QEMU_ARCH_riscv = $(if $(CONFIG_32BIT),riscv32,riscv64) QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 QEMU_ARCH = $(QEMU_ARCH_$(ARCH)) @@ -76,6 +82,7 @@ else Q=@ endif +CFLAGS_riscv = $(if $(CONFIG_32BIT),-march=rv32i -mabi=ilp32) CFLAGS_s390 = -m64 CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \ -- 2.25.1
On 2023-06-02 12:06:25+0800, Zhangjin Wu wrote:
Willy, Arnd and Thomas
Based on your suggestions, in the comming v3, I plan to split the whole rv32 support to something like this:
Is each of these parts a new patchset? I would suggest to do so.
Generic part1
(The old feedbacks are applied with the new Suggested-by lines, welcome your additional feedbacks if there are ;-))
selftests/nolibc: syscall_args: use generic __NR_statx tools/nolibc: add missing nanoseconds support for __NR_statx selftests/nolibc: allow specify extra arguments for qemu selftests/nolibc: fix up compile warning with glibc on x86_64 selftests/nolibc: not include limits.h for nolibc selftests/nolibc: use INT_MAX instead of __INT_MAX__ tools/nolibc: arm: add missing my_syscall6 tools/nolibc: open: fix up compile warning for arm selftests/nolibc: support two errnos with EXPECT_SYSER2() selftests/nolibc: remove gettimeofday_bad1/2 completely selftests/nolibc: add new gettimeofday test cases
These all look good and non-controversial.
Add Compile support for rv32
(Convert all of the unsupported syscalls to a return of -ENOSYS, this allows us to fix up the test failures one by one not that urgently later)
tools/nolibc: fix up #error compile failures with -ENOSYS tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
These should be their own series in my opinion. It will likely generate some discussion.
selftests/nolibc: riscv: customize makefile for rv32(The first two are new but clear enough, based on the idea of suggestion from Arnd [1])
- Fix up the left test failures one by one
I'm not a fan of adding an "official" rv32 support with still failing tests.
(Plan to add everyone as a standalone patchset, which will easier the review and merge progress)
wait4 -> waitid lseek -> llseek gettimeofday -> clock_gettime/clock_gettime64 select -> pselect6/pselect6_time64 ppoll -> ppoll_time64
I guess these new codepaths will also be used on non-rv32 architectures and will therefore validated without rv32.
So you could submit these before the final rv32 patch in a series.
Clean up some old test cases one by one
Like statx ...
Best regards, Zhangjin
Both riscv64 and riscv32 have:
- the same ARCH value, it is riscv
- the same arch/riscv source code tree
The only differences are:
- riscv64 uses defconfig, riscv32 uses rv32_defconfig
- riscv64 uses qemu-system-riscv64, riscv32 uses qemu-system-riscv32
- riscv32 has different compiler options (-march= and -mabi=)
So, riscv32 can share most of the settings with riscv64, there is no need to add it as a whole new architecture but just need a flag to record and reflect the difference.
The 32bit mips and loongarch may be able to use the same method, so, let's use a meaningful flag: CONFIG_32BIT. If required in the future, this flag can also be automatically loaded from include/config/auto.conf.
With this patch, it is able to run nolibc test for rv32 like this:
$ make run ARCH=riscv32 CROSS_COMPILE=riscv64-linux-gnu- ...Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/Makefile | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 44088535682e..ea434a0acdc1 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -14,6 +14,12 @@ include $(srctree)/scripts/subarch.include ARCH = $(SUBARCH) endif +# Allow pass ARCH=riscv|riscv32|riscv64, riscv implies riscv64 +ifneq ($(findstring xriscv,x$(ARCH)),)
- CONFIG_32BIT := $(if $(findstring 32x,$(ARCH)x),1)
- override ARCH := riscv
+endif
# kernel image names by architecture IMAGE_i386 = arch/x86/boot/bzImage IMAGE_x86_64 = arch/x86/boot/bzImage @@ -34,7 +40,7 @@ DEFCONFIG_x86 = defconfig DEFCONFIG_arm64 = defconfig DEFCONFIG_arm = multi_v7_defconfig DEFCONFIG_mips = malta_defconfig -DEFCONFIG_riscv = defconfig +DEFCONFIG_riscv = $(if $(CONFIG_32BIT),rv32_defconfig,defconfig) DEFCONFIG_s390 = defconfig DEFCONFIG_loongarch = defconfig DEFCONFIG = $(DEFCONFIG_$(ARCH)) @@ -49,7 +55,7 @@ QEMU_ARCH_x86 = x86_64 QEMU_ARCH_arm64 = aarch64 QEMU_ARCH_arm = arm QEMU_ARCH_mips = mipsel # works with malta_defconfig -QEMU_ARCH_riscv = riscv64 +QEMU_ARCH_riscv = $(if $(CONFIG_32BIT),riscv32,riscv64) QEMU_ARCH_s390 = s390x QEMU_ARCH_loongarch = loongarch64 QEMU_ARCH = $(QEMU_ARCH_$(ARCH)) @@ -76,6 +82,7 @@ else Q=@ endif +CFLAGS_riscv = $(if $(CONFIG_32BIT),-march=rv32i -mabi=ilp32) CFLAGS_s390 = -m64 CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \ -- 2.25.1
On 2023-06-02 12:06:25+0800, Zhangjin Wu wrote:
Willy, Arnd and Thomas
Based on your suggestions, in the comming v3, I plan to split the whole rv32 support to something like this:
Is each of these parts a new patchset?
Yeah, It is also my plan, just like the v2 series.
I would suggest to do so.
Generic part1
(The old feedbacks are applied with the new Suggested-by lines, welcome your additional feedbacks if there are ;-))
selftests/nolibc: syscall_args: use generic __NR_statx tools/nolibc: add missing nanoseconds support for __NR_statx selftests/nolibc: allow specify extra arguments for qemu selftests/nolibc: fix up compile warning with glibc on x86_64 selftests/nolibc: not include limits.h for nolibc selftests/nolibc: use INT_MAX instead of __INT_MAX__ tools/nolibc: arm: add missing my_syscall6 tools/nolibc: open: fix up compile warning for arm selftests/nolibc: support two errnos with EXPECT_SYSER2() selftests/nolibc: remove gettimeofday_bad1/2 completely selftests/nolibc: add new gettimeofday test cases
These all look good and non-controversial.
Add Compile support for rv32
(Convert all of the unsupported syscalls to a return of -ENOSYS, this allows us to fix up the test failures one by one not that urgently later)
tools/nolibc: fix up #error compile failures with -ENOSYS tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS
These should be their own series in my opinion. It will likely generate some discussion.
The 1st one is not rv32 specific, but the 2nd one requires rv32 compile support to be validated.
selftests/nolibc: riscv: customize makefile for rv32(The first two are new but clear enough, based on the idea of suggestion from Arnd [1])
- Fix up the left test failures one by one
I'm not a fan of adding an "official" rv32 support with still failing tests.
That is reasonable, but in another side, without the rv32 compile support, It may be a little hard to test the left patchsets (see below explain).
The other reasons for rv32 compile support is:
* Some people may use nolibc without the left syscalls. * It is able to detect the new test failures.
But anyway, the compile support is not urgent.
(Plan to add everyone as a standalone patchset, which will easier the review and merge progress)
wait4 -> waitid lseek -> llseek gettimeofday -> clock_gettime/clock_gettime64 select -> pselect6/pselect6_time64 ppoll -> ppoll_time64
I guess these new codepaths will also be used on non-rv32 architectures and will therefore validated without rv32.
Unfortunately, most of them are time32 syscalls related (except the llseek), rv32 is the first architecture who has no kernel side time32 syscalls support, that's why I plan to add compile support at first ;-)
If the new time64 syscalls will be added as the first 'branch', then, they will be validated on the other 32bit architecture, but some of them may be not added as the first 'branch', for example, the waitid() emulated wait4() is bigger than the original one.
So you could submit these before the final rv32 patch in a series.
Thanks for your suggestion.
I'm working on cleaning up them independently and carefully, will send them out as standalone patchsets.
Best regards, Zhangjin
Clean up some old test cases one by one
Like statx ...
Best regards, Zhangjin
Hi, Willy
These two patches are based on part2 of support for rv32 [1], I have forgotten to send them together.
Best regards, Zhangjin --- [1]: https://lore.kernel.org/linux-riscv/cover.1685387484.git.falcon@tinylab.org/
Zhangjin Wu (2): selftests/nolibc: add new gettimeofday test cases selftests/nolibc: add sizeof test for the new 64bit data types
tools/testing/selftests/nolibc/nolibc-test.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) { + struct timeval tv; + struct timezone tz; struct stat stat_buf; int euid0; int proc; @@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break; + CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break; + CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break; + CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break; CASE_TEST(getpagesize); EXPECT_SYSZR(1, test_getpagesize()); break; CASE_TEST(ioctl_tiocinq); EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break; CASE_TEST(ioctl_tiocinq); EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break;
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) {
- struct timeval tv;
- struct timezone tz; struct stat stat_buf; int euid0; int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
Calling gettimeofday(NULL, ...) will actually segfault on glibc. It works when calling through the VDSO, but not the logic in glibc itself, which is guess is allowed by POSIX.
I propose to avoid doing it :-)
Either we gate the existing test in #ifdef NOLIBC or we remove it.
CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;-- 2.25.1
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) {
- struct timeval tv;
- struct timezone tz; struct stat stat_buf; int euid0; int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;Calling gettimeofday(NULL, ...) will actually segfault on glibc. It works when calling through the VDSO, but not the logic in glibc itself, which is guess is allowed by POSIX.
In low level gettimeofday syscall, it should be ok, will check the non-vdso code of glibc later. in musl, it returns 0 when it is NULL (src/time/gettimeofday.c).
I propose to avoid doing it :-)
Just checked it again, these two cases can also detect the issues (1):
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break; CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
(1) nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
we can directly remove the second one ;-)
Either we gate the existing test in #ifdef NOLIBC or we remove it.
so, we still need to check the original gettimeofday_null test case for glibc?
Thanks, Zhangjin
CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
On 2023-05-30 19:28:06+0800, Zhangjin Wu wrote:
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) {
- struct timeval tv;
- struct timezone tz; struct stat stat_buf; int euid0; int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;Calling gettimeofday(NULL, ...) will actually segfault on glibc. It works when calling through the VDSO, but not the logic in glibc itself, which is guess is allowed by POSIX.
In low level gettimeofday syscall, it should be ok, will check the non-vdso code of glibc later. in musl, it returns 0 when it is NULL (src/time/gettimeofday.c).
It's fine for the Linux syscall, vdso, musl and nolibc. It's not fine for glibc and gnulib.
I propose to avoid doing it :-)
Just checked it again, these two cases can also detect the issues (1):
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break; CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break; (1) nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
To be honest this does not look like a nolibc issue, more a compiler setup problem. GCC generates calls to a function in libgcc that is not linked for some reason.
Can you provide reproduction steps?
Also I guess a testcase that tests at runtime that the compilation has worked is of limited use :-) It either works or it never executes.
we can directly remove the second one ;-)
Either we gate the existing test in #ifdef NOLIBC or we remove it.
so, we still need to check the original gettimeofday_null test case for glibc?
The original testcase also needs adaption, yes.
Thanks, Zhangjin
CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
On Tue, May 30, 2023 at 12:59:31PM +0200, Thomas Weißschuh wrote:
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) {
- struct timeval tv;
- struct timezone tz; struct stat stat_buf; int euid0; int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;Calling gettimeofday(NULL, ...) will actually segfault on glibc. It works when calling through the VDSO, but not the logic in glibc itself, which is guess is allowed by POSIX.
Then that's shocking, because the man page says:
If either tv or tz is NULL, the corresponding structure is not set or returned. (However, compilation warnings will result if tv is NULL.)
I'd expect glibc to at least support what'd documented in the man page :-/
I propose to avoid doing it :-)
If you're certain that's the case, then I agree.
Either we gate the existing test in #ifdef NOLIBC or we remove it.
Better not keep tests specific to nolibc if they aim at verifying some compatibliity.
Willy
On Mai 30 2023, Willy Tarreau wrote:
On Tue, May 30, 2023 at 12:59:31PM +0200, Thomas Weißschuh wrote:
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) {
- struct timeval tv;
- struct timezone tz; struct stat stat_buf; int euid0; int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;Calling gettimeofday(NULL, ...) will actually segfault on glibc. It works when calling through the VDSO, but not the logic in glibc itself, which is guess is allowed by POSIX.
Then that's shocking, because the man page says:
If either tv or tz is NULL, the corresponding structure is not set or returned. (However, compilation warnings will result if tv is NULL.)I'd expect glibc to at least support what'd documented in the man page :-/
The manual page is not part of glibc. Neither the glibc documentation nor the POSIX spec has ever supported NULL for the first argument. The generic POSIX implementation in glibc originally returned EINVAL for that case, though.
On 2023-05-30 14:05:00+0200, Willy Tarreau wrote:
On Tue, May 30, 2023 at 12:59:31PM +0200, Thomas Weißschuh wrote:
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
These 3 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'This issue happens while there is no "unsigned int" conversion in the new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 8ba8c2fc71a0..20d184da9a2b 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -533,6 +533,8 @@ static int test_stat_timestamps(void) */ int run_syscall(int min, int max) {
- struct timeval tv;
- struct timezone tz; struct stat stat_buf; int euid0; int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max) CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break; CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break; CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
CASE_TEST(gettimeofday_tv); EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;CASE_TEST(gettimeofday_tz); EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;Calling gettimeofday(NULL, ...) will actually segfault on glibc. It works when calling through the VDSO, but not the logic in glibc itself, which is guess is allowed by POSIX.
Then that's shocking, because the man page says:
If either tv or tz is NULL, the corresponding structure is not set or returned. (However, compilation warnings will result if tv is NULL.)I'd expect glibc to at least support what'd documented in the man page :-/
That is gettimeofday(2), which comes from the Linux man-pages project.
gettimeofday(3p) which specifies the posix function does not have that wording. It also specifies that there is no way to indicate errors...
Seems to be a weird situation.
I propose to avoid doing it :-)
If you're certain that's the case, then I agree.
Either we gate the existing test in #ifdef NOLIBC or we remove it.
Better not keep tests specific to nolibc if they aim at verifying some compatibliity.
Thomas
These test cases are required to make sure the new added data types are really 64bit based.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 20d184da9a2b..43ce4d34b596 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -721,6 +721,14 @@ int run_stdlib(int min, int max) #else # warning "__SIZEOF_LONG__ is undefined" #endif /* __SIZEOF_LONG__ */ + CASE_TEST(sizeof_time_t); EXPECT_EQ(1, 8, sizeof(time_t)); break; + CASE_TEST(sizeof_timespec); EXPECT_EQ(1, 16, sizeof(struct timespec)); break; +#ifdef NOLIBC + CASE_TEST(sizeof_itimerspec); EXPECT_EQ(1, 32, sizeof(struct itimerspec)); break; +#endif + CASE_TEST(sizeof_timeval); EXPECT_EQ(1, 16, sizeof(struct timeval)); break; + CASE_TEST(sizeof_itimerval); EXPECT_EQ(1, 32, sizeof(struct itimerval)); break; + CASE_TEST(sizeof_off_t); EXPECT_EQ(1, 8, sizeof(off_t)); break; case __LINE__: return ret; /* must be last */ /* note: do not set any defaults so as to permit holes above */
On 2023-05-30 14:42:56+0800, Zhangjin Wu wrote:
These test cases are required to make sure the new added data types are really 64bit based.
Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 20d184da9a2b..43ce4d34b596 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -721,6 +721,14 @@ int run_stdlib(int min, int max) #else # warning "__SIZEOF_LONG__ is undefined" #endif /* __SIZEOF_LONG__ */
CASE_TEST(sizeof_time_t); EXPECT_EQ(1, 8, sizeof(time_t)); break;CASE_TEST(sizeof_timespec); EXPECT_EQ(1, 16, sizeof(struct timespec)); break;+#ifdef NOLIBC
CASE_TEST(sizeof_itimerspec); EXPECT_EQ(1, 32, sizeof(struct itimerspec)); break;+#endif
CASE_TEST(sizeof_timeval); EXPECT_EQ(1, 16, sizeof(struct timeval)); break;CASE_TEST(sizeof_itimerval); EXPECT_EQ(1, 32, sizeof(struct itimerval)); break;CASE_TEST(sizeof_off_t); EXPECT_EQ(1, 8, sizeof(off_t)); break;
These will break on 32bit glibc configurations. (At least on x86)
case __LINE__: return ret; /* must be last */ /* note: do not set any defaults so as to permit holes above */-- 2.25.1
On 2023-05-30 14:42:56+0800, Zhangjin Wu wrote:
These test cases are required to make sure the new added data types are really 64bit based.
Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 20d184da9a2b..43ce4d34b596 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -721,6 +721,14 @@ int run_stdlib(int min, int max) #else # warning "__SIZEOF_LONG__ is undefined" #endif /* __SIZEOF_LONG__ */
CASE_TEST(sizeof_time_t); EXPECT_EQ(1, 8, sizeof(time_t)); break;CASE_TEST(sizeof_timespec); EXPECT_EQ(1, 16, sizeof(struct timespec)); break;+#ifdef NOLIBC
CASE_TEST(sizeof_itimerspec); EXPECT_EQ(1, 32, sizeof(struct itimerspec)); break;+#endif
CASE_TEST(sizeof_timeval); EXPECT_EQ(1, 16, sizeof(struct timeval)); break;CASE_TEST(sizeof_itimerval); EXPECT_EQ(1, 32, sizeof(struct itimerval)); break;CASE_TEST(sizeof_off_t); EXPECT_EQ(1, 8, sizeof(off_t)); break;These will break on 32bit glibc configurations. (At least on x86)
Yes, I added a big #ifdef at first, but narrowed it down after a default x86_64 gcc+glibc test, 32bit has been ignored from my mind ;-(
Will add the big #ifdef back.
Thanks, Zhangjin
case __LINE__: return ret; /* must be last */ /* note: do not set any defaults so as to permit holes above */--
Hi Zhangjin,
On Tue, May 30, 2023 at 03:45:23AM +0800, Zhangjin Wu wrote:
Hi, all
Thanks very much for your review suggestions of the v1 series [1], we just sent out the generic part1 [2], and here is the part2 of the whole v2 revision.
(...)
Just to let you know, I'm now done with my release and will slowly try to catch up with all your series!
Stay tuned! Willy
linux-kselftest-mirror@lists.linaro.org
-
Andreas Schwab -
Arnd Bergmann -
Thomas Weißschuh -
Willy Tarreau -
Zhangjin Wu