[PATCH v2 00/13] nolibc: add part2 of support for rv32

Zhangjin Wu posted 13 patches 2 years, 8 months ago
There is a newer version of this series
tools/include/nolibc/arch-aarch64.h          |   3 -
tools/include/nolibc/arch-loongarch.h        |   3 -
tools/include/nolibc/arch-riscv.h            |   3 -
tools/include/nolibc/std.h                   |  28 ++--
tools/include/nolibc/sys.h                   | 134 +++++++++++++++----
tools/include/nolibc/types.h                 |  58 +++++++-
tools/testing/selftests/nolibc/Makefile      |  11 +-
tools/testing/selftests/nolibc/nolibc-test.c |  20 +--
8 files changed, 202 insertions(+), 58 deletions(-)
[PATCH v2 00/13] nolibc: add part2 of support for rv32
Posted by Zhangjin Wu 2 years, 8 months ago
Hi, all

Thanks very much for your review suggestions of the v1 series [1], we
just sent out the generic part1 [2], and here is the part2 of the whole
v2 revision.

Changes from v1 -> v2:

* Don't emulate the return values in the new syscalls path, fix up or
  support the new syscalls in the side of the related test cases (1-3)

  selftests/nolibc: remove gettimeofday_bad1/2 completely
  selftests/nolibc: support two errnos with EXPECT_SYSER2()
  selftests/nolibc: waitpid_min: add waitid syscall support

  (Review suggestions from Willy and Thomas)

