2023-06-03 08:14:18

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 00/12] nolibc: add generic part1 of prepare for rv32

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/[email protected]/T/#t
[2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
[3]: https://lore.kernel.org/lkml/[email protected]/
[4]: https://lore.kernel.org/lkml/61bdfe7bacebdef8aa9195f6f2550a5b0d33aab3.1685426545.git.falcon@tinylab.org/
[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(-)

--
2.25.1



2023-06-03 08:16:58

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 02/12] tools/nolibc: add missing nanoseconds support for __NR_statx

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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
--
2.25.1



2023-06-03 08:40:52

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 07/12] tools/nolibc: arm: add missing my_syscall6

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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));

--
2.25.1


2023-06-03 08:40:52

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 08/12] tools/nolibc: open: fix up compile warning for arm

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 <[email protected]>
---
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);
}

--
2.25.1


2023-06-03 08:41:49

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 04/12] selftests/nolibc: fix up compile warning with glibc on x86_64

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
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 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;
}
--
2.25.1


2023-06-03 08:43:33

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 12/12] selftests/nolibc: test_fork: fix up duplicated print

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 <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
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:
--
2.25.1


2023-06-03 08:43:36

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 05/12] selftests/nolibc: not include limits.h for nolibc

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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

--
2.25.1


2023-06-03 08:44:33

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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;
--
2.25.1


2023-06-03 08:49:48

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 01/12] selftests/nolibc: syscall_args: use generic __NR_statx

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
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 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 */
--
2.25.1


2023-06-03 08:50:55

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 06/12] selftests/nolibc: use INT_MAX instead of __INT_MAX__

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 <[email protected]>
---
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, ':');
--
2.25.1


2023-06-03 08:54:38

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 03/12] selftests/nolibc: allow specify extra arguments for qemu

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-generic-fw_dynamic.bin

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
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.
--
2.25.1


2023-06-03 09:03:37

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 10/12] selftests/nolibc: remove gettimeofday_bad1/2 completely

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
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 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;
--
2.25.1


2023-06-03 09:04:27

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 09/12] selftests/nolibc: support two errnos with EXPECT_SYSER2()

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 <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
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 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");
--
2.25.1


2023-06-04 07:44:29

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

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 <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Zhangjin Wu <[email protected]>
> ---
> 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;
> --
> 2.25.1
>

2023-06-04 07:55:25

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH v3 00/12] nolibc: add generic part1 of prepare for rv32

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
> ---
>
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> [2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> [3]: https://lore.kernel.org/lkml/[email protected]/
> [4]: https://lore.kernel.org/lkml/61bdfe7bacebdef8aa9195f6f2550a5b0d33aab3.1685426545.git.falcon@tinylab.org/
> [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(-)

For the full series:

Reviewed-by: Thomas Weißschuh <[email protected]>

> --
> 2.25.1
>

2023-06-04 09:54:55

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

On Sun, Jun 4, 2023, at 10:29, 吴章金 wrote:
>
> Sorry for missing part of your feedbacks, I will check if -nostdlib
> stops the linking of libgcc_s or my own separated test script forgot
> linking the libgcc_s manually.

According to the gcc documentation, -nostdlib drops libgcc.a, but
adding -lgcc is the recommended way to bring it back.

> And as suggestion from Thomas' reply,
>
>>> Perhaps we really need to add the missing __divdi3 and __aeabi_ldivmod and the
>>> ones for the other architectures, or get one from lib/math/div64.c.
>
>>No, these ones come from the compiler via libgcc_s, we must not try to
> reimplement them. And we should do our best to avoid depending on them
> to avoid the error you got above.
>
> So, the explicit conversion is used instead in the patch.

I think a cast to a 32-bit type is ideal when converting the
clock_gettime() result into microseconds, since the kernel guarantees
that the timespec value is normalized, with all zeroes in the
upper 34 bits. Going through __aeabi_ldivmod would make the
conversion much slower.

For user supplied non-normalized timeval values, it's not obvious
whether we need the full 64-bit division

Arnd

2023-06-04 11:35:15

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] tools/nolibc: add missing nanoseconds support for __NR_statx

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 <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Zhangjin Wu <[email protected]>

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

2023-06-04 12:00:27

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

On Sun, Jun 04, 2023 at 01:38:39PM +0200, Arnd Bergmann wrote:
> > Over time we managed
> > to make simple code compile with both glibc and nolibc, but when it
> > comes at the cost of adding size and burden for the developers, such
> > as forcing them to add libgcc, I prefer that we slightly limit the
> > domain of application instead.
>
> Good point. This also reminds me that the compilers I build for
> https://mirrors.edge.kernel.org/pub/tools/crosstool/ don't always
> have every version of libgcc that may be needed, for instance
> the mips compilers only provide a big-endian libgcc and the
> arm compilers only provide a little-endian one, even though
> the compilers can build code both ways with the right flags.

That reminds me something indeed, I know that MIPS is a great platform
for testing portability due to libgcc and/or atomics not always being
complete depending how it's built. At work when I double-check that
haproxy still builds and starts on my EdgeRouter-X, then it will build
everywhere ;-)

