linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zhangjin Wu <falcon@tinylab.org>
To: thomas@t-8ch.de, w@1wt.eu
Cc: falcon@tinylab.org, aou@eecs.berkeley.edu, arnd@arndb.de,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	paul.walmsley@sifive.com, shuah@kernel.org
Subject: Re: [PATCH] selftests/nolibc: Fix up compile error for rv32
Date: Wed, 24 May 2023 02:03:40 +0800	[thread overview]
Message-ID: <20230523180340.466619-1-falcon@tinylab.org> (raw)
In-Reply-To: <20230521180823.164289-1-falcon@tinylab.org>

Hi, Willy, Thomas

Good news, I just fixed up all of the time32 syscalls related build and
test failures for rv32 and plan to send out the whole patchset tomorrow.

The patchset is based on 20230520-nolibc-rv32+stkp2 of [1] and the new
statckprotect patchset [2] (If v2 is ready, I prefer to rebase on v2).

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git
[2]: https://lore.kernel.org/linux-riscv/20230521100818.GA29408@1wt.eu/T/#t

For the fstat compile issue, I prefer to use a __NR_statx for rv32
instead of using fstatfs, because of different fstatfs* have different
errno's, it is ugly to use lots of macros ;-) what's your suggestion?

Here is the __NR_statx version:

    +static int test_syscall_args(void)
    +{
    +#ifdef __NR_statx
    +	return syscall(__NR_statx, 0, NULL, 0, 0, NULL);
    +#elif defined(__NR_fstat)
    +	return syscall(__NR_fstat, 0, NULL);
    +#else
    +#error Neither __NR_statx nor __NR_fstat defined, cannot implement syscall_args test
    +#endif
    +}

And the fstatfs version:

    +#ifdef __NR_fstatfs64
    +#define EXPECT_SYSER_SYSCALL_ARGS() EXPECT_SYSER(1, syscall(__NR_fstatfs64, 0, 0, NULL), -1, EINVAL)
    +#elif defined(__NR_fstatfs)
    +#define EXPECT_SYSER_SYSCALL_ARGS() EXPECT_SYSER(1, syscall(__NR_fstatfs, 0, NULL), -1, EFAULT)
    +#elif defined(__NR_fstat)
    +#define EXPECT_SYSER_SYSCALL_ARGS() EXPECT_SYSER(1, syscall(__NR_fstat, 0, NULL), -1, EFAULT)
    +#else
    +#error None of __NR_fstatfs64, __NR_fstatfs and __NR_fstat defined, cannot implement syscall_args test
    +#endif

And I plan to move __NR_fstat as the first branch to make sure it works
as before on the other platforms.

>     30 fork = 1 ENOSYS                                              [FAIL]
>     33 gettimeofday_null = -1 ENOSYS                                [FAIL]
>     35 gettimeofday_bad1 = -1 ENOSYS  != (-1 EFAULT)                [FAIL]
>     36 gettimeofday_bad2 = -1 ENOSYS  != (-1 EFAULT)                [FAIL]
>     37 gettimeofday_bad2 = -1 ENOSYS  != (-1 EFAULT)                [FAIL]
>     51 poll_null = -1 ENOSYS                                        [FAIL]
>     52 poll_stdout = -1 ENOSYS                                      [FAIL]
>     53 poll_fault = -1 ENOSYS  != (-1 EFAULT)                       [FAIL]
>     56 select_null = -1 ENOSYS                                      [FAIL]
>     57 select_stdout = -1 ENOSYS                                    [FAIL]
>     58 select_fault = -1 ENOSYS  != (-1 EFAULT)                     [FAIL]
>     64 wait_child = -1 ENOSYS  != (-1 ECHILD)                       [FAIL]
>     65 waitpid_min = -1 ENOSYS  != (-1 ESRCH)                       [FAIL]
>     66 waitpid_child = -1 ENOSYS  != (-1 ECHILD)                    [FAIL]
>

All of the above failures have been fixed up, some of them are very
easy, some of them are a little hard.

1. 30, 64-66 depends on wait4, use waitid instead
2. 33-37 depends on gettimeofday, use clock_gettime64 instead
3. 51-53 depends on ppoll, use ppoll_time64 instead
4. 56-58 depends on pselect*, use pselect_time64 instead

And I have found there are two same 'gettimeofday_bad2', is it designed?
If no, I will send a patch to dedup it:

    CASE_TEST(gettimeofday_bad2); EXPECT_SYSER(1, gettimeofday(NULL, (void *)1), -1, EFAULT); break;
    CASE_TEST(gettimeofday_bad2); EXPECT_SYSER(1, gettimeofday(NULL, (void *)1), -1, EFAULT); break;

> 
> My suggestion is directly fix up the failures one by one in current stage,
> because nolibc currently only uses part of the time32 syscalls, it may be not
> that complex to fix up them. Have read the code of musl and glibc today, both
> of them have good time64 syscalls support, I plan to fix up the above failures
> in several days, hope so but no promise ;-)
>

both musl and glibc help a lot, but also encounter some critical issues, for
example, to pass some test cases, it is required to emulate the same path of
the target syscalls and return the same errno's for them, the code comment will
exaplain the details.

> And another question: for the new changes, If a platform support both of the
> 32bit and 64bit syscalls, do we need to put the 64bit syscalls at first?
>

To make sure not touch too much on the code and reduce test cost, the patchset
just kept the default code order and let the old code in the first branch.

I plan to send the whole patchset tomorrow.

Thanks,
Zhangjin

  reply	other threads:[~2023-05-23 18:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-20  9:53 [PATCH] tools/nolibc: riscv: add stackprotector support Thomas Weißschuh
2023-05-20 12:02 ` [PATCH] selftests/nolibc: Fix up compile error for rv32 Zhangjin Wu
2023-05-20 13:32   ` Willy Tarreau
2023-05-20 14:07     ` Thomas Weißschuh
2023-05-20 14:13       ` Willy Tarreau
2023-05-20 14:00   ` Thomas Weißschuh
2023-05-20 14:09     ` Willy Tarreau
2023-05-20 18:30       ` Zhangjin Wu
2023-05-21  3:58         ` Willy Tarreau
2023-05-21 18:08           ` Zhangjin Wu
2023-05-23 18:03             ` Zhangjin Wu [this message]
2023-05-23 18:56               ` Willy Tarreau
2023-05-20 13:52 ` Zhangjin Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230523180340.466619-1-falcon@tinylab.org \
    --to=falcon@tinylab.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=shuah@kernel.org \
    --cc=thomas@t-8ch.de \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).