| Message ID | 20210420111639.155580-1-lmb@cloudflare.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | BPF |
| Headers | show |
| Series | Some CO-RE negative testcases are buggy | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Apr 20, 2021 at 4:23 AM Lorenz Bauer <lmb@cloudflare.com> wrote: > > Hi Andrii, > > I was looking at some CORE testcases, and noticed two problems: > > * The checks for negative test cases use an incorrect CHECK(false) > invocation. This means negative test cases don't fail when they > should. CHECK() misuse? shocker ;) This is why I'm harping on ASSERT_xxx() vs CHECK() all the time lately. > * Some existence tests use incorrect file names, but the test harness > is unable to detect this. Basically, failure to load due to a failed > CORE relocation is not distinguished from ENOENT. I found the CHECK > issue when investigating this problem. > > I've written the patch attached below, but there are now 12 failures. > I don't understand the tests well enough to fix them, maybe you can > take a look? Yep, thanks for reporting, I'll take a look. > > Best > Lorenz > > --- > .../selftests/bpf/prog_tests/core_reloc.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c > index d94dcead72e6..bd759290347c 100644 > --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c > +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c > @@ -644,12 +644,12 @@ static struct core_reloc_test_case test_cases[] = { > .output_len = sizeof(struct core_reloc_existence_output), > }, > > - FIELD_EXISTS_ERR_CASE(existence__err_int_sz), > - FIELD_EXISTS_ERR_CASE(existence__err_int_type), > - FIELD_EXISTS_ERR_CASE(existence__err_int_kind), > - FIELD_EXISTS_ERR_CASE(existence__err_arr_kind), > - FIELD_EXISTS_ERR_CASE(existence__err_arr_value_type), > - FIELD_EXISTS_ERR_CASE(existence__err_struct_type), > + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_sz), > + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_type), > + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_kind), > + FIELD_EXISTS_ERR_CASE(existence___err_wrong_arr_kind), > + FIELD_EXISTS_ERR_CASE(existence___err_wrong_arr_value_type), > + FIELD_EXISTS_ERR_CASE(existence___err_wrong_struct_type), > > /* bitfield relocation checks */ > BITFIELDS_CASE(bitfields, { > @@ -864,7 +864,7 @@ void test_core_reloc(void) > err = bpf_object__load_xattr(&load_attr); > if (err) { > if (!test_case->fails) > - CHECK(false, "obj_load", "failed to load prog '%s': %d\n", probe_name, err); > + CHECK(true, "obj_load", "failed to load prog '%s': %d\n", probe_name, err); > goto cleanup; > } > > @@ -904,7 +904,7 @@ void test_core_reloc(void) > } > > if (test_case->fails) { > - CHECK(false, "obj_load_fail", "should fail to load prog '%s'\n", probe_name); > + CHECK(true, "obj_load_fail", "should fail to load prog '%s'\n", probe_name); > goto cleanup; > } > > -- > 2.27.0 >
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c index d94dcead72e6..bd759290347c 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c @@ -644,12 +644,12 @@ static struct core_reloc_test_case test_cases[] = { .output_len = sizeof(struct core_reloc_existence_output), }, - FIELD_EXISTS_ERR_CASE(existence__err_int_sz), - FIELD_EXISTS_ERR_CASE(existence__err_int_type), - FIELD_EXISTS_ERR_CASE(existence__err_int_kind), - FIELD_EXISTS_ERR_CASE(existence__err_arr_kind), - FIELD_EXISTS_ERR_CASE(existence__err_arr_value_type), - FIELD_EXISTS_ERR_CASE(existence__err_struct_type), + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_sz), + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_type), + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_kind), + FIELD_EXISTS_ERR_CASE(existence___err_wrong_arr_kind), + FIELD_EXISTS_ERR_CASE(existence___err_wrong_arr_value_type), + FIELD_EXISTS_ERR_CASE(existence___err_wrong_struct_type), /* bitfield relocation checks */ BITFIELDS_CASE(bitfields, { @@ -864,7 +864,7 @@ void test_core_reloc(void) err = bpf_object__load_xattr(&load_attr); if (err) { if (!test_case->fails) - CHECK(false, "obj_load", "failed to load prog '%s': %d\n", probe_name, err); + CHECK(true, "obj_load", "failed to load prog '%s': %d\n", probe_name, err); goto cleanup; } @@ -904,7 +904,7 @@ void test_core_reloc(void) } if (test_case->fails) { - CHECK(false, "obj_load_fail", "should fail to load prog '%s'\n", probe_name); + CHECK(true, "obj_load_fail", "should fail to load prog '%s'\n", probe_name); goto cleanup; }