On Fri, Sep 30, 2022 at 9:50 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> An update for `bpftool btf dump file ... format c`.
> Add a missing newline print for structures that consist of
> anonymous-only padding fields. E.g. here is struct bpf_timer from
> vmlinux.h before this patch:
>
> struct bpf_timer {
> long: 64;
> long: 64;};
>
> And after this patch:
>
> struct bpf_dynptr {
> long: 64;
> long: 64;
> };
Without looking at source code it wasn't clear what the original issue
was. It would be good to explain that libbpf's btf_dumper attempts to
emit empty structs with {} on the same line. But this breaks for
non-zero-sized structs due to padding.
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> tools/lib/bpf/btf_dump.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index 4221f73a74d0..ebbba19ac122 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -852,7 +852,7 @@ static int chip_away_bits(int total, int at_most)
> return total % at_most ? : at_most;
> }
>
> -static void btf_dump_emit_bit_padding(const struct btf_dump *d,
> +static bool btf_dump_emit_bit_padding(const struct btf_dump *d,
> int cur_off, int m_off, int m_bit_sz,
> int align, int lvl)
> {
> @@ -861,10 +861,10 @@ static void btf_dump_emit_bit_padding(const struct btf_dump *d,
>
> if (off_diff <= 0)
> /* no gap */
> - return;
> + return false;
> if (m_bit_sz == 0 && off_diff < align * 8)
> /* natural padding will take care of a gap */
> - return;
> + return false;
>
> while (off_diff > 0) {
> const char *pad_type;
> @@ -886,6 +886,8 @@ static void btf_dump_emit_bit_padding(const struct btf_dump *d,
> btf_dump_printf(d, "\n%s%s: %d;", pfx(lvl), pad_type, pad_bits);
> off_diff -= pad_bits;
> }
> +
> + return true;
> }
>
> static void btf_dump_emit_struct_fwd(struct btf_dump *d, __u32 id,
> @@ -906,6 +908,7 @@ static void btf_dump_emit_struct_def(struct btf_dump *d,
> bool is_struct = btf_is_struct(t);
> int align, i, packed, off = 0;
> __u16 vlen = btf_vlen(t);
> + bool padding_added = false;
>
> packed = is_struct ? btf_is_struct_packed(d->btf, id, t) : 0;
>
> @@ -940,11 +943,11 @@ static void btf_dump_emit_struct_def(struct btf_dump *d,
> /* pad at the end, if necessary */
> if (is_struct) {
> align = packed ? 1 : btf__align_of(d->btf, id);
> - btf_dump_emit_bit_padding(d, off, t->size * 8, 0, align,
> - lvl + 1);
> + padding_added = btf_dump_emit_bit_padding(d, off, t->size * 8, 0, align,
> + lvl + 1);
> }
>
> - if (vlen)
> + if (vlen || padding_added)
What if instead of returning the padding_added flag we just check that
struct is non-zero-sized? Clearly vlen isn't an appropriate check
here.
> btf_dump_printf(d, "\n");
> btf_dump_printf(d, "%s}", pfx(lvl));
> if (packed)
> --
> 2.37.3
>
