Re: [PATCH bpf-next v1 4/4] selftests/bpf: Check that sorting preserves types in bpftool dump

On 2026-08-20 1:42 p.m., Eduard Zingerman wrote:
> On Wed, 2026-08-19 at 17:06 -0700, Ihor Solodrai wrote:
> 
> ...
> 
>> @@ -87,6 +90,36 @@ static struct btf *mk_btf(void)
>>   	if (!ASSERT_OK(err, "holey_tail"))
>>   		goto err_out;
>>   
>> +	for (i = 0; i < 3; i++) {
> 
> Testing this locally, a single enum suffices to showcase the bug,
> why the complication?

Hi Eduard, thank you for reviewing so quickly.

For the bug that was fixed, yes. But in general, I thought it's good to 
have a couple of different things in the test sample.

> 
>> +		char name[16];
>> +
>> +		snprintf(name, sizeof(name), "E%d", i);
>> +
>> +		id = btf__add_enum(btf, NULL, 4);
>> +		if (!ASSERT_GT(id, 0, "anon_enum"))
>> +			goto err_out;
>> +
>> +		err = btf__add_enum_value(btf, name, i + 1);
>> +		if (!ASSERT_OK(err, "enum_val"))
>> +			goto err_out;
>> +	}
>> +
>> +	id = btf__add_struct(btf, "s", 4);
>> +	if (!ASSERT_GT(id, 0, "struct_s"))
>> +		goto err_out;
>> +
>> +	err = btf__add_field(btf, "f", 1, 0, 0);
>> +	if (!ASSERT_OK(err, "s_field"))
>> +		goto err_out;
>> +
>> +	id = btf__add_union(btf, "u", 4);
>> +	if (!ASSERT_GT(id, 0, "union_u"))
>> +		goto err_out;
>> +
>> +	err = btf__add_field(btf, "f", 1, 0, 0);
>> +	if (!ASSERT_OK(err, "u_field"))
>> +		goto err_out;
>> +
>>   	btf__set_pointer_size(btf, 4);
>>   
>>   	return btf;
>> @@ -95,6 +128,42 @@ static struct btf *mk_btf(void)
> 
> ...
> 
>> +static void test_sort_preserves_types(const char *path)
>> +{
>> +	char *sorted = NULL, *unsorted = NULL;
>> +	int n_sorted, n_unsorted;
>> +
>> +	sorted = dump_c(path, true);
>> +	unsorted = dump_c(path, false);
>> +	if (!sorted || !unsorted)
>> +		goto out;
>> +
>> +	/*
>> +	 * Count definitions by the closing brace in their first column. Counting
>> +	 * "struct"/"union"/"enum" openers instead would also count forward
>> +	 * declarations, and which types need one depends on emission order.
>> +	 */
>> +	n_unsorted = count_substr(unsorted, "\n}");
>> +	n_sorted = count_substr(sorted, "\n}");
> 
> This does not include e.g. typedef. I'd just match for presence of enum.
> Or better yet, compare full expected output with actual one and tailor
> several BTF definitions, each for a specific sub-test.

I wanted to avoid committing a sample generated header. I think the
exact string check will also be too sensitive to the changes in libbpf
dumper or bpftool.

Presence of enum only tests the concrete bug, and I wanted some simple
proxy to a more generic sorted vs unsorted output check.
That is, "did we drop anything while sorting?" check.

Ideally we'd need to parse the headers and compare the full set of
emitted types.. An overkill here, I think.

We can just add counting typedefs as well.
That's an easy improvement. wdyt?


> 
>> +	ASSERT_GT(n_unsorted, 0, "types_emitted");
>> +	ASSERT_EQ(n_sorted, n_unsorted, "same_type_count");
>> +
>> +out:
>> +	free(sorted);
>> +	free(unsorted);
>> +}
>> +
>>   /*
>>    * Check only what the selftests build cannot:
>>    *   - bpf_helpers.h defines __ksym and __weak as well, and no program uses
> 
> ...