Re: [PATCH bpf-next v1 2/4] selftests/bpf: Add tests for bpftool btf dump format c

On 2026-08-20 12:09 p.m., Eduard Zingerman wrote:
> On Wed, 2026-08-19 at 17:06 -0700, Ihor Solodrai wrote:
>> "bpftool btf dump format c" generates the vmlinux.h that BPF programs
>> are built against, and it has no test coverage at all. The only
>> in-tree consumers are build systems, and none of them diff the result.
>>
>> Building against the header only catches what a compiler rejects.
>> Missing macros, a missing preserve_access_index pragma and the wrong
>> padding width all compile.
>>
>> Add a test for those three, over both sort orderings.
>>
>> Signed-off-by: Ihor Solodrai <[email protected]>
>> ---
>>   .../bpf/prog_tests/bpftool_btf_dump.c         | 151 ++++++++++++++++++
>>   1 file changed, 151 insertions(+)
>>   create mode 100644 tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c b/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
>> new file mode 100644
>> index 000000000000..53bb7065b3ca
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
>> @@ -0,0 +1,151 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <test_progs.h>
>> +#include <bpftool_helpers.h>
>> +#include <bpf/btf.h>
>> +#include <unistd.h>
>> +
>> +#define DUMP_BUF_SZ	(16 * 1024)
>> +
>> +static int btf_to_tmpfile(struct btf *btf, char *path, size_t path_sz)
> 
> Nit: let's drop path_sz and assume PATH_MAX.
> 
>> [...]
>> +static struct btf *mk_btf(void)
>> +{
>> +	struct btf *btf;
>> +	int id, err;
>> +
>> +	btf = btf__new_empty();
>> +	if (!ASSERT_OK_PTR(btf, "new_empty"))
>> +		return NULL;
>> +
>> +	id = btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
>> +	if (!ASSERT_EQ(id, 1, "int"))
>> +		goto err_out;
>> +
>> +	id = btf__add_int(btf, "long int", 4, BTF_INT_SIGNED);
>> +	if (!ASSERT_GT(id, 0, "long"))
>> +		goto err_out;
>> +
>> +	id = btf__add_struct(btf, "holey", 16);
>> +	if (!ASSERT_GT(id, 0, "struct_holey"))
>> +		goto err_out;
>> +
>> +	err = btf__add_field(btf, "c", 1, 0, 0);
>> +	if (!ASSERT_OK(err, "holey_c"))
>> +		goto err_out;
>> +
>> +	err = btf__add_field(btf, "tail", 1, 96, 0);
>> +	if (!ASSERT_OK(err, "holey_tail"))
>> +		goto err_out;
>> +
>> +	btf__set_pointer_size(btf, 4);
> 
> Nit: I think the approach taken by btf_distill.c:test_distilled_base()
>       is a bit better, it drops verbose error checks after each btf__*
>       constructor call and calls VALIDATE_RAW_BTF() instead.
>       This is both concise and self-documenting.

Wasn't aware of that one. Will try it, thanks.

> 
>> +
>> +	return btf;
>> +err_out:
>> +	btf__free(btf);
>> +	return NULL;
>> +}
>> +
>> +/*
>> + * Check only what the selftests build cannot:
>> + *   - bpf_helpers.h defines __ksym and __weak as well, and no program uses
>> + *     __bpf_fastcall, so losing the macro block changes nothing;
> 
> The above bullet point does not make sense.
> 
>> + *   - building without the preserve_access_index pragma is a supported mode
>> + *     (BPF_NO_PRESERVE_ACCESS_INDEX), so losing it only costs CO-RE;
>> + *   - the padding width comes from the BTF's pointer size, and a native build
>> + *     never runs the host bpftool over a differently sized target's BTF.
> 
> Please make these two inline with actual asserts,
> also "... so losing it only costs CO-RE" is completely out of context here.

This block made sense to me yesterday.
I was either too "in context" or too tired. Will fix.

> 
>> [...]