Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump

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

On 16/12/2025 19:46, Eduard Zingerman wrote:
> On Tue, 2025-12-16 at 11:00 -0800, Eduard Zingerman wrote:
>> On Tue, 2025-12-16 at 17:18 +0000, Alan Maguire wrote:
>>
>> [...]
>>
>>> @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
>>>  		case BTF_KIND_UNION:
>>>  			btf_dump_emit_mods(d, decls);
>>>  			/* inline anonymous struct/union */
>>> -			if (t->name_off == 0 && !d->skip_anon_defs)
>>> +			if (t->name_off == 0 && !d->skip_anon_defs) {
>>>  				btf_dump_emit_struct_def(d, id, t, lvl);
>>> -			else
>>> +			} else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) {
>>> +				/* anonymize nested struct and emit it */
>>> +				btf_dump_set_anon_type(d, id, true);
>>> +				btf_dump_emit_struct_def(d, id, t, lvl);
>>> +				btf_dump_set_anon_type(d, id, false);
>>
>>
>> Hi Alan,
>>
>> I think this is a solid idea.
>>
>> It seems to me that with current implementation there would be a
>> trouble in the following scenario:
>>
>>   struct foo { struct foo *ptr; };
>>   struct bar {
>>     struct foo;
>>   }
>>
>> Because state for 'foo' will be anonymize == true at the moment when
>> 'ptr' field is printed.
>>
>> Maybe pass a direct parameter to btf_dump_emit_struct_def()?
> 
> Digging a bit more into this, here are a couple of weird examples:
> 
>   $ cat ~/tmp/ms-ext-test.c
>   struct foo {
>     struct foo *ptr;
>   };
> 
>   struct bar {
>     struct foo;
>   };
> 
>   struct bar root;
>   $ gcc -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c
>   $ pahole -J ~/tmp/ms-ext-test.o
>   $ bpftool btf dump file ~/tmp/ms-ext-test.o format c
>   #ifndef __VMLINUX_H__
>   #define __VMLINUX_H__
> 
>   #ifndef BPF_NO_PRESERVE_ACCESS_INDEX
>   #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
>   #endif
> 
>   #ifndef __ksym
>   #define __ksym __attribute__((section(".ksyms")))
>   #endif
> 
>   #ifndef __weak
>   #define __weak __attribute__((weak))
>   #endif
> 
>   #ifndef __bpf_fastcall
>   #if __has_attribute(bpf_fastcall)
>   #define __bpf_fastcall __attribute__((bpf_fastcall))
>   #else
>   #define __bpf_fastcall
>   #endif
>   #endif
> 
>   struct foo {
>   	struct foo *ptr;
>   };
> 
> 
>   /* BPF kfuncs */
>   #ifndef BPF_NO_KFUNC_PROTOTYPES
>   #endif
> 
>   #ifndef BPF_NO_PRESERVE_ACCESS_INDEX
>   #pragma clang attribute pop
>   #endif
> 
>   #endif /* __VMLINUX_H__ */
> 
> 
>   $ cat ~/tmp/ms-ext-test.c
>   struct foo {
>     struct foo *ptr;
>   };
> 
>   struct bar {
>     struct foo;
>     int a;
>   };
> 
>   struct bar root;
>   $ cgcc -fms-extensions -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c
>   $ pahole -J ~/tmp/ms-ext-test.o
>   $ tools/bpf/bpftool/bpftool btf dump file ~/tmp/ms-ext-test.o format c
>   #ifndef __VMLINUX_H__
>   #define __VMLINUX_H__
> 
>   #ifndef BPF_NO_PRESERVE_ACCESS_INDEX
>   #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
>   #endif
> 
>   #ifndef __ksym
>   #define __ksym __attribute__((section(".ksyms")))
>   #endif
> 
>   #ifndef __weak
>   #define __weak __attribute__((weak))
>   #endif
> 
>   #ifndef __bpf_fastcall
>   #if __has_attribute(bpf_fastcall)
>   #define __bpf_fastcall __attribute__((bpf_fastcall))
>   #else
>   #define __bpf_fastcall
>   #endif
>   #endif
> 
>   struct foo {
>   	struct foo *ptr;
>   };
> 
>   struct bar {
>   	struct  {
>   		struct  *ptr;
>   	};
>   	int a;
>   };
> 
> 
>   /* BPF kfuncs */
>   #ifndef BPF_NO_KFUNC_PROTOTYPES
>   #endif
> 
>   #ifndef BPF_NO_PRESERVE_ACCESS_INDEX
>   #pragma clang attribute pop
>   #endif
> 
>   #endif /* __VMLINUX_H__ */
> 

Ack to all suggestions; in particular handling both cases with #ifdef is really nice since
it does away with the need for libbpf/bpftool options. With that in place - along with passing 
parameters rather than setting field values, and being more selective to ensure we only emit
the #ifdef/#else/#endif for a composite type nested in a composite type - the above
gives us the following:

struct foo {
	struct foo *ptr;
};

struct bar {
	
#ifdef __MS_EXTENSIONS__
	struct foo;
#else
	struct {
		struct foo *ptr;
	};
#endif

	int a;
};

I think that's the format we want. 

With respect to the padding behaviour, I may be missing something but I'm not sure these changes
will impact that. We pad in two cases:

1. between struct fields, where the current offset is less than the member offset or we have
alignment requirements to be met.
2. at the end of the struct, to pad it out the the size/alignment requirements.

I don't see anything different here for the case where we force-anonymize-inline the struct; 
it still does 1 and 2 identical with the out-of-line declaration. To test this I augmented
Eduard's example to add internal and whole struct alignment requirements: 

struct foo {
    struct foo *ptr;
    char __attribute__((__aligned__(16))) s;
    int t;
} __attribute__((__aligned__(64)));

struct bar {
    struct foo;
    int a;
};

struct bar root;

int main(int argc, char *argv[])
{
        struct bar *r = (struct bar *)argv[0];
}

$ gcc -fms-extensions -g -c -o /tmp/ms-ext-test.o /tmp/ms-ext-test.c 
$ pahole -J /tmp/ms-ext-test.o
$ bpftool btf dump file /tmp/ms-ext-test.o format c

struct foo {
	struct foo *ptr;
	long: 64;
	char s;
	int t;
	long: 64;
	long: 64;
	long: 64;
	long: 64;
	long: 64;
};

struct bar {
	
#ifdef __MS_EXTENSIONS__
	struct foo;
#else
	struct {
		struct foo *ptr;
		long: 64;
		char s;
		int t;
		long: 64;
		long: 64;
		long: 64;
		long: 64;
		long: 64;
	};
#endif

	int a;
	long: 64;
	long: 64;
	long: 64;
	long: 64;
	long: 64;
	long: 64;
	long: 64;
};

This looks equivalent to me, but there may some other condition you're thinking
of here?

Thanks!

Alan