| Message ID | 20220719214449.2520-1-w@1wt.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | nolibc: add preliminary self tests | expand |
On Tue, Jul 19, 2022 at 11:44:31PM +0200, Willy Tarreau wrote: > Hi Paul, > > as previously promised, here comes the nolibc update which introduces the > minimal self-test infrastructure that aims at being reasonably easy to > expand further. > > It's based on your branch "dev.2022.06.30b" that contains the previous > minor fixes that aimed at addressing Linus' concerns about the build > process inconsistencies. > > The way it works tries to mimmick as much as possible the regular build > process, so that it reuses the same ARCH, CC, CROSS_COMPILE to build the > test program, that will be embedded into an initramfs and the kernel is > (re)built with that initramfs. Then you can decide to run that kernel > under QEMU for the supported archs, and the output of the tests appears > in an output text file in a format that's easily greppable and diffable. > A single target "run" does everything. > > By default it will reuse your existing .config (so that developers > continue to use their regular config handling), though it can also > create a known-to-work defconfig for each arch. The reason behind this > is that it took me a moment to figure certain defconfig + machine name > combinations and I found it better to put them there once for all. > > I've successfully tested it on arm, arm64, i386, x86_64. riscv64 works > except two syscalls which return unexpected errors, and mips segfaults > in sbrk(). I don't know why yet, but this proves that it's worth having > such a test. Excellent, thank you!!! As we often said during my misspent youth, "If it ain't tested, it don't work." But I do get "71 test(s) passed." when running on x86. I will let you decide whether that constitutes all being well or indicates a bug in the tests. ;-) > There are not that many tests yet (71), those that have to run can be > filtered either from the program's command line or from a NOLIBC_TEST > environment variable so that it's possible to skip broken ones or to > focus on a few ranges only. > > Tests are numerically numbered, and are conveniently handled in a > switch/case statement so that a relative line number assigns the number > to the test. That's convenient because the vast majority of syscall tests > are one-liners. This sometimes slightly upsets check-patch when lines get > moderately long but that significantly improves legibility. > > There are expectation for both successes and failures (e.g. -1 ENOTDIR). > I'm sure this can be improved later (and that's the goal). Right now it > covers two test families: > - syscalls > - stdlib (str* functions mostly) > > I suspect that over time we might want to split syscalls into different > parts (e.g. core, fs, etc maybe) but I could be wrong. This is a good start, and we can let experience drive any additional changes that might be required. > The program can automatically modulate QEMU's return value on x86 when > QEMU is run with the appropriate options, but for now I'm not using it > as I felt like it didn't bring much value, and the output is more useful. > That's debatable, and maybe some might want to use it in bisect scripts > for example. It's too early to say IMHO. For the moment, grepping the output works. And perhaps indefinitely. > Oh, I also arranged the code so that the test also builds with glibc. I > noticed that when adding a new test that fails, sometimes it's convenient > to see if it's the nolibc part that's broken or the test. I don't find > this critical but the required includes and ifdefs are there so that it > should be easy to maintain over time as well. If nothing else, the ability to run against glibc is a good way to test the test. > I'm obviously interested in comments, but really, I don't want to > overdesign something for a first step, it remains a very modest test > program and I'd like that it remains easy to hack on it and to contribute > new tests that are deemed useful. I am good with a simple starting point. > I'm CCing the few who already contributed some patches and/or expressed > interest, as well as Linus who had a first bad experience when trying to > test it, hoping this one will be better. I'm pasting below [1] a copy of > a test on x86_64 below, that's summed up as "71 test(s) passed" at the > end of the "run" target. > > If there's no objection, it would be nice to have this with your current > series, as it definitely helps spot and fix the bugs. In parallel I'll see > if I can figure the problems with the two tests that fail each on a > specific arch and I might possibly have a few extra fixes for the current > nolibc. This series is now on the -rcu tree's "dev" branch. I got two almost identical copies of patch 7, so I took the later of the two. Please let me know if I guessed wrong. Thanx, Paul > Thank you! > Willy > > [1] example output > ----8<---- > Running test 'syscall' > 0 getpid = 1 [OK] > 1 getppid = 0 [OK] > 5 getpgid_self = 0 [OK] > 6 getpgid_bad = -1 ESRCH [OK] > 7 kill_0 = 0 [OK] > 8 kill_CONT = 0 [OK] > 9 kill_BADPID = -1 ESRCH [OK] > 10 sbrk = 0 [OK] > 11 brk = 0 [OK] > 12 chdir_root = 0 [OK] > 13 chdir_dot = 0 [OK] > 14 chdir_blah = -1 ENOENT [OK] > 15 chmod_net = 0 [OK] > 16 chmod_self = -1 EPERM [OK] > 17 chown_self = -1 EPERM [OK] > 18 chroot_root = 0 [OK] > 19 chroot_blah = -1 ENOENT [OK] > 20 chroot_exe = -1 ENOTDIR [OK] > 21 close_m1 = -1 EBADF [OK] > 22 close_dup = 0 [OK] > 23 dup_0 = 3 [OK] > 24 dup_m1 = -1 EBADF [OK] > 25 dup2_0 = 100 [OK] > 26 dup2_m1 = -1 EBADF [OK] > 27 dup3_0 = 100 [OK] > 28 dup3_m1 = -1 EBADF [OK] > 29 execve_root = -1 EACCES [OK] > 30 getdents64_root = 120 [OK] > 31 getdents64_null = -1 ENOTDIR [OK] > 32 gettimeofday_null = 0 [OK] > 38 ioctl_tiocinq = 0 [OK] > 39 ioctl_tiocinq = 0 [OK] > 40 link_root1 = -1 EEXIST [OK] > 41 link_blah = -1 ENOENT [OK] > 42 link_dir = -1 EPERM [OK] > 43 link_cross = -1 EXDEV [OK] > 44 lseek_m1 = -1 EBADF [OK] > 45 lseek_0 = -1 ESPIPE [OK] > 46 mkdir_root = -1 EEXIST [OK] > 47 open_tty = 3 [OK] > 48 open_blah = -1 ENOENT [OK] > 49 poll_null = 0 [OK] > 50 poll_stdout = 1 [OK] > 51 poll_fault = -1 EFAULT [OK] > 52 read_badf = -1 EBADF [OK] > 53 sched_yield = 0 [OK] > 54 select_null = 0 [OK] > 55 select_stdout = 1 [OK] > 56 select_fault = -1 EFAULT [OK] > 57 stat_blah = -1 ENOENT [OK] > 58 stat_fault = -1 EFAULT [OK] > 59 symlink_root = -1 EEXIST [OK] > 60 unlink_root = -1 EISDIR [OK] > 61 unlink_blah = -1 ENOENT [OK] > 62 wait_child = -1 ECHILD [OK] > 63 waitpid_min = -1 ESRCH [OK] > 64 waitpid_child = -1 ECHILD [OK] > 65 write_badf = -1 EBADF [OK] > 66 write_zero = 0 [OK] > Errors during this test: 0 > > Running test 'stdlib' > 0 getenv_TERM = <linux> [OK] > 1 getenv_blah = <(null)> [OK] > 2 setcmp_blah_blah = 0 [OK] > 3 setcmp_blah_blah2 = -50 [OK] > 4 setncmp_blah_blah = 0 [OK] > 5 setncmp_blah_blah4 = 0 [OK] > 6 setncmp_blah_blah5 = -53 [OK] > 7 setncmp_blah_blah6 = -54 [OK] > 8 strchr_foobar_o = <oobar> [OK] > 9 strchr_foobar_z = <(null)> [OK] > 10 strrchr_foobar_o = <obar> [OK] > 11 strrchr_foobar_z = <(null)> [OK] > Errors during this test: 0 > > Total number of errors: 0 > ---->8---- > > > -- > > Willy Tarreau (17): > tools/nolibc: make argc 32-bit in riscv startup code > tools/nolibc: fix build warning in sys_mmap() when my_syscall6 is not > defined > tools/nolibc: make sys_mmap() automatically use the right __NR_mmap > definition > selftests/nolibc: add basic infrastructure to ease creation of nolibc > tests > selftests/nolibc: support a test definition format > selftests/nolibc: implement a few tests for various syscalls > selftests/nolibc: add a few tests for some stdlib functions > selftests/nolibc: exit with poweroff on success when getpid() == 1 > selftests/nolibc: on x86, support exiting with isa-debug-exit > selftests/nolibc: recreate and populate /dev and /proc if missing > selftests/nolibc: condition some tests on /proc existence > selftests/nolibc: support glibc as well > selftests/nolibc: add a "kernel" target to build the kernel with the > initramfs > selftests/nolibc: add a "defconfig" target > selftests/nolibc: add a "run" target to start the kernel in QEMU > selftests/nolibc: "sysroot" target installs a local copy of the > sysroot > selftests/nolibc: add a "help" target > > MAINTAINERS | 1 + > tools/include/nolibc/arch-riscv.h | 2 +- > tools/include/nolibc/sys.h | 4 +- > tools/testing/selftests/nolibc/Makefile | 135 ++++ > tools/testing/selftests/nolibc/nolibc-test.c | 757 +++++++++++++++++++ > 5 files changed, 896 insertions(+), 3 deletions(-) > create mode 100644 tools/testing/selftests/nolibc/Makefile > create mode 100644 tools/testing/selftests/nolibc/nolibc-test.c > > -- > 2.17.5 >
On Tue, Jul 19, 2022 at 03:49:47PM -0700, Paul E. McKenney wrote: > But I do get "71 test(s) passed." when running on x86. I will let you > decide whether that constitutes all being well or indicates a bug in > the tests. ;-) Technically speaking both are possible, but one is more likely :-) > > The program can automatically modulate QEMU's return value on x86 when > > QEMU is run with the appropriate options, but for now I'm not using it > > as I felt like it didn't bring much value, and the output is more useful. > > That's debatable, and maybe some might want to use it in bisect scripts > > for example. It's too early to say IMHO. > > For the moment, grepping the output works. And perhaps indefinitely. That's my intuition as well, given that there will always be a bit of scripting around that anyway. > This series is now on the -rcu tree's "dev" branch. Thank you! > I got two almost > identical copies of patch 7, so I took the later of the two. Please let > me know if I guessed wrong. Oh you're right, I'm sorry about that. I adjusted one commit message late and failed to erase the previous one from the directory. In either case we really don't care but I thought that the one mentioning "stdlib" which is the term used in the test was better. Thanks again for your time, Willy
On 7/20/22 4:44 AM, Willy Tarreau wrote: > I'm obviously interested in comments, but really, I don't want to > overdesign something for a first step, it remains a very modest test > program and I'd like that it remains easy to hack on it and to contribute > new tests that are deemed useful. I personally hate how the test framework mandates: "There must be exactly one test per line." which makes the test case, for example, one long liner like this: if ((p1 = p2 = sbrk(4096)) != (void *)-1) p2 = sbrk(-4096); EXPECT_SYSZR(1, (p2 == (void *)-1) || p2 == p1); break; that's ugly and hard to read. Can we get rid of this "one test per line" rule? It would be great if we followed the documented coding style that says: "Statements longer than 80 columns should be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information." [1] What we have here doesn't really increase the readability at all. Maybe it's too late for 5.20, just for next in case we want to fix it. Willy? [1]: https://www.kernel.org/doc/html/v5.15/process/coding-style.html#breaking-long-lines-and-strings
On Wed, Jul 20, 2022 at 11:03:58PM +0700, Ammar Faizi wrote: > On 7/20/22 4:44 AM, Willy Tarreau wrote: > > I'm obviously interested in comments, but really, I don't want to > > overdesign something for a first step, it remains a very modest test > > program and I'd like that it remains easy to hack on it and to contribute > > new tests that are deemed useful. > > I personally hate how the test framework mandates: > > "There must be exactly one test per line." I know, that's a design choice that makes them trivial to add, because it's the compiler that assigns the test IDs, and it comes with a non negligible benefit. > which makes the test case, for example, one long liner like this: > > if ((p1 = p2 = sbrk(4096)) != (void *)-1) p2 = sbrk(-4096); EXPECT_SYSZR(1, (p2 == (void *)-1) || p2 == p1); break; > > that's ugly and hard to read. Can we get rid of this "one test per line" rule? If you find a better solution, I'm open. What I certainly don't want to do is to have to cross-reference IDs with arrays, nor start to stack endless if/else that are even more painful to deal with, or have to renumber everything by hand once in a while. > It would be great if we followed the documented coding style that says: > > "Statements longer than 80 columns should be broken into sensible chunks, > unless exceeding 80 columns significantly increases readability and does > not hide information." [1] Admittedly this is not core code but debugging code running in userland to help developers spot bugs in their code which is somewhere else and well maintained. I personally think that the tradeoff is positive here, i.e. non-pretty but easily hackable short tests that encourage additions and variations. The ease of adding tests allowed me to create 71 of them in a single afternoon and two of them brought me bugs in existing code, which I think is efficient. But I'm not fond of the approach either, I just couldn't produce anything as efficient that was prettier, but I'm quite open to being proven wrong by an alternate proposal. > What we have here doesn't really increase the readability at all. Maybe > it's too late for 5.20, just for next in case we want to fix it. The goal was not to increase *readability* but *writability*. We're still missing test for most syscalls and I would like them to be added quickly so that we can continue to add tested code. The readability I care about is understanding the output. When I'm seeing: ... 29 execve_root = -1 EACCES [OK] 30 getdents64_root = -1 EBADF [FAIL] 31 getdents64_null = -1 EBADF != (-1 ENOTDIR) [FAIL] 32 gettimeofday_null = 0 [OK] ... on riscv64, I don't have to search long to figure that we did something wrong with getdents64() on this arch and that the error path works differently. Similarly, this on mips: 8 kill_CONT = 0 [OK] 9 kill_BADPID = -1 ESRCH [OK] 10 sbrkdo_page_fault(): sending SIGSEGV to init for invalid read access from 0000000a epc = 0000000a in init[400000+4000] ra = 0000000a in init[400000+4000] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b tells me that sbrk() definitely doesn't work there. In both cases I know what and where to look without even having to *read* that test. This does matter to me, as a developer of the component subject to the test. But again, I'm open to better proposals. I reached the limits of my imagination there, but I do value the ability to "yyp" one line, change two arguments and gain one extra test for a different combination, and I really do not want to lose that simplicity. Note that for more complex tests, it's trivial to add a dedicated function and that's what was done for getdents64() which also serves as an example. Willy
On 7/20/22 11:20 PM, Willy Tarreau wrote: > What I certainly don't want to do is to have to cross-reference IDs > with arrays, nor start to stack endless if/else that are even more > painful to deal with, or have to renumber everything by hand once in > a while. Noted. > But again, I'm open to better proposals. I reached the limits of my > imagination there, but I do value the ability to "yyp" one line, change > two arguments and gain one extra test for a different combination, and > I really do not want to lose that simplicity. Note that for more complex > tests, it's trivial to add a dedicated function and that's what was done > for getdents64() which also serves as an example. OK, I understand the reason behind this now. I and Fernanda will try to visit this again at around 5.20-rc. *If* we can find a better design that matches your requirements, we will send you an RFC to improve it too. Thank you!
On Thu, Jul 21, 2022 at 12:05:14AM +0700, Ammar Faizi wrote: > > But again, I'm open to better proposals. I reached the limits of my > > imagination there, but I do value the ability to "yyp" one line, change > > two arguments and gain one extra test for a different combination, and > > I really do not want to lose that simplicity. Note that for more complex > > tests, it's trivial to add a dedicated function and that's what was done > > for getdents64() which also serves as an example. > > OK, I understand the reason behind this now. I and Fernanda will try > to visit this again at around 5.20-rc. *If* we can find a better > design that matches your requirements, we will send you an RFC to > improve it too. You would be very welcome, thank you! Willy
Hi Paul, as previously promised, here comes the nolibc update which introduces the minimal self-test infrastructure that aims at being reasonably easy to expand further. It's based on your branch "dev.2022.06.30b" that contains the previous minor fixes that aimed at addressing Linus' concerns about the build process inconsistencies. The way it works tries to mimmick as much as possible the regular build process, so that it reuses the same ARCH, CC, CROSS_COMPILE to build the test program, that will be embedded into an initramfs and the kernel is (re)built with that initramfs. Then you can decide to run that kernel under QEMU for the supported archs, and the output of the tests appears in an output text file in a format that's easily greppable and diffable. A single target "run" does everything. By default it will reuse your existing .config (so that developers continue to use their regular config handling), though it can also create a known-to-work defconfig for each arch. The reason behind this is that it took me a moment to figure certain defconfig + machine name combinations and I found it better to put them there once for all. I've successfully tested it on arm, arm64, i386, x86_64. riscv64 works except two syscalls which return unexpected errors, and mips segfaults in sbrk(). I don't know why yet, but this proves that it's worth having such a test. There are not that many tests yet (71), those that have to run can be filtered either from the program's command line or from a NOLIBC_TEST environment variable so that it's possible to skip broken ones or to focus on a few ranges only. Tests are numerically numbered, and are conveniently handled in a switch/case statement so that a relative line number assigns the number to the test. That's convenient because the vast majority of syscall tests are one-liners. This sometimes slightly upsets check-patch when lines get moderately long but that significantly improves legibility. There are expectation for both successes and failures (e.g. -1 ENOTDIR). I'm sure this can be improved later (and that's the goal). Right now it covers two test families: - syscalls - stdlib (str* functions mostly) I suspect that over time we might want to split syscalls into different parts (e.g. core, fs, etc maybe) but I could be wrong. The program can automatically modulate QEMU's return value on x86 when QEMU is run with the appropriate options, but for now I'm not using it as I felt like it didn't bring much value, and the output is more useful. That's debatable, and maybe some might want to use it in bisect scripts for example. It's too early to say IMHO. Oh, I also arranged the code so that the test also builds with glibc. I noticed that when adding a new test that fails, sometimes it's convenient to see if it's the nolibc part that's broken or the test. I don't find this critical but the required includes and ifdefs are there so that it should be easy to maintain over time as well. I'm obviously interested in comments, but really, I don't want to overdesign something for a first step, it remains a very modest test program and I'd like that it remains easy to hack on it and to contribute new tests that are deemed useful. I'm CCing the few who already contributed some patches and/or expressed interest, as well as Linus who had a first bad experience when trying to test it, hoping this one will be better. I'm pasting below [1] a copy of a test on x86_64 below, that's summed up as "71 test(s) passed" at the end of the "run" target. If there's no objection, it would be nice to have this with your current series, as it definitely helps spot and fix the bugs. In parallel I'll see if I can figure the problems with the two tests that fail each on a specific arch and I might possibly have a few extra fixes for the current nolibc. Thank you! Willy [1] example output ----8<---- Running test 'syscall' 0 getpid = 1 [OK] 1 getppid = 0 [OK] 5 getpgid_self = 0 [OK] 6 getpgid_bad = -1 ESRCH [OK] 7 kill_0 = 0 [OK] 8 kill_CONT = 0 [OK] 9 kill_BADPID = -1 ESRCH [OK] 10 sbrk = 0 [OK] 11 brk = 0 [OK] 12 chdir_root = 0 [OK] 13 chdir_dot = 0 [OK] 14 chdir_blah = -1 ENOENT [OK] 15 chmod_net = 0 [OK] 16 chmod_self = -1 EPERM [OK] 17 chown_self = -1 EPERM [OK] 18 chroot_root = 0 [OK] 19 chroot_blah = -1 ENOENT [OK] 20 chroot_exe = -1 ENOTDIR [OK] 21 close_m1 = -1 EBADF [OK] 22 close_dup = 0 [OK] 23 dup_0 = 3 [OK] 24 dup_m1 = -1 EBADF [OK] 25 dup2_0 = 100 [OK] 26 dup2_m1 = -1 EBADF [OK] 27 dup3_0 = 100 [OK] 28 dup3_m1 = -1 EBADF [OK] 29 execve_root = -1 EACCES [OK] 30 getdents64_root = 120 [OK] 31 getdents64_null = -1 ENOTDIR [OK] 32 gettimeofday_null = 0 [OK] 38 ioctl_tiocinq = 0 [OK] 39 ioctl_tiocinq = 0 [OK] 40 link_root1 = -1 EEXIST [OK] 41 link_blah = -1 ENOENT [OK] 42 link_dir = -1 EPERM [OK] 43 link_cross = -1 EXDEV [OK] 44 lseek_m1 = -1 EBADF [OK] 45 lseek_0 = -1 ESPIPE [OK] 46 mkdir_root = -1 EEXIST [OK] 47 open_tty = 3 [OK] 48 open_blah = -1 ENOENT [OK] 49 poll_null = 0 [OK] 50 poll_stdout = 1 [OK] 51 poll_fault = -1 EFAULT [OK] 52 read_badf = -1 EBADF [OK] 53 sched_yield = 0 [OK] 54 select_null = 0 [OK] 55 select_stdout = 1 [OK] 56 select_fault = -1 EFAULT [OK] 57 stat_blah = -1 ENOENT [OK] 58 stat_fault = -1 EFAULT [OK] 59 symlink_root = -1 EEXIST [OK] 60 unlink_root = -1 EISDIR [OK] 61 unlink_blah = -1 ENOENT [OK] 62 wait_child = -1 ECHILD [OK] 63 waitpid_min = -1 ESRCH [OK] 64 waitpid_child = -1 ECHILD [OK] 65 write_badf = -1 EBADF [OK] 66 write_zero = 0 [OK] Errors during this test: 0 Running test 'stdlib' 0 getenv_TERM = <linux> [OK] 1 getenv_blah = <(null)> [OK] 2 setcmp_blah_blah = 0 [OK] 3 setcmp_blah_blah2 = -50 [OK] 4 setncmp_blah_blah = 0 [OK] 5 setncmp_blah_blah4 = 0 [OK] 6 setncmp_blah_blah5 = -53 [OK] 7 setncmp_blah_blah6 = -54 [OK] 8 strchr_foobar_o = <oobar> [OK] 9 strchr_foobar_z = <(null)> [OK] 10 strrchr_foobar_o = <obar> [OK] 11 strrchr_foobar_z = <(null)> [OK] Errors during this test: 0 Total number of errors: 0 ---->8---- -- Willy Tarreau (17): tools/nolibc: make argc 32-bit in riscv startup code tools/nolibc: fix build warning in sys_mmap() when my_syscall6 is not defined tools/nolibc: make sys_mmap() automatically use the right __NR_mmap definition selftests/nolibc: add basic infrastructure to ease creation of nolibc tests selftests/nolibc: support a test definition format selftests/nolibc: implement a few tests for various syscalls selftests/nolibc: add a few tests for some stdlib functions selftests/nolibc: exit with poweroff on success when getpid() == 1 selftests/nolibc: on x86, support exiting with isa-debug-exit selftests/nolibc: recreate and populate /dev and /proc if missing selftests/nolibc: condition some tests on /proc existence selftests/nolibc: support glibc as well selftests/nolibc: add a "kernel" target to build the kernel with the initramfs selftests/nolibc: add a "defconfig" target selftests/nolibc: add a "run" target to start the kernel in QEMU selftests/nolibc: "sysroot" target installs a local copy of the sysroot selftests/nolibc: add a "help" target MAINTAINERS | 1 + tools/include/nolibc/arch-riscv.h | 2 +- tools/include/nolibc/sys.h | 4 +- tools/testing/selftests/nolibc/Makefile | 135 ++++ tools/testing/selftests/nolibc/nolibc-test.c | 757 +++++++++++++++++++ 5 files changed, 896 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/nolibc/Makefile create mode 100644 tools/testing/selftests/nolibc/nolibc-test.c