Willy

2023-06-04 12:02:59

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

On Sun, Jun 04, 2023 at 11:24:39AM +0200, Arnd Bergmann wrote:
> On Sun, Jun 4, 2023, at 10:29, ??? wrote:
> >
> > Sorry for missing part of your feedbacks, I will check if -nostdlib
> > stops the linking of libgcc_s or my own separated test script forgot
> > linking the libgcc_s manually.
>
> According to the gcc documentation, -nostdlib drops libgcc.a, but
> adding -lgcc is the recommended way to bring it back.
>
> > And as suggestion from Thomas' reply,
> >
> >>> Perhaps we really need to add the missing __divdi3 and __aeabi_ldivmod and the
> >>> ones for the other architectures, or get one from lib/math/div64.c.
> >
> >>No, these ones come from the compiler via libgcc_s, we must not try to
> > reimplement them. And we should do our best to avoid depending on them
> > to avoid the error you got above.
> >
> > So, the explicit conversion is used instead in the patch.
>
> I think a cast to a 32-bit type is ideal when converting the
> clock_gettime() result into microseconds, since the kernel guarantees
> that the timespec value is normalized, with all zeroes in the
> upper 34 bits. Going through __aeabi_ldivmod would make the
> conversion much slower.
>
> For user supplied non-normalized timeval values, it's not obvious
> whether we need the full 64-bit division

We don't have to care about these here for the microsecond part,
because for decades these were exclusively 32-bit. Also the only
one consuming this field would have been settimeofday() and it's
already documented as returning EINVAL if tv_usec is not within
the expected 0..999999 range.

And when in doubt we should keep in mind that nolibc's purpose is not
to become a yet-another full-blown libc alternative but just a small
piece of software allowing to produce portable and compact binaries
for testing or booting. Being a bit stricter than other libcs for the
sake of code compactness is better here. Originally for example it was
necessary to always pass the 3 arguments to open(). Over time we managed
to make simple code compile with both glibc and nolibc, but when it
comes at the cost of adding size and burden for the developers, such
as forcing them to add libgcc, I prefer that we slightly limit the
domain of application instead.

Thanks!
Willy

2023-06-04 12:17:33

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] tools/nolibc: add missing nanoseconds support for __NR_statx

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 <[email protected]>
> > Link: https://lore.kernel.org/linux-riscv/[email protected]/
> > Signed-off-by: Zhangjin Wu <[email protected]>
>
> 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

2023-06-04 12:17:45

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

On Sun, Jun 4, 2023, at 13:27, Willy Tarreau wrote:
> On Sun, Jun 04, 2023 at 11:24:39AM +0200, Arnd Bergmann wrote:
>>
>> For user supplied non-normalized timeval values, it's not obvious
>> whether we need the full 64-bit division
>
> We don't have to care about these here for the microsecond part,
> because for decades these were exclusively 32-bit. Also the only
> one consuming this field would have been settimeofday() and it's
> already documented as returning EINVAL if tv_usec is not within
> the expected 0..999999 range.

