2023-05-30 11:04:00

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH 0/4] selftests/nolibc: add user-space 'efault' handler

Hi, Willy, Thomas

This is not really for merge, but only let it work as a demo code to
test whether it is possible to restore the next test when there is a bad
pointer access in user-space [1].

Besides, a new 'run' command is added to 'NOLIBC_TEST' environment
variable or arguments to control the running iterations, this may be
used to test the reentrancy issues, but no failures found currently ;-)

With glibc, it works as following:

$ ./nolibc-test run:2,syscall:28-30,stdlib:1
Running iteration(s): 2

Current iteration: 1

Running test 'syscall', from 28 to 30
28 dup3_m1 = -1 EBADF [OK]
29 efault_handler ! 11 SIGSEGV [OK]
30 execve_root = -1 EACCES [OK]
Errors during this test: 0

Running test 'stdlib'
1 getenv_blah = <(null)> [OK]
Errors during this test: 0

Total number of errors in the 1 iteration(s): 0

Current iteration: 2

Running test 'syscall'
28 dup3_m1 = -1 EBADF [OK]
29 efault_handler ! 11 SIGSEGV [OK]
30 execve_root = -1 EACCES [OK]
Errors during this test: 0

Running test 'stdlib'
1 getenv_blah = <(null)> [OK]
Errors during this test: 0

Total number of errors in the 2 iteration(s): 0

With nolibc, it will be skipped (run:2,syscall:28-30,stdlib:10):

Running iteration(s): 2

Current iteration: 1

Running test 'syscall', from 28 to 30
28 dup3_m1 = -1 EBADF [OK]
29 efault_handler [SKIPPED]
30 execve_root = -1 EACCES [OK]
Errors during this test: 0

Running test 'stdlib', from 10 to 10
10 strrchr_foobar_o = <obar> [OK]
Errors during this test: 0

Total number of errors in the 1 iteration(s): 0

Current iteration: 2

Running test 'syscall', from 28 to 30
28 dup3_m1 = -1 EBADF [OK]
29 efault_handler [SKIPPED]
30 execve_root = -1 EACCES [OK]
Errors during this test: 0

Running test 'stdlib', from 10 to 10
10 strrchr_foobar_o = <obar> [OK]
Errors during this test: 0

Total number of errors in the 2 iteration(s): 0

Best regards,
Zhangjin
---

[1]: https://lore.kernel.org/linux-riscv/[email protected]/

Zhangjin Wu (4):
selftests/nolibc: allow rerun with the same settings
selftests/nolibc: add rerun support
selftests/nolibc: add user space efault handler
selftests/nolibc: add user-space efault restore test case

tools/testing/selftests/nolibc/nolibc-test.c | 247 +++++++++++++++++--
1 file changed, 221 insertions(+), 26 deletions(-)

--
2.25.1



2023-05-30 11:11:05

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH 1/4] selftests/nolibc: allow rerun with the same settings

Record the user settings from NOLIBC_TEST and allow reuse them in
another run iteration.

This allows to rerun the test cases with the same setting.

Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 46 ++++++++++++--------
1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index fd7515f6b1d2..be718fa5dc86 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -46,6 +46,9 @@ char **environ;
/* definition of a series of tests */
struct test {
const char *name; /* test name */
+ int min;
+ int max;
+ int run;
int (*func)(int min, int max); /* handler */
};

@@ -940,12 +943,12 @@ int prepare(void)
}

/* This is the definition of known test names, with their functions */
-static const struct test test_names[] = {
+static struct test test_names[] = {
/* add new tests here */
- { .name = "syscall", .func = run_syscall },
- { .name = "stdlib", .func = run_stdlib },
- { .name = "vfprintf", .func = run_vfprintf },
- { .name = "protection", .func = run_protection },
+ { .name = "syscall", .min = 0, .max = INT_MAX, .run = -1, .func = run_syscall },
+ { .name = "stdlib", .min = 0, .max = INT_MAX, .run = -1, .func = run_stdlib },
+ { .name = "vfprintf", .min = 0, .max = INT_MAX, .run = -1, .func = run_vfprintf },
+ { .name = "protection", .min = 0, .max = INT_MAX, .run = -1, .func = run_protection },
{ 0 }
};

@@ -994,7 +997,11 @@ int main(int argc, char **argv, char **envp)
break;
}

