* [PATCH bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel
@ 2021-09-24 2:58 Yonghong Song
2021-09-24 18:12 ` Alan Maguire
0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2021-09-24 2:58 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
With clang build kernel (adding LLVM=1 to kernel and selftests/bpf build
command line), I hit the following test failure:
$ ./test_progs -t btf_dump
...
btf_dump_data:PASS:ensure expected/actual match 0 nsec
btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
test_btf_dump_int_data:FAIL:dump __int128 unexpected error: -2 (errno 2)
#15/9 btf_dump/btf_dump: int_data:FAIL
Further analysis showed gcc build kernel has type "__int128" in dwarf/BTF
and it doesn't exist in clang build kernel. Code searching for kernel code
found the following:
arch/s390/include/asm/types.h: unsigned __int128 pair;
crypto/ecc.c: unsigned __int128 m = (unsigned __int128)left * right;
include/linux/math64.h: return (u64)(((unsigned __int128)a * mul) >> shift);
include/linux/math64.h: return (u64)(((unsigned __int128)a * mul) >> shift);
lib/ubsan.h:typedef __int128 s_max;
lib/ubsan.h:typedef unsigned __int128 u_max;
In my case, CONFIG_UBSAN is not enabled. Even if we only have "unsigned __int128"
in the code, somehow gcc still put "__int128" in dwarf while clang didn't.
Hence current test works fine for gcc but not for clang.
Enabling CONFIG_UBSAN is an option to provide __int128 type into dwarf
reliably for both gcc and clang, but not everybody enables CONFIG_UBSAN
in their kernel build. So the best choice is to use "unsigned __int128" type
which is available in both clang and gcc build kernels. But clang and gcc
dwarf encoded names for "unsigned __int128" are different:
[$ ~] cat t.c
unsigned __int128 a;
[$ ~] gcc -g -c t.c && llvm-dwarfdump t.o | grep __int128
DW_AT_type (0x00000031 "__int128 unsigned")
DW_AT_name ("__int128 unsigned")
[$ ~] clang -g -c t.c && llvm-dwarfdump t.o | grep __int128
DW_AT_type (0x00000033 "unsigned __int128")
DW_AT_name ("unsigned __int128")
The test change in this patch tries to test type name before
doing actual test.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
.../selftests/bpf/prog_tests/btf_dump.c | 27 ++++++++++++++-----
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index 52ccf0cf35e1..87f9df653e4e 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -358,12 +358,27 @@ static void test_btf_dump_int_data(struct btf *btf, struct btf_dump *d,
TEST_BTF_DUMP_DATA_OVER(btf, d, NULL, str, int, sizeof(int)-1, "", 1);
#ifdef __SIZEOF_INT128__
- TEST_BTF_DUMP_DATA(btf, d, NULL, str, __int128, BTF_F_COMPACT,
- "(__int128)0xffffffffffffffff",
- 0xffffffffffffffff);
- ASSERT_OK(btf_dump_data(btf, d, "__int128", NULL, 0, &i, 16, str,
- "(__int128)0xfffffffffffffffffffffffffffffffe"),
- "dump __int128");
+ /* gcc encode unsigned __int128 type with name "__int128 unsigned" in dwarf,
+ * and clang encode it with name "unsigned __int128" in dwarf.
+ * Do an availability test for either variant before doing actual test.
+ */
+ if (btf__find_by_name(btf, "unsigned __int128") > 0) {
+ TEST_BTF_DUMP_DATA(btf, d, NULL, str, unsigned __int128, BTF_F_COMPACT,
+ "(unsigned __int128)0xffffffffffffffff",
+ 0xffffffffffffffff);
+ ASSERT_OK(btf_dump_data(btf, d, "unsigned __int128", NULL, 0, &i, 16, str,
+ "(unsigned __int128)0xfffffffffffffffffffffffffffffffe"),
+ "dump unsigned __int128");
+ } else if (btf__find_by_name(btf, "__int128 unsigned") > 0) {
+ TEST_BTF_DUMP_DATA(btf, d, NULL, str, __int128 unsigned, BTF_F_COMPACT,
+ "(__int128 unsigned)0xffffffffffffffff",
+ 0xffffffffffffffff);
+ ASSERT_OK(btf_dump_data(btf, d, "__int128 unsigned", NULL, 0, &i, 16, str,
+ "(__int128 unsigned)0xfffffffffffffffffffffffffffffffe"),
+ "dump unsigned __int128");
+ } else {
+ ASSERT_TRUE(false, "unsigned_int128_not_found");
+ }
#endif
}
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel
2021-09-24 2:58 [PATCH bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel Yonghong Song
@ 2021-09-24 18:12 ` Alan Maguire
2021-09-24 23:35 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Alan Maguire @ 2021-09-24 18:12 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
On Fri, 24 Sep 2021, Yonghong Song wrote:
> With clang build kernel (adding LLVM=1 to kernel and selftests/bpf build
> command line), I hit the following test failure:
>
> $ ./test_progs -t btf_dump
> ...
> btf_dump_data:PASS:ensure expected/actual match 0 nsec
> btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
> btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
> test_btf_dump_int_data:FAIL:dump __int128 unexpected error: -2 (errno 2)
> #15/9 btf_dump/btf_dump: int_data:FAIL
>
> Further analysis showed gcc build kernel has type "__int128" in dwarf/BTF
> and it doesn't exist in clang build kernel. Code searching for kernel code
> found the following:
> arch/s390/include/asm/types.h: unsigned __int128 pair;
> crypto/ecc.c: unsigned __int128 m = (unsigned __int128)left * right;
> include/linux/math64.h: return (u64)(((unsigned __int128)a * mul) >> shift);
> include/linux/math64.h: return (u64)(((unsigned __int128)a * mul) >> shift);
> lib/ubsan.h:typedef __int128 s_max;
> lib/ubsan.h:typedef unsigned __int128 u_max;
>
> In my case, CONFIG_UBSAN is not enabled. Even if we only have "unsigned __int128"
> in the code, somehow gcc still put "__int128" in dwarf while clang didn't.
> Hence current test works fine for gcc but not for clang.
>
> Enabling CONFIG_UBSAN is an option to provide __int128 type into dwarf
> reliably for both gcc and clang, but not everybody enables CONFIG_UBSAN
> in their kernel build. So the best choice is to use "unsigned __int128" type
> which is available in both clang and gcc build kernels. But clang and gcc
> dwarf encoded names for "unsigned __int128" are different:
>
> [$ ~] cat t.c
> unsigned __int128 a;
> [$ ~] gcc -g -c t.c && llvm-dwarfdump t.o | grep __int128
> DW_AT_type (0x00000031 "__int128 unsigned")
> DW_AT_name ("__int128 unsigned")
> [$ ~] clang -g -c t.c && llvm-dwarfdump t.o | grep __int128
> DW_AT_type (0x00000033 "unsigned __int128")
> DW_AT_name ("unsigned __int128")
>
> The test change in this patch tries to test type name before
> doing actual test.
>
Thanks for finding and fixing this!
> Signed-off-by: Yonghong Song <yhs@fb.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> .../selftests/bpf/prog_tests/btf_dump.c | 27 ++++++++++++++-----
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> index 52ccf0cf35e1..87f9df653e4e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> @@ -358,12 +358,27 @@ static void test_btf_dump_int_data(struct btf *btf, struct btf_dump *d,
> TEST_BTF_DUMP_DATA_OVER(btf, d, NULL, str, int, sizeof(int)-1, "", 1);
>
> #ifdef __SIZEOF_INT128__
> - TEST_BTF_DUMP_DATA(btf, d, NULL, str, __int128, BTF_F_COMPACT,
> - "(__int128)0xffffffffffffffff",
> - 0xffffffffffffffff);
> - ASSERT_OK(btf_dump_data(btf, d, "__int128", NULL, 0, &i, 16, str,
> - "(__int128)0xfffffffffffffffffffffffffffffffe"),
> - "dump __int128");
> + /* gcc encode unsigned __int128 type with name "__int128 unsigned" in dwarf,
> + * and clang encode it with name "unsigned __int128" in dwarf.
> + * Do an availability test for either variant before doing actual test.
> + */
> + if (btf__find_by_name(btf, "unsigned __int128") > 0) {
> + TEST_BTF_DUMP_DATA(btf, d, NULL, str, unsigned __int128, BTF_F_COMPACT,
> + "(unsigned __int128)0xffffffffffffffff",
> + 0xffffffffffffffff);
> + ASSERT_OK(btf_dump_data(btf, d, "unsigned __int128", NULL, 0, &i, 16, str,
> + "(unsigned __int128)0xfffffffffffffffffffffffffffffffe"),
> + "dump unsigned __int128");
> + } else if (btf__find_by_name(btf, "__int128 unsigned") > 0) {
> + TEST_BTF_DUMP_DATA(btf, d, NULL, str, __int128 unsigned, BTF_F_COMPACT,
> + "(__int128 unsigned)0xffffffffffffffff",
> + 0xffffffffffffffff);
> + ASSERT_OK(btf_dump_data(btf, d, "__int128 unsigned", NULL, 0, &i, 16, str,
> + "(__int128 unsigned)0xfffffffffffffffffffffffffffffffe"),
> + "dump unsigned __int128");
> + } else {
> + ASSERT_TRUE(false, "unsigned_int128_not_found");
> + }
> #endif
> }
>
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel
2021-09-24 18:12 ` Alan Maguire
@ 2021-09-24 23:35 ` Andrii Nakryiko
0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2021-09-24 23:35 UTC (permalink / raw)
To: Alan Maguire
Cc: Yonghong Song, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team
On Fri, Sep 24, 2021 at 11:13 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Fri, 24 Sep 2021, Yonghong Song wrote:
>
> > With clang build kernel (adding LLVM=1 to kernel and selftests/bpf build
> > command line), I hit the following test failure:
> >
> > $ ./test_progs -t btf_dump
> > ...
> > btf_dump_data:PASS:ensure expected/actual match 0 nsec
> > btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
> > btf_dump_data:FAIL:find type id unexpected find type id: actual -2 < expected 0
> > test_btf_dump_int_data:FAIL:dump __int128 unexpected error: -2 (errno 2)
> > #15/9 btf_dump/btf_dump: int_data:FAIL
> >
> > Further analysis showed gcc build kernel has type "__int128" in dwarf/BTF
> > and it doesn't exist in clang build kernel. Code searching for kernel code
> > found the following:
> > arch/s390/include/asm/types.h: unsigned __int128 pair;
> > crypto/ecc.c: unsigned __int128 m = (unsigned __int128)left * right;
> > include/linux/math64.h: return (u64)(((unsigned __int128)a * mul) >> shift);
> > include/linux/math64.h: return (u64)(((unsigned __int128)a * mul) >> shift);
> > lib/ubsan.h:typedef __int128 s_max;
> > lib/ubsan.h:typedef unsigned __int128 u_max;
> >
> > In my case, CONFIG_UBSAN is not enabled. Even if we only have "unsigned __int128"
> > in the code, somehow gcc still put "__int128" in dwarf while clang didn't.
> > Hence current test works fine for gcc but not for clang.
> >
> > Enabling CONFIG_UBSAN is an option to provide __int128 type into dwarf
> > reliably for both gcc and clang, but not everybody enables CONFIG_UBSAN
> > in their kernel build. So the best choice is to use "unsigned __int128" type
> > which is available in both clang and gcc build kernels. But clang and gcc
> > dwarf encoded names for "unsigned __int128" are different:
> >
> > [$ ~] cat t.c
> > unsigned __int128 a;
> > [$ ~] gcc -g -c t.c && llvm-dwarfdump t.o | grep __int128
> > DW_AT_type (0x00000031 "__int128 unsigned")
> > DW_AT_name ("__int128 unsigned")
> > [$ ~] clang -g -c t.c && llvm-dwarfdump t.o | grep __int128
> > DW_AT_type (0x00000033 "unsigned __int128")
> > DW_AT_name ("unsigned __int128")
> >
> > The test change in this patch tries to test type name before
> > doing actual test.
> >
>
> Thanks for finding and fixing this!
>
> > Signed-off-by: Yonghong Song <yhs@fb.com>
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>
Applied to bpf-next, thanks.
> > ---
> > .../selftests/bpf/prog_tests/btf_dump.c | 27 ++++++++++++++-----
> > 1 file changed, 21 insertions(+), 6 deletions(-)
> >
[...]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-24 23:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 2:58 [PATCH bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel Yonghong Song
2021-09-24 18:12 ` Alan Maguire
2021-09-24 23:35 ` Andrii Nakryiko
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).