Right

> Over time we managed
> to make simple code compile with both glibc and nolibc, but when it
> comes at the cost of adding size and burden for the developers, such
> as forcing them to add libgcc, I prefer that we slightly limit the
> domain of application instead.

Good point. This also reminds me that the compilers I build for
https://mirrors.edge.kernel.org/pub/tools/crosstool/ don't always
have every version of libgcc that may be needed, for instance
the mips compilers only provide a big-endian libgcc and the
arm compilers only provide a little-endian one, even though
the compilers can build code both ways with the right flags.

Arnd

2023-06-04 13:01:14

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] tools/nolibc: add missing nanoseconds support for __NR_statx

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 <[email protected]>
> > > Link: https://lore.kernel.org/linux-riscv/[email protected]/
> > > Signed-off-by: Zhangjin Wu <[email protected]>
> >
> > 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

2023-06-04 13:11:03

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 00/12] nolibc: add generic part1 of prepare for rv32

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
> > ---
> >
> > [1]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> > [2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> > [3]: https://lore.kernel.org/lkml/[email protected]/
> > [4]: https://lore.kernel.org/lkml/61bdfe7bacebdef8aa9195f6f2550a5b0d33aab3.1685426545.git.falcon@tinylab.org/
> > [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(-)
>
> For the full series:
>
> Reviewed-by: Thomas Wei?schuh <[email protected]>

I forgot to say, all the series is now queued, and I squashed the
__NR_statx fix into Thomas' patch.

Willy

2023-06-05 11:29:15

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v3 11/12] selftests/nolibc: add new gettimeofday test cases

> On Sun, Jun 04, 2023 at 11:24:39AM +0200, Arnd Bergmann wrote:
> > On Sun, Jun 4, 2023, at 10:29, ??? wrote:
> > >
> > > Sorry for missing part of your feedbacks, I will check if -nostdlib
> > > stops the linking of libgcc_s or my own separated test script forgot
> > > linking the libgcc_s manually.
> >
> > According to the gcc documentation, -nostdlib drops libgcc.a, but
> > adding -lgcc is the recommended way to bring it back.
> >
> > > And as suggestion from Thomas' reply,
> > >
> > >>> Perhaps we really need to add the missing __divdi3 and __aeabi_ldivmod and the
> > >>> ones for the other architectures, or get one from lib/math/div64.c.
> > >
> > >>No, these ones come from the compiler via libgcc_s, we must not try to
> > > reimplement them. And we should do our best to avoid depending on them
> > > to avoid the error you got above.
> > >
> > > So, the explicit conversion is used instead in the patch.
> >
> > I think a cast to a 32-bit type is ideal when converting the
> > clock_gettime() result into microseconds, since the kernel guarantees
> > that the timespec value is normalized, with all zeroes in the
> > upper 34 bits. Going through __aeabi_ldivmod would make the
> > conversion much slower.
> >

Perfectly, this message is really required to be added to the coming
clock_gettime/time64 patches, I did worry about the (unsigned int)
conversion may lose the upper bits, thanks Arnd.

> > For user supplied non-normalized timeval values, it's not obvious
> > whether we need the full 64-bit division
>
> We don't have to care about these here for the microsecond part,
> because for decades these were exclusively 32-bit. Also the only
> one consuming this field would have been settimeofday() and it's
> already documented as returning EINVAL if tv_usec is not within
> the expected 0..999999 range.
>

And this one, thanks Willy.

> And when in doubt we should keep in mind that nolibc's purpose is not
> to become a yet-another full-blown libc alternative but just a small
> piece of software allowing to produce portable and compact binaries
> for testing or booting. Being a bit stricter than other libcs for the
> sake of code compactness is better here. Originally for example it was
> necessary to always pass the 3 arguments to open(). Over time we managed
> to make simple code compile with both glibc and nolibc, but when it
> comes at the cost of adding size and burden for the developers, such
> as forcing them to add libgcc, I prefer that we slightly limit the
> domain of application instead.

This explains why it is 'no' libc ;-)

Best regards,
Zhangjin

>
> Thanks!
> Willy