Hi, Willy
This is the v3 generic part1 for rv32, all of the found issues of v2 part1 [1] have been fixed up, several generic patches have been fixed up and merged from v2 part2 [2] to this series, the standalone test_fork patch [4] is merged with a Reviewed-by line into this series too.
This series is based on 20230528-nolibc-rv32+stkp5 branch of [5].
Changes from v2 -> v3:
* selftests/nolibc: fix up compile warning with glibc on x86_64
Use simpler 'long long' conversion instead of old #ifdef ... (Suggestion from Willy)
* tools/nolibc: add missing nanoseconds support for __NR_statx
Split the compound assignment into two single assignments (Suggestion from Thomas)
* selftests/nolibc: add new gettimeofday test cases
Removed the gettimeofday(NULL, &tz) (Suggestion from Thomas)
All of the commit messages have been re-checked, some missing Suggested-by lines are added.
The whole patchset have been tested on arm, aarch64, rv32 and rv64, no regressions (the next compile patchset is required to do rv32 test).
The nolibc-test has been tested with glibc on x86_64 too.
Btw, we have found such poll failures on arm (not introduced by this patchset), this will be fixed in our coming ppoll_time64 patchset:
48 poll_null = -1 ENOSYS [FAIL] 49 poll_stdout = -1 ENOSYS [FAIL] 50 poll_fault = -1 ENOSYS != (-1 EFAULT) [FAIL]
And the gettimeofday_null removal patch from Thomas [3] may conflicts with the gettimeofday removal and addition patches, but it is not hard to fix.
Best regards, Zhangjin ---
[1]: https://lore.kernel.org/linux-riscv/cover.1685362482.git.falcon@tinylab.org/... [2]: https://lore.kernel.org/linux-riscv/cover.1685387484.git.falcon@tinylab.org/... [3]: https://lore.kernel.org/lkml/20230530-nolibc-gettimeofday-v1-1-7307441a002b@... [4]: https://lore.kernel.org/lkml/61bdfe7bacebdef8aa9195f6f2550a5b0d33aab3.168542... [5]: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git
Zhangjin Wu (12): 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 selftests/nolibc: test_fork: fix up duplicated print
tools/include/nolibc/arch-arm.h | 23 +++++++++++ tools/include/nolibc/stdint.h | 14 +++++++ tools/include/nolibc/sys.h | 39 +++++++++--------- tools/testing/selftests/nolibc/Makefile | 2 +- tools/testing/selftests/nolibc/nolibc-test.c | 42 ++++++++++++-------- 5 files changed, 85 insertions(+), 35 deletions(-)
Compiling nolibc-test.c for rv32 got such error:
tools/testing/selftests/nolibc/nolibc-test.c:599:57: error: ‘__NR_fstat’ undeclared (first use in this function) 599 | CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT); break;
The generic include/uapi/asm-generic/unistd.h used by rv32 doesn't support __NR_fstat, use the more generic __NR_statx instead:
Running test 'syscall' 69 syscall_noargs = 1 [OK] 70 syscall_args = -1 EFAULT [OK]
__NR_statx has been added from v4.10:
commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")
It has been supported by all of the platforms since at least from v4.20.
Fixes: 8e3ab529bef9 ("tools/nolibc/unistd: add syscall()") Suggested-by: Arnd Bergmann arnd@arndb.de Link: https://lore.kernel.org/linux-riscv/ee8b1f02-ded1-488b-a3a5-68774f0349b5@app... 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 7de46305f419..d417ca5d976f 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -621,7 +621,7 @@ int run_syscall(int min, int max) 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; CASE_TEST(syscall_noargs); EXPECT_SYSEQ(1, syscall(__NR_getpid), getpid()); break; - CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT); break; + CASE_TEST(syscall_args); EXPECT_SYSER(1, syscall(__NR_statx, 0, NULL, 0, 0, NULL), -1, EFAULT); break; case __LINE__: return ret; /* must be last */ /* note: do not set any defaults so as to permit holes above */
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.h */ struct timespec { __kernel_old_time_t tv_sec; /* seconds */ long tv_nsec; /* nanoseconds */ };
Without this patch, the stat_timestamps test case would fail when __NR_statx defined.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 1d6f33f58629..0160605444e7 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -1161,23 +1161,26 @@ int sys_stat(const char *path, struct stat *buf) long ret;
ret = sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &statx); - buf->st_dev = ((statx.stx_dev_minor & 0xff) - | (statx.stx_dev_major << 8) - | ((statx.stx_dev_minor & ~0xff) << 12)); - buf->st_ino = statx.stx_ino; - buf->st_mode = statx.stx_mode; - buf->st_nlink = statx.stx_nlink; - buf->st_uid = statx.stx_uid; - buf->st_gid = statx.stx_gid; - buf->st_rdev = ((statx.stx_rdev_minor & 0xff) - | (statx.stx_rdev_major << 8) - | ((statx.stx_rdev_minor & ~0xff) << 12)); - 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_dev = ((statx.stx_dev_minor & 0xff) + | (statx.stx_dev_major << 8) + | ((statx.stx_dev_minor & ~0xff) << 12)); + buf->st_ino = statx.stx_ino; + buf->st_mode = statx.stx_mode; + buf->st_nlink = statx.stx_nlink; + buf->st_uid = statx.stx_uid; + buf->st_gid = statx.stx_gid; + buf->st_rdev = ((statx.stx_rdev_minor & 0xff) + | (statx.stx_rdev_major << 8) + | ((statx.stx_rdev_minor & ~0xff) << 12)); + buf->st_size = statx.stx_size; + buf->st_blksize = statx.stx_blksize; + buf->st_blocks = statx.stx_blocks; + buf->st_atim.tv_sec = statx.stx_atime.tv_sec; + buf->st_atim.tv_nsec = statx.stx_atime.tv_nsec; + buf->st_mtim.tv_sec = statx.stx_mtime.tv_sec; + buf->st_mtim.tv_nsec = statx.stx_mtime.tv_nsec; + buf->st_ctim.tv_sec = statx.stx_ctime.tv_sec; + buf->st_ctim.tv_nsec = statx.stx_ctime.tv_nsec; return ret; } #else
On Sat, Jun 03, 2023 at 04:02:04PM +0800, Zhangjin Wu wrote:
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.h */ struct timespec { __kernel_old_time_t tv_sec; /* seconds */ long tv_nsec; /* nanoseconds */ };
Without this patch, the stat_timestamps test case would fail when __NR_statx defined.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org
Thank you. I've queued it immediately after Thomas' patch. I'll let the two of you tell me if it's better to squash them together to avoid breaking bisect and mark you co-authors.
Willy
On 2023-06-04 13:18:35+0200, Willy Tarreau wrote:
On Sat, Jun 03, 2023 at 04:02:04PM +0800, Zhangjin Wu wrote:
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.h */ struct timespec { __kernel_old_time_t tv_sec; /* seconds */ long tv_nsec; /* nanoseconds */ };
Without this patch, the stat_timestamps test case would fail when __NR_statx defined.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org
Thank you. I've queued it immediately after Thomas' patch. I'll let the two of you tell me if it's better to squash them together to avoid breaking bisect and mark you co-authors.
Squashing them sounds like the correct solution to me.
Thomas
On Sun, Jun 04, 2023 at 02:00:02PM +0200, Thomas Weißschuh wrote:
On 2023-06-04 13:18:35+0200, Willy Tarreau wrote:
On Sat, Jun 03, 2023 at 04:02:04PM +0800, Zhangjin Wu wrote:
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.h */ struct timespec { __kernel_old_time_t tv_sec; /* seconds */ long tv_nsec; /* nanoseconds */ };
Without this patch, the stat_timestamps test case would fail when __NR_statx defined.
Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()") Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org
Thank you. I've queued it immediately after Thomas' patch. I'll let the two of you tell me if it's better to squash them together to avoid breaking bisect and mark you co-authors.
Squashing them sounds like the correct solution to me.
OK I've done it for now in my branch. I'm going to push it as 20230604-nolibc-rv32+stkp6. All tests pass fine again for me now on all supported archs. I'll pass this one to Paul, I think it's fine for 6.5. I just don't know if he still has tests planned on his side for 6.5 (Paul always re-runs the whole tests after integration and often spots failures).
By the way, I'm still using my test-all script that's extremely convenient to test the expected results from user-mode (it basically does what run-user does, but for all archs and at -O0, -Os, -O3).
I'm sharing it attached since I think it can help you and Zhangjin in your respective tests. That's how I'm cheating to spot build issues in contributed changes. I have not committed it because it's ugly and I don't know where to put it, but I think you'll find it convenient nevertheless. I'm starting it like this:
$ ./test-all-opts.sh | tee test16.out $ grep passed test16.out 136 test(s) passed, 2 skipped, 0 failed. See all results in run-arm64.out 135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv5t_-marm.out 135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv5t_-mthumb.out 135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv7-a_-marm.out 135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv7-a_-mthumb.out 136 test(s) passed, 2 skipped, 0 failed. See all results in run-i386.out 136 test(s) passed, 2 skipped, 0 failed. See all results in run-i386-march=i586.out (...) $ grep ' [^0] failed' test16.out || echo OK OK
Hoping this helps, Willy
The opensbi package from Ubuntu 20.04 only provides rv64 firmwares:
$ dpkg -S opensbi | grep -E "fw_.*bin|fw_.*elf" | uniq opensbi: /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_dynamic.bin opensbi: /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.bin opensbi: /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_dynamic.elf opensbi: /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf
To run this nolibc test for rv32, users must build opensbi or download a prebuilt one from qemu repository:
https://gitlab.com/qemu-project/qemu/-/blob/master/pc-bios/opensbi-riscv32-g...
And then use -bios to tell qemu use it to avoid such failure:
$ qemu-system-riscv32 -display none -no-reboot -kernel /path/to/arch/riscv/boot/Image -serial stdio -M virt -append "console=ttyS0 panic=-1" qemu-system-riscv32: Unable to load the RISC-V firmware "opensbi-riscv32-generic-fw_dynamic.bin"
To run from makefile, QEMU_ARGS_EXTRA is added to allow pass extra arguments like -bios:
$ make run QEMU_ARGS_EXTRA="-bios /path/to/opensbi-riscv32-generic-fw_dynamic.bin" ...
Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/2ab94136-d341-4a26-964e-6d6c32e66c9b@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 47c3c89092e4..44088535682e 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -64,7 +64,7 @@ QEMU_ARGS_mips = -M malta -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_s390 = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)" QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" -QEMU_ARGS = $(QEMU_ARGS_$(ARCH)) +QEMU_ARGS = $(QEMU_ARGS_$(ARCH)) $(QEMU_ARGS_EXTRA)
# OUTPUT is only set when run from the main makefile, otherwise # it defaults to this nolibc directory.
Compiling nolibc-test.c with gcc on x86_64 got such warning:
tools/testing/selftests/nolibc/nolibc-test.c: In function ‘expect_eq’: tools/testing/selftests/nolibc/nolibc-test.c:177:24: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 2 has type ‘uint64_t’ {aka ‘long unsigned int’} [-Wformat=] 177 | llen += printf(" = %lld ", expr); | ~~~^ ~~~~ | | | | | uint64_t {aka long unsigned int} | long long int | %ld
It because that glibc defines uint64_t as "unsigned long int" when word size (means sizeof(long)) is 64bit (see include/bits/types.h), but nolibc directly use the 64bit "unsigned long long" (see tools/include/nolibc/stdint.h), which is simpler, seems kernel uses it too (include/uapi/asm-generic/int-ll64.h).
use a simple conversion to solve it.
Suggested-by: Willy Tarreau w@1wt.eu Link: https://lore.kernel.org/linux-riscv/20230529130449.GA2813@1wt.eu/ 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 d417ca5d976f..403f6255c177 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -174,7 +174,7 @@ static int expect_eq(uint64_t expr, int llen, uint64_t val) { int ret = !(expr == val);
- llen += printf(" = %lld ", expr); + llen += printf(" = %lld ", (long long)expr); pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n"); return ret; }
When compile nolibc-test.c with 2.31 glibc, we got such error:
In file included from /usr/riscv64-linux-gnu/include/sys/cdefs.h:452, from /usr/riscv64-linux-gnu/include/features.h:461, from /usr/riscv64-linux-gnu/include/bits/libc-header-start.h:33, from /usr/riscv64-linux-gnu/include/limits.h:26, from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:194, from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/syslimits.h:7, from /usr/lib/gcc-cross/riscv64-linux-gnu/9/include/limits.h:34, from /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/nolibc-test.c:6: /usr/riscv64-linux-gnu/include/bits/wordsize.h:28:3: error: #error "rv32i-based targets are not supported" 28 | # error "rv32i-based targets are not supported"
Glibc (>= 2.33) commit 5b6113d62efa ("RISC-V: Support the 32-bit ABI implementation") fixed up above error.
As suggested by Thomas, defining INT_MIN/INT_MAX for nolibc can remove the including of limits.h, and therefore no above error. of course, the other libcs still require limits.h, move it to the right place.
The LONG_MIN/LONG_MAX are also defined too.
Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/09d60dc2-e298-4c22-8e2f-8375861bd9be@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/stdint.h | 14 ++++++++++++++ tools/testing/selftests/nolibc/nolibc-test.c | 4 +--- 2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h index c1ce4f5e0603..31a5264539ae 100644 --- a/tools/include/nolibc/stdint.h +++ b/tools/include/nolibc/stdint.h @@ -96,4 +96,18 @@ typedef uint64_t uintmax_t; #define UINT_FAST32_MAX SIZE_MAX #define UINT_FAST64_MAX SIZE_MAX
+#ifndef INT_MIN +#define INT_MIN (-__INT_MAX__ - 1) +#endif +#ifndef INT_MAX +#define INT_MAX __INT_MAX__ +#endif + +#ifndef LONG_MIN +#define LONG_MIN (-__LONG_MAX__ - 1) +#endif +#ifndef LONG_MAX +#define LONG_MAX __LONG_MAX__ +#endif + #endif /* _NOLIBC_STDINT_H */ diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 403f6255c177..2a2954cb7bef 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -2,9 +2,6 @@
#define _GNU_SOURCE
-/* platform-specific include files coming from the compiler */ -#include <limits.h> - /* libc-specific include files * The program may be built in 3 ways: * $(CC) -nostdlib -include /path/to/nolibc.h => NOLIBC already defined @@ -39,6 +36,7 @@ #include <stddef.h> #include <stdint.h> #include <unistd.h> +#include <limits.h> #endif #endif
nolibc now has INT_MAX in stdint.h, so, don't mix INT_MAX and __INT_MAX__, unify them to INT_MAX.
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 2a2954cb7bef..a8fcad801cf2 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -931,7 +931,7 @@ static const struct test test_names[] = { int main(int argc, char **argv, char **envp) { int min = 0; - int max = __INT_MAX__; + int max = INT_MAX; int ret = 0; int err; int idx; @@ -979,7 +979,7 @@ int main(int argc, char **argv, char **envp) * here, which defaults to the full range. */ do { - min = 0; max = __INT_MAX__; + min = 0; max = INT_MAX; value = colon; if (value && *value) { colon = strchr(value, ':');
This is required by the coming removal of the oldselect and newselect support.
pselect6/pselect6_time64 will be used unconditionally, they have 6 arguments.
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-arm.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h index 45b89ffe8247..ca4c66987497 100644 --- a/tools/include/nolibc/arch-arm.h +++ b/tools/include/nolibc/arch-arm.h @@ -198,6 +198,29 @@ struct sys_stat_struct { _arg1; \ })
+#define my_syscall6(num, arg1, arg2, arg3, arg4, arg5, arg6) \ +({ \ + register long _num __asm__(_NOLIBC_SYSCALL_REG) = (num); \ + register long _arg1 __asm__ ("r0") = (long)(arg1); \ + register long _arg2 __asm__ ("r1") = (long)(arg2); \ + register long _arg3 __asm__ ("r2") = (long)(arg3); \ + register long _arg4 __asm__ ("r3") = (long)(arg4); \ + register long _arg5 __asm__ ("r4") = (long)(arg5); \ + register long _arg6 __asm__ ("r5") = (long)(arg6); \ + \ + __asm__ volatile ( \ + _NOLIBC_THUMB_SET_R7 \ + "svc #0\n" \ + _NOLIBC_THUMB_RESTORE_R7 \ + : "=r"(_arg1), "=r" (_num) \ + : "r"(_arg1), "r"(_arg2), "r"(_arg3), "r"(_arg4), "r"(_arg5), \ + "r"(_arg6), "r"(_num) \ + : "memory", "cc", "lr" \ + ); \ + _arg1; \ +}) + + char **environ __attribute__((weak)); const unsigned long *_auxv __attribute__((weak));
In function ‘open’: nolibc/sysroot/arm/include/sys.h:919:23: warning: ‘mode_t’ {aka ‘short unsigned int’} is promoted to ‘int’ when passed through ‘...’ 919 | mode = va_arg(args, mode_t); | ^ nolibc/sysroot/arm/include/sys.h:919:23: note: (so you should pass ‘int’ not ‘mode_t’ {aka ‘short unsigned int’} to ‘va_arg’) nolibc/sysroot/arm/include/sys.h:919:23: note: if this code is reached, the program will abort
Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/include/nolibc/sys.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 0160605444e7..856249a11890 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -862,7 +862,7 @@ int open(const char *path, int flags, ...) va_list args;
va_start(args, flags); - mode = va_arg(args, mode_t); + mode = va_arg(args, int); va_end(args); }
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.
Suggested-by: Willy Tarreau w@1wt.eu Link: https://lore.kernel.org/linux-riscv/20230528113325.GJ1956@1wt.eu/ 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 a8fcad801cf2..a1c402ce32f4 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");
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.
Suggested-by: Willy Tarreau w@1wt.eu Link: https://lore.kernel.org/linux-riscv/20230528113325.GJ1956@1wt.eu/ 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 a1c402ce32f4..bf63fc66e486 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -588,10 +588,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;
These 2 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue with nolibc:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
This issue happens while there is no "unsigned int" conversion in the coming new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;
Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/280867a8-7601-4a96-9b85-87668e1f1282@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index bf63fc66e486..e68c5692ec54 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,8 @@ 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_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-06-03 16:16:07+0800, Zhangjin Wu wrote:
These 2 test cases are added to cover the normal using scenes of gettimeofday().
They have been used to trigger and fix up such issue with nolibc:
nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
This issue happens while there is no "unsigned int" conversion in the coming new clock_gettime / clock_gettime64 syscall path of gettimeofday():
tv->tv_usec = ts.tv_nsec / 1000;
As mentioned before this looks to me like an issue in the build setup. Could you provide reproduction steps?
Nevertheless I guess the tests themselves are fine to have.
Suggested-by: Thomas Weißschuh linux@weissschuh.net Link: https://lore.kernel.org/linux-riscv/280867a8-7601-4a96-9b85-87668e1f1282@t-8... Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/nolibc-test.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index bf63fc66e486..e68c5692ec54 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,8 @@ 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(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;CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
-- 2.25.1
running nolibc-test with glibc on x86_64 got such print issue:
29 execve_root = -1 EACCES [OK] 30 fork30 fork = 0 [OK] 31 getdents64_root = 712 [OK]
The fork test case has three printf calls:
(1) llen += printf("%d %s", test, #name); (2) llen += printf(" = %d %s ", expr, errorname(errno)); (3) llen += pad_spc(llen, 64, "[FAIL]\n"); --> vfprintf()
In the following scene, the above issue happens:
(a) The parent calls (1) (b) The parent calls fork() (c) The child runs and shares the print buffer of (1) (d) The child exits, flushs the print buffer and closes its own stdout/stderr * "30 fork" is printed at the first time. (e) The parent calls (2) and (3), with "\n" in (3), it flushs the whole buffer * "30 fork = 0 ..." is printed
Therefore, there are two "30 fork" in the stdout.
Between (a) and (b), if flush the stdout (and the sterr), the child in stage (c) will not be able to 'see' the print buffer.
Reviewed-by: Thomas Weißschuh linux@weissschuh.net Signed-off-by: Zhangjin Wu falcon@tinylab.org --- tools/testing/selftests/nolibc/nolibc-test.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index e68c5692ec54..7dd950879161 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -490,7 +490,13 @@ static int test_getpagesize(void) static int test_fork(void) { int status; - pid_t pid = fork(); + pid_t pid; + + /* flush the printf buffer to avoid child flush it */ + fflush(stdout); + fflush(stderr); + + pid = fork();
switch (pid) { case -1:
On 2023-06-03 15:59:29+0800, Zhangjin Wu wrote:
Hi, Willy
This is the v3 generic part1 for rv32, all of the found issues of v2 part1 [1] have been fixed up, several generic patches have been fixed up and merged from v2 part2 [2] to this series, the standalone test_fork patch [4] is merged with a Reviewed-by line into this series too.
This series is based on 20230528-nolibc-rv32+stkp5 branch of [5].
Changes from v2 -> v3:
selftests/nolibc: fix up compile warning with glibc on x86_64
Use simpler 'long long' conversion instead of old #ifdef ... (Suggestion from Willy)
tools/nolibc: add missing nanoseconds support for __NR_statx
Split the compound assignment into two single assignments (Suggestion from Thomas)
selftests/nolibc: add new gettimeofday test cases
Removed the gettimeofday(NULL, &tz) (Suggestion from Thomas)
All of the commit messages have been re-checked, some missing Suggested-by lines are added.
The whole patchset have been tested on arm, aarch64, rv32 and rv64, no regressions (the next compile patchset is required to do rv32 test).
The nolibc-test has been tested with glibc on x86_64 too.
Btw, we have found such poll failures on arm (not introduced by this patchset), this will be fixed in our coming ppoll_time64 patchset:
48 poll_null = -1 ENOSYS [FAIL] 49 poll_stdout = -1 ENOSYS [FAIL] 50 poll_fault = -1 ENOSYS != (-1 EFAULT) [FAIL]
And the gettimeofday_null removal patch from Thomas [3] may conflicts with the gettimeofday removal and addition patches, but it is not hard to fix.
Best regards, Zhangjin
Zhangjin Wu (12): 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 selftests/nolibc: test_fork: fix up duplicated print
tools/include/nolibc/arch-arm.h | 23 +++++++++++ tools/include/nolibc/stdint.h | 14 +++++++ tools/include/nolibc/sys.h | 39 +++++++++--------- tools/testing/selftests/nolibc/Makefile | 2 +- tools/testing/selftests/nolibc/nolibc-test.c | 42 ++++++++++++-------- 5 files changed, 85 insertions(+), 35 deletions(-)
For the full series:
Reviewed-by: Thomas Weißschuh linux@weissschuh.net
-- 2.25.1
On Sun, Jun 04, 2023 at 08:49:52AM +0200, Thomas Weißschuh wrote:
On 2023-06-03 15:59:29+0800, Zhangjin Wu wrote:
Hi, Willy
This is the v3 generic part1 for rv32, all of the found issues of v2 part1 [1] have been fixed up, several generic patches have been fixed up and merged from v2 part2 [2] to this series, the standalone test_fork patch [4] is merged with a Reviewed-by line into this series too.
This series is based on 20230528-nolibc-rv32+stkp5 branch of [5].
Changes from v2 -> v3:
selftests/nolibc: fix up compile warning with glibc on x86_64
Use simpler 'long long' conversion instead of old #ifdef ... (Suggestion from Willy)
tools/nolibc: add missing nanoseconds support for __NR_statx
Split the compound assignment into two single assignments (Suggestion from Thomas)
selftests/nolibc: add new gettimeofday test cases
Removed the gettimeofday(NULL, &tz) (Suggestion from Thomas)
All of the commit messages have been re-checked, some missing Suggested-by lines are added.
The whole patchset have been tested on arm, aarch64, rv32 and rv64, no regressions (the next compile patchset is required to do rv32 test).
The nolibc-test has been tested with glibc on x86_64 too.
Btw, we have found such poll failures on arm (not introduced by this patchset), this will be fixed in our coming ppoll_time64 patchset:
48 poll_null = -1 ENOSYS [FAIL] 49 poll_stdout = -1 ENOSYS [FAIL] 50 poll_fault = -1 ENOSYS != (-1 EFAULT) [FAIL]
And the gettimeofday_null removal patch from Thomas [3] may conflicts with the gettimeofday removal and addition patches, but it is not hard to fix.
Best regards, Zhangjin
Zhangjin Wu (12): 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 selftests/nolibc: test_fork: fix up duplicated print
tools/include/nolibc/arch-arm.h | 23 +++++++++++ tools/include/nolibc/stdint.h | 14 +++++++ tools/include/nolibc/sys.h | 39 +++++++++--------- tools/testing/selftests/nolibc/Makefile | 2 +- tools/testing/selftests/nolibc/nolibc-test.c | 42 ++++++++++++-------- 5 files changed, 85 insertions(+), 35 deletions(-)
For the full series:
Reviewed-by: Thomas Weißschuh linux@weissschuh.net
I forgot to say, all the series is now queued, and I squashed the __NR_statx fix into Thomas' patch.
Willy
linux-kselftest-mirror@lists.linaro.org