| Message ID | 20210924025856.2192476-1-yhs@fb.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel | expand |
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 > >
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(-) > > [...]
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 }
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(-)