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/[email protected]/T/#t
[2]: https://lore.kernel.org/linux-riscv/[email protected]/
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(-)
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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 */
--
2.25.1
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 <[email protected]>
---
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
--
2.25.1
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 <[email protected]>
---
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;
--
2.25.1
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 <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
---
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))
--
2.25.1
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 <[email protected]>
---
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
--
2.25.1
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 <[email protected]>
---
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;
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Suggested-by: Arnd Bergmann <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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
}
--
2.25.1
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 <[email protected]>
---
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
--
2.25.1
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 <[email protected]>
---
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");
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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))
--
2.25.1
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 <[email protected]>
---
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))
--
2.25.1
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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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
--
2.25.1
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 <[email protected]>
---
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-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_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 <[email protected]>
> ---
> 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_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 <[email protected]>
> > ---
> > 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
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/[email protected]/
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(+)
--
2.25.1
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 <[email protected]>
---
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;
--
2.25.1
These test cases are required to make sure the new added data types are
really 64bit based.
Signed-off-by: Zhangjin Wu <[email protected]>
---
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 */
--
2.25.1
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 <[email protected]>
> Signed-off-by: Willy Tarreau <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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: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 <[email protected]>
> ---
> 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;
> 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;
> --
> 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 <[email protected]>
> > ---
> > 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;
> > 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: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 <[email protected]>
> > ---
> > 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 */
> > --
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 <[email protected]>
> > > ---
> > > 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;
> > > 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 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>> > ---
>> > 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.
--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
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 <[email protected]>
> > > ---
> > > 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
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 <[email protected]>
> > Signed-off-by: Willy Tarreau <[email protected]>
> > ---
> > 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
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/[email protected]/
> 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 <[email protected]>
> ---
> 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.
> 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
These all look good and non-controversial.
> 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
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])
>
> 3. 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.
> 4. Clean up some old test cases one by one
>
> Like statx ...
>
> Best regards,
> Zhangjin
>
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/
>
> > 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 <[email protected]>
> > ---
> > 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.
>
> > 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
>
> These all look good and non-controversial.
>
> > 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
>
> 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])
> >
> > 3. 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
> > 4. Clean up some old test cases one by one
> >
> > Like statx ...
> >
> > Best regards,
> > Zhangjin
> >
> > [1]: https://lore.kernel.org/linux-riscv/[email protected]/
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
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