Re: [PATCH bpf-next] selftests/bpf: fix btf_dump __int128 test failure with clang build kernel

[Alan Maguire <alan.maguire@oracle.com>]

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
> 
>