Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed
On 06/09/2017 03:26, Shuah Khan wrote: > On 09/05/2017 02:38 PM, Mickaël Salaün wrote: >> >> >> On 28/08/2017 18:42, Shuah Khan wrote: >>> On 08/06/2017 05:23 PM, Mickaël Salaün wrote: When a test process is not able to write to TH_LOG_STREAM, this step mechanism enable to print the assert number which triggered the failure. This can be enabled by setting _metadata->no_print to true at the beginning of the test sequence. Update the seccomp-bpf test to return 0 if a test succeeded. This feature is needed for the Landlock tests. Signed-off-by: Mickaël Salaün Cc: Andy Lutomirski Cc: Kees Cook Cc: Shuah Khan Cc: Will Drewry Link: https://lkml.kernel.org/r/cagxu5j+d-fp8kt9unnoqkrqjp4dytpmgkjxwykzyryivpz3...@mail.gmail.com >>> >>> I am looking through my Inbox and found this one. Okay to pull >>> this in for 4.14-rc1? >> >> Yes, could you please pull this one instead of the one from my Landlock >> patch series [1] (which is already in your tree)? This patch is more >> up-to-date and include documentation. >> > > Now this patch is in linux-kselftest next for 4.14-rc1 instead of > the one from your landlock series. Please take a look and see if > I git it right. Yes, it is right. Thanks! Mickaël signature.asc Description: OpenPGP digital signature
Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed
On 09/05/2017 02:38 PM, Mickaël Salaün wrote: > > > On 28/08/2017 18:42, Shuah Khan wrote: >> On 08/06/2017 05:23 PM, Mickaël Salaün wrote: >>> When a test process is not able to write to TH_LOG_STREAM, this step >>> mechanism enable to print the assert number which triggered the failure. >>> This can be enabled by setting _metadata->no_print to true at the >>> beginning of the test sequence. >>> >>> Update the seccomp-bpf test to return 0 if a test succeeded. >>> >>> This feature is needed for the Landlock tests. >>> >>> Signed-off-by: Mickaël Salaün >>> Cc: Andy Lutomirski >>> Cc: Kees Cook >>> Cc: Shuah Khan >>> Cc: Will Drewry >>> Link: >>> https://lkml.kernel.org/r/cagxu5j+d-fp8kt9unnoqkrqjp4dytpmgkjxwykzyryivpz3...@mail.gmail.com >> >> I am looking through my Inbox and found this one. Okay to pull >> this in for 4.14-rc1? > > Yes, could you please pull this one instead of the one from my Landlock > patch series [1] (which is already in your tree)? This patch is more > up-to-date and include documentation. > Now this patch is in linux-kselftest next for 4.14-rc1 instead of the one from your landlock series. Please take a look and see if I git it right. thanks, -- Shuah signature.asc Description: OpenPGP digital signature
Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed
On 28/08/2017 18:42, Shuah Khan wrote: > On 08/06/2017 05:23 PM, Mickaël Salaün wrote: >> When a test process is not able to write to TH_LOG_STREAM, this step >> mechanism enable to print the assert number which triggered the failure. >> This can be enabled by setting _metadata->no_print to true at the >> beginning of the test sequence. >> >> Update the seccomp-bpf test to return 0 if a test succeeded. >> >> This feature is needed for the Landlock tests. >> >> Signed-off-by: Mickaël Salaün >> Cc: Andy Lutomirski >> Cc: Kees Cook >> Cc: Shuah Khan >> Cc: Will Drewry >> Link: >> https://lkml.kernel.org/r/cagxu5j+d-fp8kt9unnoqkrqjp4dytpmgkjxwykzyryivpz3...@mail.gmail.com > > I am looking through my Inbox and found this one. Okay to pull > this in for 4.14-rc1? Yes, could you please pull this one instead of the one from my Landlock patch series [1] (which is already in your tree)? This patch is more up-to-date and include documentation. Thanks, Mickaël [1] https://lkml.kernel.org/r/af24658e-4225-118e-59aa-0b78d6227...@kernel.org >> --- >> >> Changes since the patch from the Landlock series: >> * add the step counter in assert/expect macros and use _metadata to >> enable the counter (suggested by Kees Cook) >> * only count asserts >> * add documentation >> --- >> tools/testing/selftests/kselftest_harness.h | 39 >> +++ >> tools/testing/selftests/seccomp/seccomp_bpf.c | 2 +- >> 2 files changed, 35 insertions(+), 6 deletions(-) >> >> diff --git a/tools/testing/selftests/kselftest_harness.h >> b/tools/testing/selftests/kselftest_harness.h >> index c56f72e07cd7..e81bd28bdd89 100644 >> --- a/tools/testing/selftests/kselftest_harness.h >> +++ b/tools/testing/selftests/kselftest_harness.h >> @@ -51,6 +51,9 @@ >> #define __KSELFTEST_HARNESS_H >> >> #define _GNU_SOURCE >> +#include >> +#include >> +#include >> #include >> #include >> #include >> @@ -84,6 +87,14 @@ >> * E.g., #define TH_LOG_ENABLED 1 >> * >> * If no definition is provided, logging is enabled by default. >> + * >> + * If there is no way to print an error message for the process running the >> + * test (e.g. not allowed to write to stderr), it is still possible to get >> the >> + * ASSERT_* number for which the test failed. This behavior can be enabled >> by >> + * writing `_metadata->no_print = true;` before the check sequence that is >> + * unable to print. When an error occur, instead of printing an error >> message >> + * and calling `abort(3)`, the test process call `_exit(2)` with the assert >> + * number as argument, which is then printed by the parent process. >> */ >> #define TH_LOG(fmt, ...) do { \ >> if (TH_LOG_ENABLED) \ >> @@ -555,12 +566,18 @@ >> * return while still providing an optional block to the API consumer. >> */ >> #define OPTIONAL_HANDLER(_assert) \ >> -for (; _metadata->trigger; _metadata->trigger = __bail(_assert)) >> +for (; _metadata->trigger; _metadata->trigger = \ >> +__bail(_assert, _metadata->no_print, _metadata->step)) >> + >> +#define __INC_STEP(_metadata) \ >> +if (_metadata->passed && _metadata->step < 255) \ >> +_metadata->step++; >> >> #define __EXPECT(_expected, _seen, _t, _assert) do { \ >> /* Avoid multiple evaluation of the cases */ \ >> __typeof__(_expected) __exp = (_expected); \ >> __typeof__(_seen) __seen = (_seen); \ >> +if (_assert) __INC_STEP(_metadata); \ >> if (!(__exp _t __seen)) { \ >> unsigned long long __exp_print = (uintptr_t)__exp; \ >> unsigned long long __seen_print = (uintptr_t)__seen; \ >> @@ -576,6 +593,7 @@ >> #define __EXPECT_STR(_expected, _seen, _t, _assert) do { \ >> const char *__exp = (_expected); \ >> const char *__seen = (_seen); \ >> +if (_assert) __INC_STEP(_metadata); \ >> if (!(strcmp(__exp, __seen) _t 0)) { \ >> __TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \ >> _metadata->passed = 0; \ >> @@ -590,6 +608,8 @@ struct __test_metadata { >> int termsig; >> int passed; >> int trigger; /* extra handler after the evaluation */ >> +__u8 step; >> +bool no_print; /* manual trigger when TH_LOG_STREAM is not available */ >> struct __test_metadata *prev, *next; >> }; >> >> @@ -634,10 +654,13 @@ static inline void __register_test(struct >> __test_metadata *t) >> } >> } >> >> -static inline int __bail(int for_realz) >> +static inline int __bail(int for_realz, bool no_print, __u8 step) >> { >> -if (for_realz) >> +if (for_realz) { >> +if (no_print) >> +_exit(step); >> abort(); >> +} >> return 0; >> } >> >> @@ -655,18 +678,24 @@ void __run_test(struct __test_metadata *t) >> t->passed = 0; >> } else if (child_pid == 0) { >> t->fn(t); >> -_exit(t->passed); >> +/* return the step that failed or 0 */ >> +
Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed
On 08/06/2017 05:23 PM, Mickaël Salaün wrote: > When a test process is not able to write to TH_LOG_STREAM, this step > mechanism enable to print the assert number which triggered the failure. > This can be enabled by setting _metadata->no_print to true at the > beginning of the test sequence. > > Update the seccomp-bpf test to return 0 if a test succeeded. > > This feature is needed for the Landlock tests. > > Signed-off-by: Mickaël Salaün > Cc: Andy Lutomirski > Cc: Kees Cook > Cc: Shuah Khan > Cc: Will Drewry > Link: > https://lkml.kernel.org/r/cagxu5j+d-fp8kt9unnoqkrqjp4dytpmgkjxwykzyryivpz3...@mail.gmail.com I am looking through my Inbox and found this one. Okay to pull this in for 4.14-rc1? thanks, -- Shuah > --- > > Changes since the patch from the Landlock series: > * add the step counter in assert/expect macros and use _metadata to > enable the counter (suggested by Kees Cook) > * only count asserts > * add documentation > --- > tools/testing/selftests/kselftest_harness.h | 39 > +++ > tools/testing/selftests/seccomp/seccomp_bpf.c | 2 +- > 2 files changed, 35 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/kselftest_harness.h > b/tools/testing/selftests/kselftest_harness.h > index c56f72e07cd7..e81bd28bdd89 100644 > --- a/tools/testing/selftests/kselftest_harness.h > +++ b/tools/testing/selftests/kselftest_harness.h > @@ -51,6 +51,9 @@ > #define __KSELFTEST_HARNESS_H > > #define _GNU_SOURCE > +#include > +#include > +#include > #include > #include > #include > @@ -84,6 +87,14 @@ > * E.g., #define TH_LOG_ENABLED 1 > * > * If no definition is provided, logging is enabled by default. > + * > + * If there is no way to print an error message for the process running the > + * test (e.g. not allowed to write to stderr), it is still possible to get > the > + * ASSERT_* number for which the test failed. This behavior can be enabled > by > + * writing `_metadata->no_print = true;` before the check sequence that is > + * unable to print. When an error occur, instead of printing an error > message > + * and calling `abort(3)`, the test process call `_exit(2)` with the assert > + * number as argument, which is then printed by the parent process. > */ > #define TH_LOG(fmt, ...) do { \ > if (TH_LOG_ENABLED) \ > @@ -555,12 +566,18 @@ > * return while still providing an optional block to the API consumer. > */ > #define OPTIONAL_HANDLER(_assert) \ > - for (; _metadata->trigger; _metadata->trigger = __bail(_assert)) > + for (; _metadata->trigger; _metadata->trigger = \ > + __bail(_assert, _metadata->no_print, _metadata->step)) > + > +#define __INC_STEP(_metadata) \ > + if (_metadata->passed && _metadata->step < 255) \ > + _metadata->step++; > > #define __EXPECT(_expected, _seen, _t, _assert) do { \ > /* Avoid multiple evaluation of the cases */ \ > __typeof__(_expected) __exp = (_expected); \ > __typeof__(_seen) __seen = (_seen); \ > + if (_assert) __INC_STEP(_metadata); \ > if (!(__exp _t __seen)) { \ > unsigned long long __exp_print = (uintptr_t)__exp; \ > unsigned long long __seen_print = (uintptr_t)__seen; \ > @@ -576,6 +593,7 @@ > #define __EXPECT_STR(_expected, _seen, _t, _assert) do { \ > const char *__exp = (_expected); \ > const char *__seen = (_seen); \ > + if (_assert) __INC_STEP(_metadata); \ > if (!(strcmp(__exp, __seen) _t 0)) { \ > __TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \ > _metadata->passed = 0; \ > @@ -590,6 +608,8 @@ struct __test_metadata { > int termsig; > int passed; > int trigger; /* extra handler after the evaluation */ > + __u8 step; > + bool no_print; /* manual trigger when TH_LOG_STREAM is not available */ > struct __test_metadata *prev, *next; > }; > > @@ -634,10 +654,13 @@ static inline void __register_test(struct > __test_metadata *t) > } > } > > -static inline int __bail(int for_realz) > +static inline int __bail(int for_realz, bool no_print, __u8 step) > { > - if (for_realz) > + if (for_realz) { > + if (no_print) > + _exit(step); > abort(); > + } > return 0; > } > > @@ -655,18 +678,24 @@ void __run_test(struct __test_metadata *t) > t->passed = 0; > } else if (child_pid == 0) { > t->fn(t); > - _exit(t->passed); > + /* return the step that failed or 0 */ > + _exit(t->passed ? 0 : t->step); > } else { > /* TODO(wad) add timeout support. */ > waitpid(child_pid, &status, 0); > if (WIFEXITED(status)) { > - t->passed = t->termsig == -1 ? WEXITSTATUS(status) : 0; > + t->passed = t->termsig == -1 ? !WEXITSTATUS(status) : 0; > if (t
[PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed
When a test process is not able to write to TH_LOG_STREAM, this step mechanism enable to print the assert number which triggered the failure. This can be enabled by setting _metadata->no_print to true at the beginning of the test sequence. Update the seccomp-bpf test to return 0 if a test succeeded. This feature is needed for the Landlock tests. Signed-off-by: Mickaël Salaün Cc: Andy Lutomirski Cc: Kees Cook Cc: Shuah Khan Cc: Will Drewry Link: https://lkml.kernel.org/r/cagxu5j+d-fp8kt9unnoqkrqjp4dytpmgkjxwykzyryivpz3...@mail.gmail.com --- Changes since the patch from the Landlock series: * add the step counter in assert/expect macros and use _metadata to enable the counter (suggested by Kees Cook) * only count asserts * add documentation --- tools/testing/selftests/kselftest_harness.h | 39 +++ tools/testing/selftests/seccomp/seccomp_bpf.c | 2 +- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index c56f72e07cd7..e81bd28bdd89 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -51,6 +51,9 @@ #define __KSELFTEST_HARNESS_H #define _GNU_SOURCE +#include +#include +#include #include #include #include @@ -84,6 +87,14 @@ * E.g., #define TH_LOG_ENABLED 1 * * If no definition is provided, logging is enabled by default. + * + * If there is no way to print an error message for the process running the + * test (e.g. not allowed to write to stderr), it is still possible to get the + * ASSERT_* number for which the test failed. This behavior can be enabled by + * writing `_metadata->no_print = true;` before the check sequence that is + * unable to print. When an error occur, instead of printing an error message + * and calling `abort(3)`, the test process call `_exit(2)` with the assert + * number as argument, which is then printed by the parent process. */ #define TH_LOG(fmt, ...) do { \ if (TH_LOG_ENABLED) \ @@ -555,12 +566,18 @@ * return while still providing an optional block to the API consumer. */ #define OPTIONAL_HANDLER(_assert) \ - for (; _metadata->trigger; _metadata->trigger = __bail(_assert)) + for (; _metadata->trigger; _metadata->trigger = \ + __bail(_assert, _metadata->no_print, _metadata->step)) + +#define __INC_STEP(_metadata) \ + if (_metadata->passed && _metadata->step < 255) \ + _metadata->step++; #define __EXPECT(_expected, _seen, _t, _assert) do { \ /* Avoid multiple evaluation of the cases */ \ __typeof__(_expected) __exp = (_expected); \ __typeof__(_seen) __seen = (_seen); \ + if (_assert) __INC_STEP(_metadata); \ if (!(__exp _t __seen)) { \ unsigned long long __exp_print = (uintptr_t)__exp; \ unsigned long long __seen_print = (uintptr_t)__seen; \ @@ -576,6 +593,7 @@ #define __EXPECT_STR(_expected, _seen, _t, _assert) do { \ const char *__exp = (_expected); \ const char *__seen = (_seen); \ + if (_assert) __INC_STEP(_metadata); \ if (!(strcmp(__exp, __seen) _t 0)) { \ __TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \ _metadata->passed = 0; \ @@ -590,6 +608,8 @@ struct __test_metadata { int termsig; int passed; int trigger; /* extra handler after the evaluation */ + __u8 step; + bool no_print; /* manual trigger when TH_LOG_STREAM is not available */ struct __test_metadata *prev, *next; }; @@ -634,10 +654,13 @@ static inline void __register_test(struct __test_metadata *t) } } -static inline int __bail(int for_realz) +static inline int __bail(int for_realz, bool no_print, __u8 step) { - if (for_realz) + if (for_realz) { + if (no_print) + _exit(step); abort(); + } return 0; } @@ -655,18 +678,24 @@ void __run_test(struct __test_metadata *t) t->passed = 0; } else if (child_pid == 0) { t->fn(t); - _exit(t->passed); + /* return the step that failed or 0 */ + _exit(t->passed ? 0 : t->step); } else { /* TODO(wad) add timeout support. */ waitpid(child_pid, &status, 0); if (WIFEXITED(status)) { - t->passed = t->termsig == -1 ? WEXITSTATUS(status) : 0; + t->passed = t->termsig == -1 ? !WEXITSTATUS(status) : 0; if (t->termsig != -1) { fprintf(TH_LOG_STREAM, "%s: Test exited normally " "instead of by signal (code: %d)\n", t->name, WEXITSTATUS(sta