On 09/05/2025 19:40, Andrii Nakryiko wrote:
> On Thu, May 8, 2025 at 6:22 AM Alan Maguire <alan.maguire@xxxxxxxxxx> 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@xxxxxxxxx/
>>
>> 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
>>