- if (test_names[idx].name) {
+ if (!test_names[idx].name) {
+ printf("Ignoring unknown test name '%s'\n", test);
+ } else {
+ test_names[idx].run = 1;
+
/* The test was named, it will be called at least
* once. We may have an optional range at <colon>
* here, which defaults to the full range.
@@ -1022,27 +1029,32 @@ int main(int argc, char **argv, char **envp)
value = colon;
}

- /* now's time to call the test */
- printf("Running test '%s'\n", test_names[idx].name);
- err = test_names[idx].func(min, max);
- ret += err;
- printf("Errors during this test: %d\n\n", err);
+ test_names[idx].min = min;
+ test_names[idx].max = max;
} while (colon && *colon);
- } else
- printf("Ignoring unknown test name '%s'\n", test);
+ }

test = comma;
} while (test && *test);
- } else {
- /* no test mentioned, run everything */
+
+ /* disable the left tests */
for (idx = 0; test_names[idx].name; idx++) {
- printf("Running test '%s'\n", test_names[idx].name);
- err = test_names[idx].func(min, max);
+ if (test_names[idx].run != 1)
+ test_names[idx].run = 0;
+ }
+ }
+
+ /* run everything or the test mentioned */
+ for (idx = 0; test_names[idx].name; idx++) {
+ if (test_names[idx].run != 0) {
+ printf("Running test '%s', from %d to %d\n", test_names[idx].name, test_names[idx].min, test_names[idx].max);
+ err = test_names[idx].func(test_names[idx].min, test_names[idx].max);
ret += err;
printf("Errors during this test: %d\n\n", err);
}
}

+
printf("Total number of errors: %d\n", ret);

if (getpid() == 1) {
--
2.25.1


2023-05-30 11:39:14

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH 3/4] selftests/nolibc: add user space efault handler

Some hooks are added to record the test case and the test context, while
traps on invalid data pointer access, try to continue next test if
possible.

Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 151 ++++++++++++++++++-
1 file changed, 149 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index b8fd7fcf56a6..9f9a09529a4f 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -114,6 +114,149 @@ const char *errorname(int err)
}
}

+/* emulate EFAULT return in user space with isigaction/sigsetjmp/siglongjmp */
+#ifndef NOLIBC
+#ifndef NO_USER_SPACE_EFAULT
+#define USER_SPACE_EFAULT
+#endif
+#endif
+
+#ifdef USER_SPACE_EFAULT
+#include <setjmp.h>
+
+static int next_test = 0;
+static int test_llen = 0;
+static int test_sig = 0;
+static int expect_sig = 0;
+static int test_idx = 0;
+static int test_ret = 0;
+static int test_iteration = 0;
+static int test_iterations = 0;
+static sigjmp_buf mark;
+
+static int pad_spc(int llen, int cnt, const char *fmt, ...);
+static struct test test_names[];
+typedef int (*func_t)(int min, int max);
+static func_t test_func = NULL;
+
+#define CASE_SIG(sig) \
+ case sig: return #sig
+
+/* returns the signal name or the decimal value for less common ones. */
+const char *signame(int sig)
+{
+ switch (sig) {
+ CASE_SIG(SIGSEGV);
+ default:
+ return itoa(sig);
+ }
+}
+
+static void record_test_context(int idx, int iteration, int iterations)
+{
+ test_idx = idx;
+ test_iteration = iteration;
+ test_iterations = iterations;
+}
+
+static void record_test_case(int test, int llen, int ret, char *name)
+{
+ test_llen = llen - 1;
+ test_ret = ret;
+ next_test = test + 1;
+}
+
+static void restore_from_trap(void)
+{
+ int idx;
+ int err;
+ int i;
+ int min = 0;
+ int max = INT_MAX;
+
+ test_llen += printf(" ! %d %s ", test_sig, signame(test_sig));
+ if (test_sig == expect_sig)
+ pad_spc(test_llen, 64, "[OK]\n");
+ else {
+ test_ret++;
+ pad_spc(test_llen, 64, "[FAIL]\n");
+ }
+
+ if (next_test <= test_names[test_idx].max) {
+ test_func = test_names[test_idx].func;
+ err = test_func(next_test, test_names[test_idx].max);
+ test_ret += err;
+ printf("Errors during this test: %d\n\n", err);
+ }
+
+ for (i = test_iteration; i < test_iterations; i++) {
+ /* for current iterations */
+ if (i == test_iteration) {
+ idx = test_idx + 1;
+ } else {
+ printf("Current iteration: %d\n\n", i + 1);
+
+ /* for left iterations */
+ idx = 0;
+ test_ret = 0;
+ }
+
+ for (; test_names[idx].name; idx++) {
+ if (test_names[idx].run != 0) {
+ printf("Running test '%s'\n", test_names[idx].name);
+ record_test_context(idx, i, test_iterations);
+ err = test_names[idx].func(test_names[idx].min, test_names[idx].max);
+ test_ret += err;
+ printf("Errors during this test: %d\n\n", err);
+ }
+ }
+ printf("Total number of errors in the %d iteration(s): %d\n\n", i + 1, test_ret);
+ }
+}
+
+static void trap_handler(int sig, siginfo_t *si, void *p)
+{
+ test_sig = sig;
+ if (sig != SIGKILL)
+ siglongjmp(mark, -1);
+}
+
+static void register_expect_trap(int experr1, int experr2)
+{
+ if (experr1 == EFAULT || experr2 == EFAULT)
+ expect_sig = SIGSEGV;
+ else
+ expect_sig = 0;
+}
+
+static void register_trap_handler(void)
+{
+ int ret = 0;
+
+ struct sigaction sa = {0};
+ sa.sa_sigaction = trap_handler;
+ sa.sa_flags = SA_SIGINFO;
+ ret = sigaction(SIGSEGV, &sa, NULL);
+ if (ret == -1) {
+ perror("sigaction");
+ exit(1);
+ }
+
+ if (sigsetjmp(mark, 1) != 0) {
+ restore_from_trap();
+ exit(0);
+ }
+}
+
+#define has_user_space_efault() (1)
+#else
+#define record_test_context(idx, iteration, iterations) do { } while (0)
+#define record_test_case(test, llen, name, ret) do { } while (0)
+#define register_expect_trap(experr1, experr2) do { } while (0)
+#define register_trap_handler() do { } while (0)
+#define has_user_space_efault() (0)
+#endif
+
static void putcharn(char c, size_t n)
{
char buf[64];
@@ -304,7 +447,7 @@ 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)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else { register_expect_trap(experr1, experr2); ret += expect_syserr2(expr, expret, experr1, experr2, llen); } } while (0)