* Fix up new failure of the state_timestamps test case (4, new)

  tools/nolibc: add missing nanoseconds support for __NR_statx

  (Fixes for the commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()")

* Add new waitstatus macros as a standalone patch for the waitid support (5)

  tools/nolibc: add more wait status related types

  (Split and Cleanup for the waitid syscall based sys_wait4)

* Pure 64bit lseek and time64 select/poll/gettimeofday support (6-11)

  tools/nolibc: add pure 64bit off_t, time_t and blkcnt_t
  tools/nolibc: sys_lseek: add pure 64bit lseek
  tools/nolibc: add pure 64bit time structs
  tools/nolibc: sys_select: add pure 64bit select
  tools/nolibc: sys_poll: add pure 64bit poll
  tools/nolibc: sys_gettimeofday: add pure 64bit gettimeofday

  (Review suggestions from Arnd, Thomas and Willy, time32 variants have
   been removed completely and some fixups)

* waitid syscall support cleanup (12)

  tools/nolibc: sys_wait4: add waitid syscall support

  (Sync with the waitstatus macros update and Removal of emulated code)

* rv32 nolibc-test support, commit message update (13)

  selftests/nolibc: riscv: customize makefile for rv32

  (Review suggestions from Thomas, explain more about the change logic in commit message)

Best regards,
Zhangjin
---

[1]: https://lore.kernel.org/linux-riscv/20230529113143.GB2762@1wt.eu/T/#t
[2]: https://lore.kernel.org/linux-riscv/cover.1685362482.git.falcon@tinylab.org/

Zhangjin Wu (13):
  selftests/nolibc: remove gettimeofday_bad1/2 completely
  selftests/nolibc: support two errnos with EXPECT_SYSER2()
  selftests/nolibc: waitpid_min: add waitid syscall support
  tools/nolibc: add missing nanoseconds support for __NR_statx
  tools/nolibc: add more wait status related types
  tools/nolibc: add pure 64bit off_t, time_t and blkcnt_t
  tools/nolibc: sys_lseek: add pure 64bit lseek
  tools/nolibc: add pure 64bit time structs
  tools/nolibc: sys_select: add pure 64bit select
  tools/nolibc: sys_poll: add pure 64bit poll
  tools/nolibc: sys_gettimeofday: add pure 64bit gettimeofday
  tools/nolibc: sys_wait4: add waitid syscall support
  selftests/nolibc: riscv: customize makefile for rv32

 tools/include/nolibc/arch-aarch64.h          |   3 -
 tools/include/nolibc/arch-loongarch.h        |   3 -
 tools/include/nolibc/arch-riscv.h            |   3 -
 tools/include/nolibc/std.h                   |  28 ++--
 tools/include/nolibc/sys.h                   | 134 +++++++++++++++----
 tools/include/nolibc/types.h                 |  58 +++++++-
 tools/testing/selftests/nolibc/Makefile      |  11 +-
 tools/testing/selftests/nolibc/nolibc-test.c |  20 +--
 8 files changed, 202 insertions(+), 58 deletions(-)

-- 
2.25.1
Re: [PATCH v2 00/13] nolibc: add part2 of support for rv32
Posted by Willy Tarreau 2 years, 8 months ago
Hi Zhangjin,

On Tue, May 30, 2023 at 03:45:23AM +0800, Zhangjin Wu wrote:
> Hi, all
> 
> Thanks very much for your review suggestions of the v1 series [1], we
> just sent out the generic part1 [2], and here is the part2 of the whole
> v2 revision.
(...)

Just to let you know, I'm now done with my release and will slowly try
to catch up with all your series!

Stay tuned!
Willy
[PATCH v2 0/2] nolibc: add part3 of support for rv32
Posted by Zhangjin Wu 2 years, 8 months ago
Hi, Willy

These two patches are based on part2 of support for rv32 [1], I have
forgotten to send them together.

Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/linux-riscv/cover.1685387484.git.falcon@tinylab.org/

Zhangjin Wu (2):
  selftests/nolibc: add new gettimeofday test cases
  selftests/nolibc: add sizeof test for the new 64bit data types

 tools/testing/selftests/nolibc/nolibc-test.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.25.1
[PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Zhangjin Wu 2 years, 8 months ago
These 3 test cases are added to cover the normal using scenes of
gettimeofday().

They have been used to trigger and fix up such issue:

    nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'

This issue happens while there is no "unsigned int" conversion in the
new clock_gettime / clock_gettime64 syscall path of gettimeofday():

    tv->tv_usec = ts.tv_nsec / 1000;

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 8ba8c2fc71a0..20d184da9a2b 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
  */
 int run_syscall(int min, int max)
 {
+	struct timeval tv;
+	struct timezone tz;
 	struct stat stat_buf;
 	int euid0;
 	int proc;
@@ -588,6 +590,9 @@ int run_syscall(int min, int max)
 		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
 		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
 		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
+		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
+		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
+		CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
 		CASE_TEST(getpagesize);       EXPECT_SYSZR(1, test_getpagesize()); break;
 		CASE_TEST(ioctl_tiocinq);     EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break;
 		CASE_TEST(ioctl_tiocinq);     EXPECT_SYSZR(1, ioctl(0, TIOCINQ, &tmp)); break;
-- 
2.25.1
Re: [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Thomas Weißschuh 2 years, 8 months ago
On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
> These 3 test cases are added to cover the normal using scenes of
> gettimeofday().
> 
> They have been used to trigger and fix up such issue:
> 
>     nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
> 
> This issue happens while there is no "unsigned int" conversion in the
> new clock_gettime / clock_gettime64 syscall path of gettimeofday():
> 
>     tv->tv_usec = ts.tv_nsec / 1000;
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 8ba8c2fc71a0..20d184da9a2b 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
>   */
>  int run_syscall(int min, int max)
>  {
> +	struct timeval tv;
> +	struct timezone tz;
>  	struct stat stat_buf;
>  	int euid0;
>  	int proc;
> @@ -588,6 +590,9 @@ int run_syscall(int min, int max)
>  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
>  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
>  		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
> +		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
> +		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;

Calling gettimeofday(NULL, ...) will actually segfault on glibc.
It works when calling through the VDSO, but not the logic in glibc
itself, which is guess is allowed by POSIX.

I propose to avoid doing it :-)

Either we gate the existing test in #ifdef NOLIBC or we remove it.

> +		CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
>  		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
>
Re: [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Willy Tarreau 2 years, 8 months ago
On Tue, May 30, 2023 at 12:59:31PM +0200, Thomas Weißschuh wrote:
> On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
> > These 3 test cases are added to cover the normal using scenes of
> > gettimeofday().
> > 
> > They have been used to trigger and fix up such issue:
> > 
> >     nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
> > 
> > This issue happens while there is no "unsigned int" conversion in the
> > new clock_gettime / clock_gettime64 syscall path of gettimeofday():
> > 
> >     tv->tv_usec = ts.tv_nsec / 1000;
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index 8ba8c2fc71a0..20d184da9a2b 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
> >   */
> >  int run_syscall(int min, int max)
> >  {
> > +	struct timeval tv;
> > +	struct timezone tz;
> >  	struct stat stat_buf;
> >  	int euid0;
> >  	int proc;
> > @@ -588,6 +590,9 @@ int run_syscall(int min, int max)
> >  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
> >  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
> >  		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
> > +		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
> > +		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
> 
> Calling gettimeofday(NULL, ...) will actually segfault on glibc.
> It works when calling through the VDSO, but not the logic in glibc
> itself, which is guess is allowed by POSIX.

Then that's shocking, because the man page says:

       If either tv or tz is NULL, the corresponding structure is not  set  or
       returned.   (However, compilation warnings will result if tv is NULL.)

I'd expect glibc to at least support what'd documented in the man
page :-/

> I propose to avoid doing it :-)

If you're certain that's the case, then I agree.

> Either we gate the existing test in #ifdef NOLIBC or we remove it.

Better not keep tests specific to nolibc if they aim at verifying some
compatibliity.

Willy
Re: [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Thomas Weißschuh 2 years, 8 months ago
On 2023-05-30 14:05:00+0200, Willy Tarreau wrote:
> On Tue, May 30, 2023 at 12:59:31PM +0200, Thomas Weißschuh wrote:
> > On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
> > > These 3 test cases are added to cover the normal using scenes of
> > > gettimeofday().
> > > 
> > > They have been used to trigger and fix up such issue:
> > > 
> > >     nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
> > > 
> > > This issue happens while there is no "unsigned int" conversion in the
> > > new clock_gettime / clock_gettime64 syscall path of gettimeofday():
> > > 
> > >     tv->tv_usec = ts.tv_nsec / 1000;
> > > 
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > ---
> > >  tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > > index 8ba8c2fc71a0..20d184da9a2b 100644
> > > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > > @@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
> > >   */
> > >  int run_syscall(int min, int max)
> > >  {
> > > +	struct timeval tv;
> > > +	struct timezone tz;
> > >  	struct stat stat_buf;
> > >  	int euid0;
> > >  	int proc;
> > > @@ -588,6 +590,9 @@ int run_syscall(int min, int max)
> > >  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
> > >  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
> > >  		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
> > > +		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
> > > +		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
> > 
> > Calling gettimeofday(NULL, ...) will actually segfault on glibc.
> > It works when calling through the VDSO, but not the logic in glibc
> > itself, which is guess is allowed by POSIX.
> 
> Then that's shocking, because the man page says:
> 
>        If either tv or tz is NULL, the corresponding structure is not  set  or
>        returned.   (However, compilation warnings will result if tv is NULL.)
> 
> I'd expect glibc to at least support what'd documented in the man
> page :-/

That is gettimeofday(2), which comes from the Linux man-pages project.

gettimeofday(3p) which specifies the posix function does not have that wording.
It also specifies that there is no way to indicate errors...

Seems to be a weird situation.

> > I propose to avoid doing it :-)
> 
> If you're certain that's the case, then I agree.
> 
> > Either we gate the existing test in #ifdef NOLIBC or we remove it.
> 
> Better not keep tests specific to nolibc if they aim at verifying some
> compatibliity.

Thomas
Re: [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Andreas Schwab 2 years, 8 months ago
On Mai 30 2023, Willy Tarreau wrote:

> On Tue, May 30, 2023 at 12:59:31PM +0200, Thomas Weißschuh wrote:
>> On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
>> > These 3 test cases are added to cover the normal using scenes of
>> > gettimeofday().
>> > 
>> > They have been used to trigger and fix up such issue:
>> > 
>> >     nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
>> > 
>> > This issue happens while there is no "unsigned int" conversion in the
>> > new clock_gettime / clock_gettime64 syscall path of gettimeofday():
>> > 
>> >     tv->tv_usec = ts.tv_nsec / 1000;
>> > 
>> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
>> > ---
>> >  tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> > 
>> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
>> > index 8ba8c2fc71a0..20d184da9a2b 100644
>> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
>> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
>> > @@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
>> >   */
>> >  int run_syscall(int min, int max)
>> >  {
>> > +	struct timeval tv;
>> > +	struct timezone tz;
>> >  	struct stat stat_buf;
>> >  	int euid0;
>> >  	int proc;
>> > @@ -588,6 +590,9 @@ int run_syscall(int min, int max)
>> >  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
>> >  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
>> >  		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
>> > +		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
>> > +		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
>> 
>> Calling gettimeofday(NULL, ...) will actually segfault on glibc.
>> It works when calling through the VDSO, but not the logic in glibc
>> itself, which is guess is allowed by POSIX.
>
> Then that's shocking, because the man page says:
>
>        If either tv or tz is NULL, the corresponding structure is not  set  or
>        returned.   (However, compilation warnings will result if tv is NULL.)
>
> I'd expect glibc to at least support what'd documented in the man
> page :-/

The manual page is not part of glibc.  Neither the glibc documentation
nor the POSIX spec has ever supported NULL for the first argument.  The
generic POSIX implementation in glibc originally returned EINVAL for
that case, though.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Re: [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Zhangjin Wu 2 years, 8 months ago
> On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
> > These 3 test cases are added to cover the normal using scenes of
> > gettimeofday().
> > 
> > They have been used to trigger and fix up such issue:
> > 
> >     nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
> > 
> > This issue happens while there is no "unsigned int" conversion in the
> > new clock_gettime / clock_gettime64 syscall path of gettimeofday():
> > 
> >     tv->tv_usec = ts.tv_nsec / 1000;
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index 8ba8c2fc71a0..20d184da9a2b 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
> >   */
> >  int run_syscall(int min, int max)
> >  {
> > +	struct timeval tv;
> > +	struct timezone tz;
> >  	struct stat stat_buf;
> >  	int euid0;
> >  	int proc;
> > @@ -588,6 +590,9 @@ int run_syscall(int min, int max)
> >  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
> >  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
> >  		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
> > +		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
> > +		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
> 
> Calling gettimeofday(NULL, ...) will actually segfault on glibc.
> It works when calling through the VDSO, but not the logic in glibc
> itself, which is guess is allowed by POSIX.
>

In low level gettimeofday syscall, it should be ok, will check the non-vdso
code of glibc later. in musl, it returns 0 when it is NULL
(src/time/gettimeofday.c).

> I propose to avoid doing it :-)

Just checked it again, these two cases can also detect the issues (1):

    CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
    CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;

    (1) nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'

we can directly remove the second one ;-)

> 
> Either we gate the existing test in #ifdef NOLIBC or we remove it.

so, we still need to check the original gettimeofday_null test case for glibc?

Thanks,
Zhangjin

> 
> > +		CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
> >  		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;
Re: [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases
Posted by Thomas Weißschuh 2 years, 8 months ago
On 2023-05-30 19:28:06+0800, Zhangjin Wu wrote:
> > On 2023-05-30 14:37:49+0800, Zhangjin Wu wrote:
> > > These 3 test cases are added to cover the normal using scenes of
> > > gettimeofday().
> > > 
> > > They have been used to trigger and fix up such issue:
> > > 
> > >     nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'
> > > 
> > > This issue happens while there is no "unsigned int" conversion in the
> > > new clock_gettime / clock_gettime64 syscall path of gettimeofday():
> > > 
> > >     tv->tv_usec = ts.tv_nsec / 1000;
> > > 
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > ---
> > >  tools/testing/selftests/nolibc/nolibc-test.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > > index 8ba8c2fc71a0..20d184da9a2b 100644
> > > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > > @@ -533,6 +533,8 @@ static int test_stat_timestamps(void)
> > >   */
> > >  int run_syscall(int min, int max)
> > >  {
> > > +	struct timeval tv;
> > > +	struct timezone tz;
> > >  	struct stat stat_buf;
> > >  	int euid0;
> > >  	int proc;
> > > @@ -588,6 +590,9 @@ int run_syscall(int min, int max)
> > >  		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
> > >  		CASE_TEST(getdents64_null);   EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
> > >  		CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;
> > > +		CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
> > > +		CASE_TEST(gettimeofday_tz);   EXPECT_SYSZR(1, gettimeofday(NULL, &tz)); break;
> > 
> > Calling gettimeofday(NULL, ...) will actually segfault on glibc.
> > It works when calling through the VDSO, but not the logic in glibc
> > itself, which is guess is allowed by POSIX.
> >
> 
> In low level gettimeofday syscall, it should be ok, will check the non-vdso
> code of glibc later. in musl, it returns 0 when it is NULL
> (src/time/gettimeofday.c).

It's fine for the Linux syscall, vdso, musl and nolibc.
It's not fine for glibc and gnulib.

> > I propose to avoid doing it :-)
> 
> Just checked it again, these two cases can also detect the issues (1):
> 
>     CASE_TEST(gettimeofday_tv);   EXPECT_SYSZR(1, gettimeofday(&tv, NULL)); break;
>     CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
> 
>     (1) nolibc-test.c:(.text.gettimeofday+0x54): undefined reference to `__aeabi_ldivmod'

To be honest this does not look like a nolibc issue, more a compiler
setup problem. GCC generates calls to a function in libgcc that is not
linked for some reason.

Can you provide reproduction steps?

Also I guess a testcase that tests at runtime that the compilation has
worked is of limited use :-)
It either works or it never executes.

> we can directly remove the second one ;-)
> 
> > 
> > Either we gate the existing test in #ifdef NOLIBC or we remove it.
> 
> so, we still need to check the original gettimeofday_null test case for glibc?

The original testcase also needs adaption, yes.

> Thanks,
> Zhangjin
> 
> > 
> > > +		CASE_TEST(gettimeofday_tv_tz);EXPECT_SYSZR(1, gettimeofday(&tv, &tz)); break;
> > >  		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;
[PATCH 2/2] selftests/nolibc: add sizeof test for the new 64bit data types
Posted by Zhangjin Wu 2 years, 8 months ago
These test cases are required to make sure the new added data types are
really 64bit based.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 20d184da9a2b..43ce4d34b596 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -721,6 +721,14 @@ int run_stdlib(int min, int max)
 #else
 # warning "__SIZEOF_LONG__ is undefined"
 #endif /* __SIZEOF_LONG__ */
+		CASE_TEST(sizeof_time_t);           EXPECT_EQ(1, 8,                sizeof(time_t)); break;
+		CASE_TEST(sizeof_timespec);         EXPECT_EQ(1, 16,               sizeof(struct timespec)); break;
+#ifdef NOLIBC
+		CASE_TEST(sizeof_itimerspec);       EXPECT_EQ(1, 32,               sizeof(struct itimerspec)); break;
+#endif
+		CASE_TEST(sizeof_timeval);          EXPECT_EQ(1, 16,               sizeof(struct timeval)); break;
+		CASE_TEST(sizeof_itimerval);        EXPECT_EQ(1, 32,               sizeof(struct itimerval)); break;
+		CASE_TEST(sizeof_off_t);            EXPECT_EQ(1, 8,                sizeof(off_t)); break;
 		case __LINE__:
 			return ret; /* must be last */
 		/* note: do not set any defaults so as to permit holes above */
-- 
2.25.1
Re: [PATCH 2/2] selftests/nolibc: add sizeof test for the new 64bit data types
Posted by Thomas Weißschuh 2 years, 8 months ago
On 2023-05-30 14:42:56+0800, Zhangjin Wu wrote:
> These test cases are required to make sure the new added data types are
> really 64bit based.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 20d184da9a2b..43ce4d34b596 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -721,6 +721,14 @@ int run_stdlib(int min, int max)
>  #else
>  # warning "__SIZEOF_LONG__ is undefined"
>  #endif /* __SIZEOF_LONG__ */
> +		CASE_TEST(sizeof_time_t);           EXPECT_EQ(1, 8,                sizeof(time_t)); break;
> +		CASE_TEST(sizeof_timespec);         EXPECT_EQ(1, 16,               sizeof(struct timespec)); break;
> +#ifdef NOLIBC
> +		CASE_TEST(sizeof_itimerspec);       EXPECT_EQ(1, 32,               sizeof(struct itimerspec)); break;
> +#endif
> +		CASE_TEST(sizeof_timeval);          EXPECT_EQ(1, 16,               sizeof(struct timeval)); break;
> +		CASE_TEST(sizeof_itimerval);        EXPECT_EQ(1, 32,               sizeof(struct itimerval)); break;
> +		CASE_TEST(sizeof_off_t);            EXPECT_EQ(1, 8,                sizeof(off_t)); break;

These will break on 32bit glibc configurations.
(At least on x86)

>  		case __LINE__:
>  			return ret; /* must be last */
>  		/* note: do not set any defaults so as to permit holes above */
> -- 
> 2.25.1
>
Re: [PATCH 2/2] selftests/nolibc: add sizeof test for the new 64bit data types
Posted by Zhangjin Wu 2 years, 8 months ago
> On 2023-05-30 14:42:56+0800, Zhangjin Wu wrote:
> > These test cases are required to make sure the new added data types are
> > really 64bit based.
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/nolibc-test.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index 20d184da9a2b..43ce4d34b596 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -721,6 +721,14 @@ int run_stdlib(int min, int max)
> >  #else
> >  # warning "__SIZEOF_LONG__ is undefined"
> >  #endif /* __SIZEOF_LONG__ */
> > +		CASE_TEST(sizeof_time_t);           EXPECT_EQ(1, 8,                sizeof(time_t)); break;
> > +		CASE_TEST(sizeof_timespec);         EXPECT_EQ(1, 16,               sizeof(struct timespec)); break;
> > +#ifdef NOLIBC
> > +		CASE_TEST(sizeof_itimerspec);       EXPECT_EQ(1, 32,               sizeof(struct itimerspec)); break;
> > +#endif
> > +		CASE_TEST(sizeof_timeval);          EXPECT_EQ(1, 16,               sizeof(struct timeval)); break;
> > +		CASE_TEST(sizeof_itimerval);        EXPECT_EQ(1, 32,               sizeof(struct itimerval)); break;
> > +		CASE_TEST(sizeof_off_t);            EXPECT_EQ(1, 8,                sizeof(off_t)); break;
> 
> These will break on 32bit glibc configurations.
> (At least on x86)

Yes, I added a big #ifdef at first, but narrowed it down after a default
x86_64 gcc+glibc test, 32bit has been ignored from my mind ;-(

Will add the big #ifdef back.

Thanks,
Zhangjin

> 
> >  		case __LINE__:
> >  			return ret; /* must be last */
> >  		/* note: do not set any defaults so as to permit holes above */
> > --