[RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Alan Maguire]

When testing v1 of [1] we noticed that functions with 0-sized structs
as parameters were not part of BTF encoding; this was fixed in v2.
However we need to make sure we handle such zero-sized structs
correctly since they confound the calling convention expectations -
no registers are used for the empty struct so this has knock-on effects
for subsequent register-parameter matching.

Patch 1 updates BPF_PROG2() to handle the zero-sized struct case.
Patch 2 makes 0-sized structs a special case, allowing them to exist
as parameter representations in BTF without failing verification.
Patch 3 is a selftest that ensures the parameters after the 0-sized
struct are represented correctly.

[1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/

Alan Maguire (3):
  libbpf: update BPF_PROG2() to handle empty structs
  bpf: allow 0-sized structs as function parameters
  selftests/bpf: add 0-length struct testing to tracing_struct tests

 kernel/bpf/btf.c                                     |  2 +-
 tools/lib/bpf/bpf_tracing.h                          |  6 ++++--
 .../selftests/bpf/prog_tests/tracing_struct.c        |  2 ++
 tools/testing/selftests/bpf/progs/tracing_struct.c   | 11 +++++++++++
 tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 12 ++++++++++++
 5 files changed, 30 insertions(+), 3 deletions(-)

-- 
2.39.3

[RFC bpf-next 1/3] libbpf: update BPF_PROG2() to handle empty structs

[Alan Maguire]

In the kernel we occasionally find empty structs as parameters to
functions, usually because some arch-specific field(s) are not present.
Ensure that when such structs are used as parameters to functions we
handle the fact that no registers are used in their representation.

Deliberately not using a Fixes: tag here because for this to be useful
we need a more recent pahole with [1].

[1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/bpf_tracing.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index a8f6cd4841b0..7629650251dc 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -694,12 +694,13 @@ ____##name(unsigned long long *ctx, ##args)
 #endif
 
 #define ___bpf_treg_cnt(t) \
+	__builtin_choose_expr(sizeof(t) == 0, 0,	\
 	__builtin_choose_expr(sizeof(t) == 1, 1,	\
 	__builtin_choose_expr(sizeof(t) == 2, 1,	\
 	__builtin_choose_expr(sizeof(t) == 4, 1,	\
 	__builtin_choose_expr(sizeof(t) == 8, 1,	\
 	__builtin_choose_expr(sizeof(t) == 16, 2,	\
-			      (void)0)))))
+			      (void)0))))))
 
 #define ___bpf_reg_cnt0()		(0)
 #define ___bpf_reg_cnt1(t, x)		(___bpf_reg_cnt0() + ___bpf_treg_cnt(t))
@@ -717,12 +718,13 @@ ____##name(unsigned long long *ctx, ##args)
 #define ___bpf_reg_cnt(args...)	 ___bpf_apply(___bpf_reg_cnt, ___bpf_narg2(args))(args)
 
 #define ___bpf_union_arg(t, x, n) \
+	__builtin_choose_expr(sizeof(t) == 0, ({ t ___t; ___t; }), \
 	__builtin_choose_expr(sizeof(t) == 1, ({ union { __u8 z[1]; t x; } ___t = { .z = {ctx[n]}}; ___t.x; }), \
 	__builtin_choose_expr(sizeof(t) == 2, ({ union { __u16 z[1]; t x; } ___t = { .z = {ctx[n]} }; ___t.x; }), \
 	__builtin_choose_expr(sizeof(t) == 4, ({ union { __u32 z[1]; t x; } ___t = { .z = {ctx[n]} }; ___t.x; }), \
 	__builtin_choose_expr(sizeof(t) == 8, ({ union { __u64 z[1]; t x; } ___t = {.z = {ctx[n]} }; ___t.x; }), \
 	__builtin_choose_expr(sizeof(t) == 16, ({ union { __u64 z[2]; t x; } ___t = {.z = {ctx[n], ctx[n + 1]} }; ___t.x; }), \
-			      (void)0)))))
+			      (void)0))))))
 
 #define ___bpf_ctx_arg0(n, args...)
 #define ___bpf_ctx_arg1(n, t, x)		, ___bpf_union_arg(t, x, n - ___bpf_reg_cnt1(t, x))
-- 
2.39.3

Re: [RFC bpf-next 1/3] libbpf: update BPF_PROG2() to handle empty structs

[Alan Maguire]

On 08/05/2025 14:22, Alan Maguire wrote:
> In the kernel we occasionally find empty structs as parameters to
> functions, usually because some arch-specific field(s) are not present.
> Ensure that when such structs are used as parameters to functions we
> handle the fact that no registers are used in their representation.
> 
> Deliberately not using a Fixes: tag here because for this to be useful
> we need a more recent pahole with [1].
>

apologies, this isn't quite right; we had exceptions in pahole encoding
ensuring struct parameters didn't require strict parameter/register
matching, it's just that moving to a size-based determination in the v1
of Tony's patch left zero-sized structs out. So we should probably mention