#define EXPECT_SYSER(cond, expr, expret, experr) \
EXPECT_SYSER2(cond, expr, expret, experr, 0)
@@ -439,7 +582,7 @@ static int expect_strne(const char *expr, int llen, const char *cmp)

/* declare tests based on line numbers. There must be exactly one test per line. */
#define CASE_TEST(name) \
- case __LINE__: llen += printf("%d %s", test, #name);
+ case __LINE__: llen += printf("%d %s", test, #name); record_test_case(test, llen, ret, #name);


/* used by some syscall tests below */
@@ -974,6 +1117,9 @@ int main(int argc, char **argv, char **envp)
if (getpid() == 1)
prepare();

+ /* register exception restore support if enabled */
+ register_trap_handler();
+
/* the definition of a series of tests comes from either argv[1] or the
* "NOLIBC_TEST" environment variable. It's made of a comma-delimited
* series of test names and optional ranges:
@@ -1071,6 +1217,7 @@ int main(int argc, char **argv, char **envp)
for (idx = 0; test_names[idx].name; idx++) {
if (test_names[idx].run != 0) {
printf("Running test '%s', from %d to %d\n", test_names[idx].name, test_names[idx].min, test_names[idx].max);
+ record_test_context(idx, i, run);
err = test_names[idx].func(test_names[idx].min, test_names[idx].max);
ret += err;
printf("Errors during this test: %d\n\n", err);
--
2.25.1


2023-05-30 11:42:24

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH 4/4] selftests/nolibc: add user-space efault restore test case

while the libc supports sigaction/sigsetjmp/siglongjump, it is able to
restore next test after an invalid data pointer access, add such a test
case for these libcs, otherwise, skip it.

With glibc/musl:

29 efault_handler ! 11 SIGSEGV [OK]

With current nolibc:

29 efault_handler [SKIPPED]

Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 9f9a09529a4f..6b4ebe4be4d6 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -248,6 +248,15 @@ static void register_trap_handler(void)
}
}

+static int test_efault(void)
+{
+ char *addr = (void *)1;
+
+ *addr = 'a';
+
+ return -1;
+}
+
#define has_user_space_efault() (1)
#else
#define record_test_context(idx, iteration, iterations) do { } while (0)
@@ -255,6 +264,7 @@ static void register_trap_handler(void)
#define register_expect_trap(experr1, experr2) do { } while (0)
#define register_trap_handler() do { } while (0)
#define has_user_space_efault() (0)
+#define test_efault(addr) (-1)
#endif

static void putcharn(char c, size_t n)
@@ -690,6 +700,7 @@ int run_syscall(int min, int max)
struct stat stat_buf;
int euid0;
int proc;
+ int efault;
int test;
int tmp;
int ret = 0;
@@ -701,6 +712,9 @@ int run_syscall(int min, int max)
/* this will be used to skip certain tests that can't be run unprivileged */
euid0 = geteuid() == 0;

+ /* user-space efault handler support */
+ efault = has_user_space_efault();
+
for (test = min; test >= 0 && test <= max; test++) {
int llen = 0; /* line length */

@@ -737,6 +751,7 @@ int run_syscall(int min, int max)
CASE_TEST(dup2_m1); tmp = dup2(-1, 100); EXPECT_SYSER(1, tmp, -1, EBADF); if (tmp != -1) close(tmp); break;
CASE_TEST(dup3_0); tmp = dup3(0, 100, 0); EXPECT_SYSNE(1, tmp, -1); close(tmp); break;
CASE_TEST(dup3_m1); tmp = dup3(-1, 100, 0); EXPECT_SYSER(1, tmp, -1, EBADF); if (tmp != -1) close(tmp); break;
+ CASE_TEST(efault_handler); EXPECT_SYSER(efault, test_efault(), -1, EFAULT); break;
CASE_TEST(execve_root); EXPECT_SYSER(1, execve("/", (char*[]){ [0] = "/", [1] = NULL }, NULL), -1, EACCES); break;
CASE_TEST(fork); EXPECT_SYSZR(1, test_fork()); break;
CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
--
2.25.1


2023-05-30 11:44:37

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH 2/4] selftests/nolibc: add rerun support

The global 'run' setting is added to allow specify the running
iterations, for example:

NOLIBC_TEST=run:5,syscall:1

This setting allows to run the first syscall for 5 times.

Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 53 ++++++++++++++------
1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index be718fa5dc86..b8fd7fcf56a6 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -956,9 +956,12 @@ int main(int argc, char **argv, char **envp)
{
int min = 0;
int max = INT_MAX;
+ int run = 1;
+ int selected = 0;
int ret = 0;
int err;
int idx;
+ int i;
char *test;

environ = envp;
@@ -974,7 +977,7 @@ int main(int argc, char **argv, char **envp)
/* the definition of a series of tests comes from either argv[1] or the
* "NOLIBC_TEST" environment variable. It's made of a comma-delimited
* series of test names and optional ranges:
- * syscall:5-15[:.*],stdlib:8-10
+ * run:3,syscall:5-15[:.*],stdlib:8-10
*/
test = argv[1];
if (!test)
@@ -992,14 +995,27 @@ int main(int argc, char **argv, char **envp)
if (colon)
*(colon++) = '\0';

- for (idx = 0; test_names[idx].name; idx++) {
- if (strcmp(test, test_names[idx].name) == 0)
+ if (strcmp(test, "run") != 0) {
+ for (idx = 0; test_names[idx].name; idx++) {
+ if (strcmp(test, test_names[idx].name) == 0)
+ break;
+ }
+ } else {
+ value = colon;
+ if (value && *value)
+ run = atoi(value);
+
+ test = comma;
+ if (test && *test)
+ continue;
+ else
break;
}

if (!test_names[idx].name) {
printf("Ignoring unknown test name '%s'\n", test);
} else {
+ selected++;
test_names[idx].run = 1;

/* The test was named, it will be called at least
@@ -1038,24 +1054,31 @@ int main(int argc, char **argv, char **envp)
} while (test && *test);

/* disable the left tests */
- for (idx = 0; test_names[idx].name; idx++) {
- if (test_names[idx].run != 1)
- test_names[idx].run = 0;
+ if (selected != 0) {
+ for (idx = 0; test_names[idx].name; idx++) {
+ if (test_names[idx].run != 1)
+ test_names[idx].run = 0;
+ }
}
}

/* run everything or the test mentioned */
- for (idx = 0; test_names[idx].name; idx++) {
- if (test_names[idx].run != 0) {
- printf("Running test '%s', from %d to %d\n", test_names[idx].name, test_names[idx].min, test_names[idx].max);
- err = test_names[idx].func(test_names[idx].min, test_names[idx].max);
- ret += err;
- printf("Errors during this test: %d\n\n", err);
- }
- }
+ printf("Running iteration(s): %d\n\n", run);
+ for (i = 0; i < run; i++) {
+ printf("Current iteration: %d\n\n", i + 1);

+ ret = 0;
+ for (idx = 0; test_names[idx].name; idx++) {
+ if (test_names[idx].run != 0) {
+ printf("Running test '%s', from %d to %d\n", test_names[idx].name, test_names[idx].min, test_names[idx].max);
+ err = test_names[idx].func(test_names[idx].min, test_names[idx].max);
+ ret += err;
+ printf("Errors during this test: %d\n\n", err);
+ }
+ }

- printf("Total number of errors: %d\n", ret);
+ printf("Total number of errors in the %d iteration(s): %d\n\n", i + 1, ret);
+ }

if (getpid() == 1) {
/* we're running as init, there's no other process on the
--
2.25.1


2023-06-04 11:36:38

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 0/4] selftests/nolibc: add user-space 'efault' handler

Hi Zhangjin,

On Tue, May 30, 2023 at 06:47:38PM +0800, Zhangjin Wu wrote:
> Hi, Willy, Thomas
>
> This is not really for merge, but only let it work as a demo code to
> test whether it is possible to restore the next test when there is a bad
> pointer access in user-space [1].
>
> Besides, a new 'run' command is added to 'NOLIBC_TEST' environment
> variable or arguments to control the running iterations, this may be
> used to test the reentrancy issues, but no failures found currently ;-)

Since the tests we're running are essentially API tests, I'm having
a hard time seeing in which case it can be useful to repeat the tests.
I'm not necessarily against doing it, I'm used to repeating tests for
example in anything sensitive to timing or race conditions, it's just
that here I'm not seeing the benefit. And the fact you found no failure
is rather satisfying because the opposite would have surprised me.

Regarding the efault handler, I don't think it's a good idea until we
have signal+longjmp support in nolibc. Because running different tests
with different libcs kind of defeats the purpose of the test in the
first place. The reason why I wanted nolibc-test to be portable to at
least one other libc is to help the developer figure if a failure is in
the nolibc syscall they're implementing or in the test itself. Here if
we start to say that some parts cannot be tested similarly, the benefit
disappears.

I mentioned previously that I'm not particularly impatient to work on
signals and longjmp. But in parallel I understand how this can make the
life of some developers easier and even allow to widen the spectrum of
some tests. Thus, maybe in the end it could be beneficial to make progress
on this front and support these. We should make sure that this doesn't
inflate the code base however. I guess I'd be fine with ignoring libc-
based restarts on EINTR, alt stacks and so on and keeping this minimal
(i.e. catch a segfault/bus error/sigill in a test program, or a Ctrl-C
in a tiny shell).

Just let us know if you think that's something you could be interested
in exploring. There might be differences between architectures, I have
not checked.

Thanks,
Willy

2023-06-04 19:24:06

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 0/4] selftests/nolibc: add user-space 'efault' handler

On Sun, Jun 04, 2023 at 09:07:25PM +0200, Thomas Wei?schuh wrote:
> On 2023-06-04 13:05:18+0200, Willy Tarreau wrote:
> > Hi Zhangjin,
> >
> > On Tue, May 30, 2023 at 06:47:38PM +0800, Zhangjin Wu wrote:
> > > Hi, Willy, Thomas
> > >
> > > This is not really for merge, but only let it work as a demo code to
> > > test whether it is possible to restore the next test when there is a bad
> > > pointer access in user-space [1].
> > >
> > > Besides, a new 'run' command is added to 'NOLIBC_TEST' environment
> > > variable or arguments to control the running iterations, this may be
> > > used to test the reentrancy issues, but no failures found currently ;-)
> >
> > Since the tests we're running are essentially API tests, I'm having
> > a hard time seeing in which case it can be useful to repeat the tests.
> > I'm not necessarily against doing it, I'm used to repeating tests for
> > example in anything sensitive to timing or race conditions, it's just
> > that here I'm not seeing the benefit. And the fact you found no failure
> > is rather satisfying because the opposite would have surprised me.
> >
> > Regarding the efault handler, I don't think it's a good idea until we
> > have signal+longjmp support in nolibc. Because running different tests
> > with different libcs kind of defeats the purpose of the test in the
> > first place. The reason why I wanted nolibc-test to be portable to at
> > least one other libc is to help the developer figure if a failure is in
> > the nolibc syscall they're implementing or in the test itself. Here if
> > we start to say that some parts cannot be tested similarly, the benefit
> > disappears.
> >
> > I mentioned previously that I'm not particularly impatient to work on
> > signals and longjmp. But in parallel I understand how this can make the
> > life of some developers easier and even allow to widen the spectrum of
> > some tests. Thus, maybe in the end it could be beneficial to make progress
> > on this front and support these. We should make sure that this doesn't
> > inflate the code base however. I guess I'd be fine with ignoring libc-
> > based restarts on EINTR, alt stacks and so on and keeping this minimal
> > (i.e. catch a segfault/bus error/sigill in a test program, or a Ctrl-C
> > in a tiny shell).
> >
> > Just let us know if you think that's something you could be interested
> > in exploring. There might be differences between architectures, I have
> > not checked.
>
> If the goal is to handle hard errors like segfaults more gracefully,
> would it not be easier to run each testcase in a subprocess?
>
> Then we can just check if the child exited successfully.
>
> It should also be completely architecture agnostic.

Could be, indeed. However it would complexify a bit strace debugging,
but yeah that might be something to think about.

Willy

2023-06-04 19:29:59

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH 0/4] selftests/nolibc: add user-space 'efault' handler

On 2023-06-04 13:05:18+0200, Willy Tarreau wrote:
> Hi Zhangjin,
>
> On Tue, May 30, 2023 at 06:47:38PM +0800, Zhangjin Wu wrote:
> > Hi, Willy, Thomas
> >
> > This is not really for merge, but only let it work as a demo code to
> > test whether it is possible to restore the next test when there is a bad
> > pointer access in user-space [1].
> >
> > Besides, a new 'run' command is added to 'NOLIBC_TEST' environment
> > variable or arguments to control the running iterations, this may be
> > used to test the reentrancy issues, but no failures found currently ;-)
>
> Since the tests we're running are essentially API tests, I'm having
> a hard time seeing in which case it can be useful to repeat the tests.
> I'm not necessarily against doing it, I'm used to repeating tests for
> example in anything sensitive to timing or race conditions, it's just
> that here I'm not seeing the benefit. And the fact you found no failure
> is rather satisfying because the opposite would have surprised me.
>
> Regarding the efault handler, I don't think it's a good idea until we
> have signal+longjmp support in nolibc. Because running different tests
> with different libcs kind of defeats the purpose of the test in the
> first place. The reason why I wanted nolibc-test to be portable to at
> least one other libc is to help the developer figure if a failure is in
> the nolibc syscall they're implementing or in the test itself. Here if
> we start to say that some parts cannot be tested similarly, the benefit
> disappears.
>
> I mentioned previously that I'm not particularly impatient to work on
> signals and longjmp. But in parallel I understand how this can make the
> life of some developers easier and even allow to widen the spectrum of
> some tests. Thus, maybe in the end it could be beneficial to make progress
> on this front and support these. We should make sure that this doesn't
> inflate the code base however. I guess I'd be fine with ignoring libc-
> based restarts on EINTR, alt stacks and so on and keeping this minimal
> (i.e. catch a segfault/bus error/sigill in a test program, or a Ctrl-C
> in a tiny shell).
>
> Just let us know if you think that's something you could be interested
> in exploring. There might be differences between architectures, I have
> not checked.

If the goal is to handle hard errors like segfaults more gracefully,
would it not be easier to run each testcase in a subprocess?

Then we can just check if the child exited successfully.

It should also be completely architecture agnostic.

Thomas

2023-06-06 04:29:27

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH 0/4] selftests/nolibc: add user-space 'efault' handler

> On 2023-06-04 13:05:18+0200, Willy Tarreau wrote:
> > Hi Zhangjin,
> >
> > On Tue, May 30, 2023 at 06:47:38PM +0800, Zhangjin Wu wrote:
> > > Hi, Willy, Thomas
> > >
> >
> > Just let us know if you think that's something you could be interested
> > in exploring. There might be differences between architectures, I have
> > not checked.
>
> If the goal is to handle hard errors like segfaults more gracefully,
> would it not be easier to run each testcase in a subprocess?
>
> Then we can just check if the child exited successfully.
>

Yeah, it is easier, it may be possible to simply pass the test case to
something like test_fork() and let the child to run it there.

I will take a try, thanks very much.

> It should also be completely architecture agnostic.

It is for we can reuse the test_fork() stuff.

Best regards,
Zhangjin

>
> Thomas