Fixes: 34586d29f8df ("libbpf: Add new BPF_PROG2 macro")

> [1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/bpf_tracing.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index a8f6cd4841b0..7629650251dc 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -694,12 +694,13 @@ ____##name(unsigned long long *ctx, ##args)
>  #endif
>  
>  #define ___bpf_treg_cnt(t) \
> +	__builtin_choose_expr(sizeof(t) == 0, 0,	\
>  	__builtin_choose_expr(sizeof(t) == 1, 1,	\
>  	__builtin_choose_expr(sizeof(t) == 2, 1,	\
>  	__builtin_choose_expr(sizeof(t) == 4, 1,	\
>  	__builtin_choose_expr(sizeof(t) == 8, 1,	\
>  	__builtin_choose_expr(sizeof(t) == 16, 2,	\
> -			      (void)0)))))
> +			      (void)0))))))
>  
>  #define ___bpf_reg_cnt0()		(0)
>  #define ___bpf_reg_cnt1(t, x)		(___bpf_reg_cnt0() + ___bpf_treg_cnt(t))
> @@ -717,12 +718,13 @@ ____##name(unsigned long long *ctx, ##args)
>  #define ___bpf_reg_cnt(args...)	 ___bpf_apply(___bpf_reg_cnt, ___bpf_narg2(args))(args)
>  
>  #define ___bpf_union_arg(t, x, n) \
> +	__builtin_choose_expr(sizeof(t) == 0, ({ t ___t; ___t; }), \
>  	__builtin_choose_expr(sizeof(t) == 1, ({ union { __u8 z[1]; t x; } ___t = { .z = {ctx[n]}}; ___t.x; }), \
>  	__builtin_choose_expr(sizeof(t) == 2, ({ union { __u16 z[1]; t x; } ___t = { .z = {ctx[n]} }; ___t.x; }), \
>  	__builtin_choose_expr(sizeof(t) == 4, ({ union { __u32 z[1]; t x; } ___t = { .z = {ctx[n]} }; ___t.x; }), \
>  	__builtin_choose_expr(sizeof(t) == 8, ({ union { __u64 z[1]; t x; } ___t = {.z = {ctx[n]} }; ___t.x; }), \
>  	__builtin_choose_expr(sizeof(t) == 16, ({ union { __u64 z[2]; t x; } ___t = {.z = {ctx[n], ctx[n + 1]} }; ___t.x; }), \
> -			      (void)0)))))
> +			      (void)0))))))
>  
>  #define ___bpf_ctx_arg0(n, args...)
>  #define ___bpf_ctx_arg1(n, t, x)		, ___bpf_union_arg(t, x, n - ___bpf_reg_cnt1(t, x))

[RFC bpf-next 2/3] bpf: allow 0-sized structs as function parameters

[Alan Maguire]

Currently we forbid 0-sized parameters and complain that the function
has a malformed void argument, however now that we can handle 0-sized
structs using BPF_PROG2(), carve out an exception for them.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 kernel/bpf/btf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6b21ca67070c..dc18a646b98a 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7385,7 +7385,7 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 				tname, i, btf_type_str(t));
 			return -EINVAL;
 		}
-		if (ret == 0) {
+		if (ret == 0 && !__btf_type_is_struct(t)) {
 			bpf_log(log,
 				"The function %s has malformed void argument.\n",
 				tname);
-- 
2.39.3

[RFC bpf-next 3/3] selftests/bpf: add 0-length struct testing to tracing_struct tests

[Alan Maguire]

If a 0-length struct is passed as a parameter this throws off
assumptions that register N in calling convention will match parameter
N.  With [1], such function representations will correctly be emitted
in BTF because the register/param mismatch is not a result of an
optimization we cannot infer from the code alone.

Test that BPF_PROG2() macro can handle this situation by having
a function with a 0-length struct as first arg and verify that we
see expected 2nd, 3rd arg values.

[1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 .../selftests/bpf/prog_tests/tracing_struct.c        |  2 ++
 tools/testing/selftests/bpf/progs/tracing_struct.c   | 11 +++++++++++
 tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 12 ++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
index 19e68d4b3532..5b7e80fb8ccc 100644
--- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
+++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
@@ -56,6 +56,8 @@ static void test_struct_args(void)
 
 	ASSERT_EQ(skel->bss->t6, 1, "t6 ret");
 
+	ASSERT_EQ(skel->bss->t7, 34, "t7 a + c");
+
 destroy_skel:
 	tracing_struct__destroy(skel);
 }
diff --git a/tools/testing/selftests/bpf/progs/tracing_struct.c b/tools/testing/selftests/bpf/progs/tracing_struct.c
index c435a3a8328a..5d181be11a96 100644
--- a/tools/testing/selftests/bpf/progs/tracing_struct.c
+++ b/tools/testing/selftests/bpf/progs/tracing_struct.c
@@ -18,6 +18,9 @@ struct bpf_testmod_struct_arg_3 {
 	int b[];
 };
 
+struct bpf_testmod_struct_arg_6 {
+};
+
 long t1_a_a, t1_a_b, t1_b, t1_c, t1_ret, t1_nregs;
 __u64 t1_reg0, t1_reg1, t1_reg2, t1_reg3;
 long t2_a, t2_b_a, t2_b_b, t2_c, t2_ret;
@@ -25,6 +28,7 @@ long t3_a, t3_b, t3_c_a, t3_c_b, t3_ret;
 long t4_a_a, t4_b, t4_c, t4_d, t4_e_a, t4_e_b, t4_ret;
 long t5_ret;
 int t6;
+int t7;
 
 SEC("fentry/bpf_testmod_test_struct_arg_1")
 int BPF_PROG2(test_struct_arg_1, struct bpf_testmod_struct_arg_2, a, int, b, int, c)
@@ -130,4 +134,11 @@ int BPF_PROG2(test_struct_arg_11, struct bpf_testmod_struct_arg_3 *, a)
 	return 0;
 }
 
+SEC("fentry/bpf_testmod_test_struct_arg_10")
+int BPF_PROG2(test_struct_arg_12, struct bpf_testmod_struct_arg_6, h, u64, a, short, c)
+{
+	t7 = a + c;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 2e54b95ad898..4a766e5ee9bc 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -62,6 +62,9 @@ struct bpf_testmod_struct_arg_5 {
 	long d;
 };
 
+struct bpf_testmod_struct_arg_6 {
+};
+
 __bpf_hook_start();
 
 noinline int
@@ -128,6 +131,13 @@ bpf_testmod_test_struct_arg_9(u64 a, void *b, short c, int d, void *e, char f,
 	return bpf_testmod_test_struct_arg_result;
 }
 
+noinline int
+bpf_testmod_test_struct_arg_10(struct bpf_testmod_struct_arg_6 h, u64 a, short c)
+{
+	bpf_testmod_test_struct_arg_result = a + c;
+	return bpf_testmod_test_struct_arg_result;
+}
+
 noinline int
 bpf_testmod_test_arg_ptr_to_struct(struct bpf_testmod_struct_arg_1 *a) {
 	bpf_testmod_test_struct_arg_result = a->a;
@@ -398,6 +408,7 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 	struct bpf_testmod_struct_arg_3 *struct_arg3;
 	struct bpf_testmod_struct_arg_4 struct_arg4 = {21, 22};
 	struct bpf_testmod_struct_arg_5 struct_arg5 = {23, 24, 25, 26};
+	struct bpf_testmod_struct_arg_6 struct_arg6 = {};
 	int i = 1;
 
 	while (bpf_testmod_return_ptr(i))
@@ -414,6 +425,7 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 					    (void *)20, struct_arg4, 23);
 	(void)bpf_testmod_test_struct_arg_9(16, (void *)17, 18, 19, (void *)20,
 					    21, 22, struct_arg5, 27);
+	(void)bpf_testmod_test_struct_arg_10(struct_arg6, 16, 18);
 
 	(void)bpf_testmod_test_arg_ptr_to_struct(&struct_arg1_2);
 
-- 
2.39.3

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Andrii Nakryiko]

On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> When testing v1 of [1] we noticed that functions with 0-sized structs
> as parameters were not part of BTF encoding; this was fixed in v2.
> However we need to make sure we handle such zero-sized structs
> correctly since they confound the calling convention expectations -
> no registers are used for the empty struct so this has knock-on effects
> for subsequent register-parameter matching.

Do you have a list (or at least an example) of the function we are
talking about, just curious to see what's that.

The question I have is whether it's safe to assume that regardless of
architecture we can assume that zero-sized struct has no effect on
register allocation (which would seem logical, but is that true for
all ABIs).

BTW, while looking at patch #2, I noticed that
btf_distill_func_proto() disallows functions returning
struct-by-value, which seems overly aggressive, at least for structs
of up to 8 bytes. So maybe if we can validate that both cases are not
introducing any new quirks across all supported architectures, we can
solve both limitations?

P.S., oh, and s390x selftest (test_struct_args) isn't happy, please check.


>
> Patch 1 updates BPF_PROG2() to handle the zero-sized struct case.
> Patch 2 makes 0-sized structs a special case, allowing them to exist
> as parameter representations in BTF without failing verification.
> Patch 3 is a selftest that ensures the parameters after the 0-sized
> struct are represented correctly.
>
> [1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/
>
> Alan Maguire (3):
>   libbpf: update BPF_PROG2() to handle empty structs
>   bpf: allow 0-sized structs as function parameters
>   selftests/bpf: add 0-length struct testing to tracing_struct tests
>
>  kernel/bpf/btf.c                                     |  2 +-
>  tools/lib/bpf/bpf_tracing.h                          |  6 ++++--
>  .../selftests/bpf/prog_tests/tracing_struct.c        |  2 ++
>  tools/testing/selftests/bpf/progs/tracing_struct.c   | 11 +++++++++++
>  tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 12 ++++++++++++
>  5 files changed, 30 insertions(+), 3 deletions(-)
>
> --
> 2.39.3
>

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Tony Ambardar]

On Fri, May 09, 2025 at 11:40:47AM -0700, Andrii Nakryiko wrote:
> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > When testing v1 of [1] we noticed that functions with 0-sized structs
> > as parameters were not part of BTF encoding; this was fixed in v2.
> > However we need to make sure we handle such zero-sized structs
> > correctly since they confound the calling convention expectations -
> > no registers are used for the empty struct so this has knock-on effects
> > for subsequent register-parameter matching.
> 
> Do you have a list (or at least an example) of the function we are
> talking about, just curious to see what's that.
> 

BTW, Alan shared an example in the other pahole patch thread:
https://lore.kernel.org/dwarves/07d92da1-36f3-44d2-a0a4-cf7dabf278c6@oracle.com/

> The question I have is whether it's safe to assume that regardless of
> architecture we can assume that zero-sized struct has no effect on
> register allocation (which would seem logical, but is that true for
> all ABIs).
> 
> BTW, while looking at patch #2, I noticed that
> btf_distill_func_proto() disallows functions returning
> struct-by-value, which seems overly aggressive, at least for structs
> of up to 8 bytes. So maybe if we can validate that both cases are not
> introducing any new quirks across all supported architectures, we can
> solve both limitations?
> 

Given pahole (and my related patch) assume pass-by-value for well-sized
structs, I'd like to see this too. But while the pahole patch works on
64/32-bit archs, I noticed from patch #1 that e.g. ___bpf_treg_cnt()
seems to hard-code a 64-bit register size. Perhaps we can fix that too? 

> P.S., oh, and s390x selftest (test_struct_args) isn't happy, please check.
> 
> 
> >
> > Patch 1 updates BPF_PROG2() to handle the zero-sized struct case.
> > Patch 2 makes 0-sized structs a special case, allowing them to exist
> > as parameter representations in BTF without failing verification.
> > Patch 3 is a selftest that ensures the parameters after the 0-sized
> > struct are represented correctly.
> >
> > [1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/
> >
> > Alan Maguire (3):
> >   libbpf: update BPF_PROG2() to handle empty structs
> >   bpf: allow 0-sized structs as function parameters
> >   selftests/bpf: add 0-length struct testing to tracing_struct tests
> >
> >  kernel/bpf/btf.c                                     |  2 +-
> >  tools/lib/bpf/bpf_tracing.h                          |  6 ++++--
> >  .../selftests/bpf/prog_tests/tracing_struct.c        |  2 ++
> >  tools/testing/selftests/bpf/progs/tracing_struct.c   | 11 +++++++++++
> >  tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 12 ++++++++++++
> >  5 files changed, 30 insertions(+), 3 deletions(-)
> >
> > --
> > 2.39.3
> >

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Alan Maguire]

On 12/05/2025 10:17, Tony Ambardar wrote:
> On Fri, May 09, 2025 at 11:40:47AM -0700, Andrii Nakryiko wrote:
>> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>
>>> When testing v1 of [1] we noticed that functions with 0-sized structs
>>> as parameters were not part of BTF encoding; this was fixed in v2.
>>> However we need to make sure we handle such zero-sized structs
>>> correctly since they confound the calling convention expectations -
>>> no registers are used for the empty struct so this has knock-on effects
>>> for subsequent register-parameter matching.
>>
>> Do you have a list (or at least an example) of the function we are
>> talking about, just curious to see what's that.
>>
> 
> BTW, Alan shared an example in the other pahole patch thread:
> https://lore.kernel.org/dwarves/07d92da1-36f3-44d2-a0a4-cf7dabf278c6@oracle.com/
>

Yep, the one I came across on x86_64 was

static int __io_run_local_work(struct io_ring_ctx *ctx, io_tw_token_t
tw, int min_events, int max_events);

(the io_tw_token_t parameter is a typedef for

struct io_tw_state {
};


>> The question I have is whether it's safe to assume that regardless of
>> architecture we can assume that zero-sized struct has no effect on
>> register allocation (which would seem logical, but is that true for
>> all ABIs).
>>
>> BTW, while looking at patch #2, I noticed that
>> btf_distill_func_proto() disallows functions returning
>> struct-by-value, which seems overly aggressive, at least for structs
>> of up to 8 bytes. So maybe if we can validate that both cases are not
>> introducing any new quirks across all supported architectures, we can
>> solve both limitations?
>>
>

Good idea. I'll try and address this and add a return value test.

> Given pahole (and my related patch) assume pass-by-value for well-sized
> structs, I'd like to see this too. But while the pahole patch works on
> 64/32-bit archs, I noticed from patch #1 that e.g. ___bpf_treg_cnt()
> seems to hard-code a 64-bit register size. Perhaps we can fix that too? 
>

So I think your concern is the assumptions


        __builtin_choose_expr(sizeof(t) == 8, 1,        \
        __builtin_choose_expr(sizeof(t) == 16, 2,        \

? We may need arch-specific macros that specify register size that we
can use here, or is there a better way?

>> P.S., oh, and s390x selftest (test_struct_args) isn't happy, please check.

Yep, working to repro this locally now, thanks.

Alan

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Andrii Nakryiko]

On Wed, May 14, 2025 at 3:31 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 12/05/2025 10:17, Tony Ambardar wrote:
> > On Fri, May 09, 2025 at 11:40:47AM -0700, Andrii Nakryiko wrote:
> >> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>>
> >>> When testing v1 of [1] we noticed that functions with 0-sized structs
> >>> as parameters were not part of BTF encoding; this was fixed in v2.
> >>> However we need to make sure we handle such zero-sized structs
> >>> correctly since they confound the calling convention expectations -
> >>> no registers are used for the empty struct so this has knock-on effects
> >>> for subsequent register-parameter matching.
> >>
> >> Do you have a list (or at least an example) of the function we are
> >> talking about, just curious to see what's that.
> >>
> >
> > BTW, Alan shared an example in the other pahole patch thread:
> > https://lore.kernel.org/dwarves/07d92da1-36f3-44d2-a0a4-cf7dabf278c6@oracle.com/
> >
>
> Yep, the one I came across on x86_64 was
>
> static int __io_run_local_work(struct io_ring_ctx *ctx, io_tw_token_t
> tw, int min_events, int max_events);
>
> (the io_tw_token_t parameter is a typedef for
>
> struct io_tw_state {
> };
>
>
> >> The question I have is whether it's safe to assume that regardless of
> >> architecture we can assume that zero-sized struct has no effect on
> >> register allocation (which would seem logical, but is that true for
> >> all ABIs).
> >>
> >> BTW, while looking at patch #2, I noticed that
> >> btf_distill_func_proto() disallows functions returning
> >> struct-by-value, which seems overly aggressive, at least for structs
> >> of up to 8 bytes. So maybe if we can validate that both cases are not
> >> introducing any new quirks across all supported architectures, we can
> >> solve both limitations?
> >>
> >
>
> Good idea. I'll try and address this and add a return value test.
>
> > Given pahole (and my related patch) assume pass-by-value for well-sized
> > structs, I'd like to see this too. But while the pahole patch works on
> > 64/32-bit archs, I noticed from patch #1 that e.g. ___bpf_treg_cnt()
> > seems to hard-code a 64-bit register size. Perhaps we can fix that too?
> >
>
> So I think your concern is the assumptions
>
>
>         __builtin_choose_expr(sizeof(t) == 8, 1,        \
>         __builtin_choose_expr(sizeof(t) == 16, 2,        \
>
> ? We may need arch-specific macros that specify register size that we
> can use here, or is there a better way?

we know the target architecture, so this shouldn't be hard to define
the word size accordingly and use that here?

>
> >> P.S., oh, and s390x selftest (test_struct_args) isn't happy, please check.
>
> Yep, working to repro this locally now, thanks.
>
> Alan

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Tony Ambardar]

On Wed, May 14, 2025 at 09:22:00AM -0700, Andrii Nakryiko wrote:
> On Wed, May 14, 2025 at 3:31 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > On 12/05/2025 10:17, Tony Ambardar wrote:
> > > On Fri, May 09, 2025 at 11:40:47AM -0700, Andrii Nakryiko wrote:
> > >> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:

[...]

Hi Alan, Andrii,

> > > Given pahole (and my related patch) assume pass-by-value for well-sized
> > > structs, I'd like to see this too. But while the pahole patch works on
> > > 64/32-bit archs, I noticed from patch #1 that e.g. ___bpf_treg_cnt()
> > > seems to hard-code a 64-bit register size. Perhaps we can fix that too?
> > >
> >
> > So I think your concern is the assumptions
> >
> >
> >         __builtin_choose_expr(sizeof(t) == 8, 1,        \
> >         __builtin_choose_expr(sizeof(t) == 16, 2,        \
> >
> > ? We may need arch-specific macros that specify register size that we
> > can use here, or is there a better way?
> 
> we know the target architecture, so this shouldn't be hard to define
> the word size accordingly and use that here?
> 

Well, I'm now unsure if this is a problem after reading the code more
closely.  The ctx is a u64 array and the PROG2-related macros process ctx
elems within the BPF VM using 64-bit regs. Does this mean the macro/union
magic all works OK then?

Unfortunately, I cannot test a 32-bit host easily since JIT trampoline
support is still a WIP in my arm32 test setup. But perhaps someone can
share tracing experience with ppc32 JIT, which supports trampoline?

Thanks,
Tony
 
[...]

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Andrii Nakryiko]

On Thu, May 15, 2025 at 1:02 AM Tony Ambardar <tony.ambardar@gmail.com> wrote:
>
> On Wed, May 14, 2025 at 09:22:00AM -0700, Andrii Nakryiko wrote:
> > On Wed, May 14, 2025 at 3:31 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > >
> > > On 12/05/2025 10:17, Tony Ambardar wrote:
> > > > On Fri, May 09, 2025 at 11:40:47AM -0700, Andrii Nakryiko wrote:
> > > >> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> [...]
>
> Hi Alan, Andrii,
>
> > > > Given pahole (and my related patch) assume pass-by-value for well-sized
> > > > structs, I'd like to see this too. But while the pahole patch works on
> > > > 64/32-bit archs, I noticed from patch #1 that e.g. ___bpf_treg_cnt()
> > > > seems to hard-code a 64-bit register size. Perhaps we can fix that too?
> > > >
> > >
> > > So I think your concern is the assumptions
> > >
> > >
> > >         __builtin_choose_expr(sizeof(t) == 8, 1,        \
> > >         __builtin_choose_expr(sizeof(t) == 16, 2,        \
> > >
> > > ? We may need arch-specific macros that specify register size that we
> > > can use here, or is there a better way?
> >
> > we know the target architecture, so this shouldn't be hard to define
> > the word size accordingly and use that here?
> >
>
> Well, I'm now unsure if this is a problem after reading the code more
> closely.  The ctx is a u64 array and the PROG2-related macros process ctx
> elems within the BPF VM using 64-bit regs. Does this mean the macro/union
> magic all works OK then?

I'd think that it's still a problem, because on 32-bit architecture if
argument is 8 byte long, we'll use 2 registers, and each register will
get their ctx[i] and ctx[i + 1] slot, so we need to take into account
32-bit vs 64-bit distinction when calculating how many ctx[] slots we
need to use/skip.

>
> Unfortunately, I cannot test a 32-bit host easily since JIT trampoline
> support is still a WIP in my arm32 test setup. But perhaps someone can
> share tracing experience with ppc32 JIT, which supports trampoline?
>
> Thanks,
> Tony
>
> [...]

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Alan Maguire]

On 09/05/2025 19:40, Andrii Nakryiko wrote:
> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> When testing v1 of [1] we noticed that functions with 0-sized structs
>> as parameters were not part of BTF encoding; this was fixed in v2.
>> However we need to make sure we handle such zero-sized structs
>> correctly since they confound the calling convention expectations -
>> no registers are used for the empty struct so this has knock-on effects
>> for subsequent register-parameter matching.
> 
> Do you have a list (or at least an example) of the function we are
> talking about, just curious to see what's that.
> 
> The question I have is whether it's safe to assume that regardless of
> architecture we can assume that zero-sized struct has no effect on
> register allocation (which would seem logical, but is that true for
> all ABIs).
>

I've been investigating this a bit, specifically in the context of s390
where we saw the test failure. The actual kernel function where I first
observed the zero-sized struct in practice is

static int __io_run_local_work(struct io_ring_ctx *ctx, io_tw_token_t
tw, int min_events, int max_events);

In s390 DWARF, we see the following representation for it:

 <1><6f7f788>: Abbrev Number: 104 (DW_TAG_subprogram)
    <6f7f789>   DW_AT_name        : (indirect string, offset: 0x2c47f5):
__io_run_local_work
    <6f7f78d>   DW_AT_decl_file   : 1
    <6f7f78e>   DW_AT_decl_line   : 1301
    <6f7f790>   DW_AT_decl_column : 12
    <6f7f791>   DW_AT_prototyped  : 1
    <6f7f791>   DW_AT_type        : <0x6f413a2>
    <6f7f795>   DW_AT_low_pc      : 0x99c850
    <6f7f79d>   DW_AT_high_pc     : 0x2b2
    <6f7f7a5>   DW_AT_frame_base  : 1 byte block: 9c
(DW_OP_call_frame_cfa)
    <6f7f7a7>   DW_AT_GNU_all_call_sites: 1
    <6f7f7a7>   DW_AT_sibling     : <0x6f802e6>
 <2><6f7f7ab>: Abbrev Number: 53 (DW_TAG_formal_parameter)
    <6f7f7ac>   DW_AT_name        : ctx
    <6f7f7b0>   DW_AT_decl_file   : 1
    <6f7f7b1>   DW_AT_decl_line   : 1301
    <6f7f7b3>   DW_AT_decl_column : 52
    <6f7f7b4>   DW_AT_type        : <0x6f6882b>
    <6f7f7b8>   DW_AT_location    : 0x2babcbe (location list)
    <6f7f7bc>   DW_AT_GNU_locviews: 0x2babcac
 <2><6f7f7c0>: Abbrev Number: 135 (DW_TAG_formal_parameter)
    <6f7f7c2>   DW_AT_name        : tw
    <6f7f7c5>   DW_AT_decl_file   : 1
    <6f7f7c6>   DW_AT_decl_line   : 1301
    <6f7f7c8>   DW_AT_decl_column : 71
    <6f7f7c9>   DW_AT_type        : <0x6f6833e>
    <6f7f7cd>   DW_AT_location    : 2 byte block: 73 0  (DW_OP_breg3
(r3): 0)


..i.e. we are using the expected calling-convention register (r3) here
for the zero-sized struct parameter.

Contrast this with x86_64 and aarch64, where regardless of -O level we
appear to use an offset from the frame ptr to reference the zero-sized
struct. As a result the next parameter after the zero-sized struct uses
the next available calling-convention register (%rdi if the zero-sized
struct is the first arg, %rsi if it was the second etc) that was unused
by the zero-sized struct parameter.

I don't see anything in the ABI specs which covers this scenario
exactly; I suspect the 0-sized object handling in cases other than s390
is just using the usual > register size aggregate object handling
(passing a large struct as a parameter), and in s390 it's not.

So long story short, we may need to take an arch-specific approach here
unfortunately. Great that CI flagged this as an issue too!

Alan




> BTW, while looking at patch #2, I noticed that
> btf_distill_func_proto() disallows functions returning
> struct-by-value, which seems overly aggressive, at least for structs
> of up to 8 bytes. So maybe if we can validate that both cases are not
> introducing any new quirks across all supported architectures, we can
> solve both limitations?
> 
> P.S., oh, and s390x selftest (test_struct_args) isn't happy, please check.
> 
> 
>>
>> Patch 1 updates BPF_PROG2() to handle the zero-sized struct case.
>> Patch 2 makes 0-sized structs a special case, allowing them to exist
>> as parameter representations in BTF without failing verification.
>> Patch 3 is a selftest that ensures the parameters after the 0-sized
>> struct are represented correctly.
>>
>> [1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/
>>
>> Alan Maguire (3):
>>   libbpf: update BPF_PROG2() to handle empty structs
>>   bpf: allow 0-sized structs as function parameters
>>   selftests/bpf: add 0-length struct testing to tracing_struct tests
>>
>>  kernel/bpf/btf.c                                     |  2 +-
>>  tools/lib/bpf/bpf_tracing.h                          |  6 ++++--
>>  .../selftests/bpf/prog_tests/tracing_struct.c        |  2 ++
>>  tools/testing/selftests/bpf/progs/tracing_struct.c   | 11 +++++++++++
>>  tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 12 ++++++++++++
>>  5 files changed, 30 insertions(+), 3 deletions(-)
>>
>> --
>> 2.39.3
>>

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Alan Maguire]

On 15/05/2025 11:56, Alan Maguire wrote:
> On 09/05/2025 19:40, Andrii Nakryiko wrote:
>> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>
>>> When testing v1 of [1] we noticed that functions with 0-sized structs
>>> as parameters were not part of BTF encoding; this was fixed in v2.
>>> However we need to make sure we handle such zero-sized structs
>>> correctly since they confound the calling convention expectations -
>>> no registers are used for the empty struct so this has knock-on effects
>>> for subsequent register-parameter matching.
>>
>> Do you have a list (or at least an example) of the function we are
>> talking about, just curious to see what's that.
>>
>> The question I have is whether it's safe to assume that regardless of
>> architecture we can assume that zero-sized struct has no effect on
>> register allocation (which would seem logical, but is that true for
>> all ABIs).
>>
> 
> I've been investigating this a bit, specifically in the context of s390
> where we saw the test failure. The actual kernel function where I first
> observed the zero-sized struct in practice is
> 
> static int __io_run_local_work(struct io_ring_ctx *ctx, io_tw_token_t
> tw, int min_events, int max_events);
> 
> In s390 DWARF, we see the following representation for it:
> 
>  <1><6f7f788>: Abbrev Number: 104 (DW_TAG_subprogram)
>     <6f7f789>   DW_AT_name        : (indirect string, offset: 0x2c47f5):
> __io_run_local_work
>     <6f7f78d>   DW_AT_decl_file   : 1
>     <6f7f78e>   DW_AT_decl_line   : 1301
>     <6f7f790>   DW_AT_decl_column : 12
>     <6f7f791>   DW_AT_prototyped  : 1
>     <6f7f791>   DW_AT_type        : <0x6f413a2>
>     <6f7f795>   DW_AT_low_pc      : 0x99c850
>     <6f7f79d>   DW_AT_high_pc     : 0x2b2
>     <6f7f7a5>   DW_AT_frame_base  : 1 byte block: 9c
> (DW_OP_call_frame_cfa)
>     <6f7f7a7>   DW_AT_GNU_all_call_sites: 1
>     <6f7f7a7>   DW_AT_sibling     : <0x6f802e6>
>  <2><6f7f7ab>: Abbrev Number: 53 (DW_TAG_formal_parameter)
>     <6f7f7ac>   DW_AT_name        : ctx
>     <6f7f7b0>   DW_AT_decl_file   : 1
>     <6f7f7b1>   DW_AT_decl_line   : 1301
>     <6f7f7b3>   DW_AT_decl_column : 52
>     <6f7f7b4>   DW_AT_type        : <0x6f6882b>
>     <6f7f7b8>   DW_AT_location    : 0x2babcbe (location list)
>     <6f7f7bc>   DW_AT_GNU_locviews: 0x2babcac
>  <2><6f7f7c0>: Abbrev Number: 135 (DW_TAG_formal_parameter)
>     <6f7f7c2>   DW_AT_name        : tw
>     <6f7f7c5>   DW_AT_decl_file   : 1
>     <6f7f7c6>   DW_AT_decl_line   : 1301
>     <6f7f7c8>   DW_AT_decl_column : 71
>     <6f7f7c9>   DW_AT_type        : <0x6f6833e>
>     <6f7f7cd>   DW_AT_location    : 2 byte block: 73 0  (DW_OP_breg3
> (r3): 0)
> 
> 
> ..i.e. we are using the expected calling-convention register (r3) here
> for the zero-sized struct parameter.
> 
> Contrast this with x86_64 and aarch64, where regardless of -O level we
> appear to use an offset from the frame ptr to reference the zero-sized
> struct. As a result the next parameter after the zero-sized struct uses
> the next available calling-convention register (%rdi if the zero-sized
> struct is the first arg, %rsi if it was the second etc) that was unused
> by the zero-sized struct parameter.
> 
> I don't see anything in the ABI specs which covers this scenario
> exactly; I suspect the 0-sized object handling in cases other than s390
> is just using the usual > register size aggregate object handling
> (passing a large struct as a parameter), and in s390 it's not.
> 
> So long story short, we may need to take an arch-specific approach here
> unfortunately. Great that CI flagged this as an issue too!
> 
> Alan
> 
> 

I discussed this with Jose, and the gcc behaviour with zero-sized
structs varies a bit between architectures. Given that complexity, my
inclination would be to class functions with 0-sized struct parameters
as having inconsistent representations. They can then be tackled by
adding location info on a per-site basis later as part of the
inline-related work. For now we would just not emit BTF for them, since
without that site-specific analysis we can't be sure from function
signature alone where parameters are stored. In practice this means
leaving one function out of kernel BTF.

So long story short, I think it might make sense to withdraw this series
for now and see if we can tweak Tony's patch to class functions with
0-sized parameters as inconsistent as the v1 version did, meaning they
don't get a BTF representation. Thanks!

Alan

> 
> 
>> BTW, while looking at patch #2, I noticed that
>> btf_distill_func_proto() disallows functions returning
>> struct-by-value, which seems overly aggressive, at least for structs
>> of up to 8 bytes. So maybe if we can validate that both cases are not
>> introducing any new quirks across all supported architectures, we can
>> solve both limitations?
>>
>> P.S., oh, and s390x selftest (test_struct_args) isn't happy, please check.
>>
>>
>>>
>>> Patch 1 updates BPF_PROG2() to handle the zero-sized struct case.
>>> Patch 2 makes 0-sized structs a special case, allowing them to exist
>>> as parameter representations in BTF without failing verification.
>>> Patch 3 is a selftest that ensures the parameters after the 0-sized
>>> struct are represented correctly.
>>>
>>> [1] https://lore.kernel.org/dwarves/20250502070318.1561924-1-tony.ambardar@gmail.com/
>>>
>>> Alan Maguire (3):
>>>   libbpf: update BPF_PROG2() to handle empty structs
>>>   bpf: allow 0-sized structs as function parameters
>>>   selftests/bpf: add 0-length struct testing to tracing_struct tests
>>>
>>>  kernel/bpf/btf.c                                     |  2 +-
>>>  tools/lib/bpf/bpf_tracing.h                          |  6 ++++--
>>>  .../selftests/bpf/prog_tests/tracing_struct.c        |  2 ++
>>>  tools/testing/selftests/bpf/progs/tracing_struct.c   | 11 +++++++++++
>>>  tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 12 ++++++++++++
>>>  5 files changed, 30 insertions(+), 3 deletions(-)
>>>
>>> --
>>> 2.39.3
>>>
> 
> 

Re: [RFC bpf-next 0/3] bpf: handle 0-sized structs properly

[Tony Ambardar]

On Tue, May 20, 2025 at 09:59:27AM +0100, Alan Maguire wrote:
[...]
> 
> I discussed this with Jose, and the gcc behaviour with zero-sized
> structs varies a bit between architectures. Given that complexity, my
> inclination would be to class functions with 0-sized struct parameters
> as having inconsistent representations. They can then be tackled by
> adding location info on a per-site basis later as part of the
> inline-related work. For now we would just not emit BTF for them, since
> without that site-specific analysis we can't be sure from function
> signature alone where parameters are stored. In practice this means
> leaving one function out of kernel BTF.
> 
> So long story short, I think it might make sense to withdraw this series
> for now and see if we can tweak Tony's patch to class functions with
> 0-sized parameters as inconsistent as the v1 version did, meaning they
> don't get a BTF representation. Thanks!
> 
> Alan
> 
[...]

Agreed that sounds reasonable, and I'd like to resolve the original
problem on 32-bit, so will update my patch and resend.

Thanks,
Tony