On Sat, Jun 19, 2021 at 1:56 AM Alan Maguire alan.maguire@oracle.com wrote:

Add a BTF dumper for typed data, so that the user can dump a typed version of the data provided.

The API is

int btf_dump__dump_type_data(struct btf_dump *d, __u32 id, void *data, size_t data_sz, const struct btf_dump_type_data_opts *opts);

...where the id is the BTF id of the data pointed to by the "void *" argument; for example the BTF id of "struct sk_buff" for a "struct skb *" data pointer. Options supported are

• a starting indent level (indent_lvl)
• a user-specified indent string which will be printed once per indent level; if NULL, tab is chosen but any string <= 32 chars can be provided.
• a set of boolean options to control dump display, similar to those used for BPF helper bpf_snprintf_btf(). Options are - compact : omit newlines and other indentation - skip_names: omit member names - emit_zeroes: show zero-value members

Default output format is identical to that dumped by bpf_snprintf_btf(), for example a "struct sk_buff" representation would look like this:

struct sk_buff){ (union){ (struct){ .next = (struct sk_buff *)0xffffffffffffffff, .prev = (struct sk_buff *)0xffffffffffffffff, (union){ .dev = (struct net_device *)0xffffffffffffffff, .dev_scratch = (long unsigned int)18446744073709551615, }, }, ...

If the data structure is larger than the *data_sz* number of bytes that are available in *data*, as much of the data as possible will be dumped and -E2BIG will be returned. This is useful as tracers will sometimes not be able to capture all of the data associated with a type; for example a "struct task_struct" is ~16k. Being able to specify that only a subset is available is important for such cases. On success, the amount of data dumped is returned.

Signed-off-by: Alan Maguire alan.maguire@oracle.com

Sorry for the long delay, I was out on vacation. It starts to look really well and pretty straightforward (after a few reading passes :) ).

I think I still spotted a bunch of problems (and made a bunch of nits, of course :), so we probably need another iteration, but I like where it's headed, thanks for sticking with this. It will be a useful and powerful API.

Also, seems like there are some compilation warnings reported, please fix those as well.

tools/lib/bpf/btf.h | 19 ++ tools/lib/bpf/btf_dump.c | 833 ++++++++++++++++++++++++++++++++++++++++++++++- tools/lib/bpf/libbpf.map | 1 + 3 files changed, 848 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index b54f1c3..5240973 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -184,6 +184,25 @@ struct btf_dump_emit_type_decl_opts { btf_dump__emit_type_decl(struct btf_dump *d, __u32 id, const struct btf_dump_emit_type_decl_opts *opts);

+struct btf_dump_type_data_opts {

•   /* size of this struct, for forward/backward compatibility */
  

•   size_t sz;
  

•   int indent_level;
  

•   const char *indent_str;
  

let's swap indent_str and indent_level to avoid unnecessary padding

•   /* below match "show" flags for bpf_show_snprintf() */
  

•   bool compact;           /* no newlines/tabs */
  

s/tabs/indentation/ ?

•   bool skip_names;        /* skip member/type names */
  

•   bool emit_zeroes;       /* show 0-valued fields */
  

•   size_t :0;
  

+}; +#define btf_dump_type_data_opts__last_field emit_zeroes

+LIBBPF_API int +btf_dump__dump_type_data(struct btf_dump *d, __u32 id,

•                    const void *data, size_t data_sz,
  

•                    const struct btf_dump_type_data_opts *opts);
  

/*

• A set of helpers for easier BTF types handling

*/ diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 5dc6b517..c59fa07 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -10,6 +10,8 @@ #include <stddef.h> #include <stdlib.h> #include <string.h> +#include <ctype.h> +#include <endian.h> #include <errno.h> #include <linux/err.h> #include <linux/btf.h> @@ -53,6 +55,28 @@ struct btf_dump_type_aux_state { __u8 referenced: 1; };

+/* indent string length; one indent string is added for each indent level */ +#define BTF_DATA_INDENT_STR_LEN 32

+/*

• • Common internal data for BTF type data dump operations.
• */

+struct btf_dump_data {

•   const void *data_end;           /* end of valid data to show */
  

•   bool compact;
  

•   bool skip_names;
  

•   bool emit_zeroes;
  

•   __u8 indent_lvl;        /* base indent level */
  

•   char indent_str[BTF_DATA_INDENT_STR_LEN + 1];
  

nit: this is 33-byte array, which is weirdly unsatisfying :) let's make it 32

•   /* below are used during iteration */
  

•   struct {
  

•           __u8 depth;
  

•           __u8 array_member:1,
  

•                array_terminated:1,
  

•                array_ischar:1;
  

•   } state;
  

nit: use of bifields here is questionable, it doesn't actually save any space, and even if it used few extra bytes it's totally fine given btf_dump_data is allocated on demand. Also, given the state struct is pretty simple, I don't see a problem of just inlining all its members into btf_dump_data as is (except is_array_member is imo more readable than state.array_member). So basically, don't try to save a few bytes here. For depth, just use int instead of __u8. I'd also make array_xxx as a bool fields (which they totally are) and call them is_array_xxx.

+};

struct btf_dump { const struct btf *btf; const struct btf_ext *btf_ext; @@ -60,6 +84,7 @@ struct btf_dump { struct btf_dump_opts opts; int ptr_sz; bool strip_mods;

•   bool skip_anon_defs;
    int last_id;
  
    /* per-type auxiliary state */
  

@@ -89,6 +114,10 @@ struct btf_dump { * name occurrences */ struct hashmap *ident_names;

•   /*
  

•    * data for typed display; allocated if needed.
  

•    */
  

•   struct btf_dump_data *typed_dump;
  

};

static size_t str_hash_fn(const void *key, void *ctx) @@ -765,11 +794,11 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id) break; case BTF_KIND_FUNC_PROTO: { const struct btf_param *p = btf_params(t);

•           __u16 vlen = btf_vlen(t);
  

•           __u16 n = btf_vlen(t);
            int i;
  
            btf_dump_emit_type(d, t->type, cont_id);
  

•           for (i = 0; i < vlen; i++, p++)
  

•           for (i = 0; i < n; i++, p++)
                    btf_dump_emit_type(d, p->type, cont_id);
  
            break;
  

@@ -852,8 +881,9 @@ static void btf_dump_emit_bit_padding(const struct btf_dump *d, static void btf_dump_emit_struct_fwd(struct btf_dump *d, __u32 id, const struct btf_type *t) {

•   btf_dump_printf(d, "%s %s",
  

•   btf_dump_printf(d, "%s%s%s",
                    btf_is_struct(t) ? "struct" : "union",
  

•                   t->name_off ? " " : "",
                    btf_dump_type_name(d, id));
  

}

@@ -1259,7 +1289,7 @@ 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)
  

•                   if (t->name_off == 0 && !d->skip_anon_defs)
                            btf_dump_emit_struct_def(d, id, t, lvl);
                    else
                            btf_dump_emit_struct_fwd(d, id, t);
  

@@ -1267,7 +1297,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, case BTF_KIND_ENUM: btf_dump_emit_mods(d, decls); /* inline anonymous enum */

•                   if (t->name_off == 0)
  

•                   if (t->name_off == 0 && !d->skip_anon_defs)
                            btf_dump_emit_enum_def(d, id, t, lvl);
                    else
                            btf_dump_emit_enum_fwd(d, id, t);
  

@@ -1392,6 +1422,39 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, btf_dump_emit_name(d, fname, last_was_ptr); }

+/* show type name as (type_name) */ +static void btf_dump_emit_type_cast(struct btf_dump *d, __u32 id,

•                               bool top_level)
  

+{

•   const struct btf_type *t;
  

•   /* for array members, we don't bother emitting type name for each
  

•    * member to avoid the redundancy of
  

•    * .name = (char[4])[(char)'f',(char)'o',(char)'o',]
  

•    */
  

•   if (d->typed_dump->state.array_member)
  

•           return;
  

•   /* avoid type name specification for variable/section; it will be done
  

•    * for the associated variable value(s).
  

•    */
  

•   t = btf__type_by_id(d->btf, id);
  

•   if (btf_is_var(t) || btf_is_datasec(t))
  

•           return;
  

•   if (top_level)
  

•           btf_dump_printf(d, "(");
  

•   d->skip_anon_defs = true;
  

•   d->strip_mods = true;
  

•   btf_dump_emit_type_decl(d, id, "", 0);
  

•   d->strip_mods = false;
  

•   d->skip_anon_defs = false;
  

•   if (top_level)
  

•           btf_dump_printf(d, ")");
  

+}

/* return number of duplicates (occurrences) of a given name */ static size_t btf_dump_name_dups(struct btf_dump *d, struct hashmap *name_map, const char *orig_name) @@ -1442,3 +1505,763 @@ static const char *btf_dump_ident_name(struct btf_dump *d, __u32 id) { return btf_dump_resolve_name(d, id, d->ident_names); }

+static int btf_dump_dump_type_data(struct btf_dump *d,

•                              const char *fname,
  

•                              const struct btf_type *t,
  

•                              __u32 id,
  

•                              const void *data,
  

•                              __u8 bits_offset,
  

•                              __u8 bit_sz);
  

+static const char *btf_dump_data_newline(struct btf_dump *d) +{

•   return (d->typed_dump->compact || d->typed_dump->state.depth == 0) ?
  

•          "" : "\n";
  

nit: unnecessary () and keep it on single line, it's ok to use all 100 characters in a single line

+}

+static const char *btf_dump_data_delim(struct btf_dump *d) +{

•   return d->typed_dump->state.depth == 0 ? "" : ",";
  

+}

+static void btf_dump_data_pfx(struct btf_dump *d) +{

•   int i, lvl = d->typed_dump->indent_lvl + d->typed_dump->state.depth;
  

•   if (d->typed_dump->compact)
  

•           return;
  

•   for (i = 0; i < lvl; i++)
  

•           btf_dump_printf(d, "%s", d->typed_dump->indent_str);
  

+}

+/* A macro is used here as btf_type_value[s]() appends format specifiers

• • to the format specifier passed in; these do the work of appending
• • delimiters etc while the caller simply has to specify the type values
• • in the format specifier + value(s).
• */

+#define btf_dump_type_values(d, fmt, ...) \

•   btf_dump_printf(d, fmt "%s%s",                                  \
  

•                   __VA_ARGS__,                                    \
  

use ##__VA_ARGS__ to handle the case of empty __VA_ARGS___

•                   btf_dump_data_delim(d),                         \
  

•                   btf_dump_data_newline(d))
  

+static int btf_dump_unsupported_data(struct btf_dump *d,

•                                const struct btf_type *t,
  

•                                __u32 id)
  

+{

•   btf_dump_printf(d, "<unsupported kind:%u>", btf_kind(t));
  

•   return -ENOTSUP;
  

+}

+static void btf_dump_int128(struct btf_dump *d,

•                       const struct btf_type *t,
  

•                       const void *data)
  

+{

•   __int128 num = *(__int128 *)data;
  

•   if ((num >> 64) == 0)
  

•           btf_dump_type_values(d, "0x%llx", (long long)num);
  

•   else
  

•           btf_dump_type_values(d, "0x%llx%016llx", (long long)num >> 32,
  

•                                (long long)num);
  

+}

+static unsigned __int128 btf_dump_bitfield_get_data(struct btf_dump *d,

•                                               const void *data,
  

•                                               __u8 bits_offset,
  

•                                               __u8 bit_sz)
  

+{

•   __u16 left_shift_bits, right_shift_bits;
  

•   __u8 nr_copy_bits, nr_copy_bytes, i;
  

•   unsigned __int128 num = 0, ret;
  

•   const __u8 *bytes = data;
  

•   /* Bitfield value retrieval is done in two steps; first relevant bytes are
  

•    * stored in num, then we left/right shift num to eliminate irrelevant bits.
  

•    */
  

•   nr_copy_bits = bit_sz + bits_offset;
  

•   nr_copy_bytes = roundup(nr_copy_bits, 8) / 8;
  

+#if __BYTE_ORDER == __LITTLE_ENDIAN

•   for (i = 0; i < nr_copy_bytes; i++)
  

•           num += bytes[i] << (8 * i);
  

don't you need (unsigned __int128)bytes[i] cast before the bit shifting?

But I'd recommend to rewrite this logic for little endian as

for (i = nr_copy_bytes - 1; i >= 0; i--) num = num * 256 + bytes[i];

and for big endian as

for (i = 0; i < nr_copy_bytes; i++) num = num * 256 + bytes[i];

+#elif __BYTE_ORDER == __BIG_ENDIAN

•   for (i = nr_copy_bytes - 1; i >= 0; i--)
  

•           num += bytes[i] << (8 * i);
  

+#else +# error "Unrecognized __BYTE_ORDER__" +#endif

•   left_shift_bits = 128 - nr_copy_bits;
  

•   right_shift_bits = 128 - bit_sz;
  

•   ret = (num << left_shift_bits) >> right_shift_bits;
  

•   return ret;
  

+}

+static int btf_dump_bitfield_data(struct btf_dump *d,

•                             const struct btf_type *t,
  

•                             const void *data,
  

•                             __u8 bits_offset,
  

•                             __u8 bit_sz)
  

+{

•   unsigned __int128 print_num;
  

•   print_num = btf_dump_bitfield_get_data(d, data, bits_offset, bit_sz);
  

•   btf_dump_int128(d, t, &print_num);
  

•   return 0;
  

+}

+static int btf_dump_int_bits(struct btf_dump *d,

•                        const struct btf_type *t,
  

•                        const void *data,
  

•                        __u8 bits_offset,
  

•                        __u8 bit_sz)
  

+{

•   data += bits_offset / 8;
  

this is very confusing... you are adjusting data offset by the full amount of bytes in bits_offset but then pass unmodified bits_offset below. I'd expect that either btf_dump_bitfield_data handles this data adjustment internally and transparently and thus callers pass data pointer as is, or each caller would need to make sure that bits_offset < 8, otherwise btf_dump_bitfield_get_data (thought btf_dump_bitfield_data) would be doing something weird. I think the former (btf_dump_bitfield_get_data handles all the cases nicely) is preferrable.

Or am I missing something?

•   return btf_dump_bitfield_data(d, t, data, bits_offset, bit_sz);
  

+}

+static int btf_dump_int_bits_check_zero(struct btf_dump *d,

•                                   const struct btf_type *t,
  

•                                   const void *data,
  

•                                   __u8 bits_offset,
  

•                                   __u8 bit_sz)
  

+{

•   __int128 print_num;
  

•   data += bits_offset / 8;
  

•   print_num = btf_dump_bitfield_get_data(d, data, bits_offset, bit_sz);
  

same confusion as above

•   if (print_num == 0)
  

•           return -ENODATA;
  

•   return 0;
  

+}

+static int btf_dump_int_check_zero(struct btf_dump *d,

•                           const struct btf_type *t,
  

•                           const void *data,
  

•                           __u8 bits_offset)
  

+{

•   __u8 nr_bits = btf_int_bits(t);
  

see below about the problem with BTF_KIND_PTR zero check. If you use t->size here instead, then all the logic will work for BTF_KIND_PTR as well and will handle both 32-bit and 64-bit cases.

•   bool zero = false;
  

•   if (bits_offset)
  

see below about bits_offset always being zero. For this case, though, I'd do the same memcmp() check and keep it nice, short, and agnostic to alignment. WDYT?

•           return btf_dump_int_bits_check_zero(d, t, data, bits_offset, 0);
  

•   switch (nr_bits) {
  

•   case 128:
  

•           zero = *(__int128 *)data == 0;
  

•           break;
  

•   case 64:
  

•           zero = *(__s64 *)data == 0;
  

•           break;
  

•   case 32:
  

•           zero = *(__s32 *)data == 0;
  

•           break;
  

•   case 16:
  

•           zero = *(__s16 *)data == 0;
  

•           break;
  

•   case 8:
  

•           zero = *(__s8 *)data == 0;
  

•           break;
  

•   default:
  

•           pr_warn("unexpected nr_bits %d for int\n", nr_bits);
  

•           return -EINVAL;
  

•   }
  

•   if (zero)
  

•           return -ENODATA;
  

•   return 0;
  

+}

+static int btf_dump_int_data(struct btf_dump *d,

•                        const struct btf_type *t,
  

•                        __u32 type_id,
  

•                        const void *data,
  

•                        __u8 bits_offset)
  

+{

•   __u8 encoding = btf_int_encoding(t);
  

•   bool sign = encoding & BTF_INT_SIGNED;
  

•   __u8 nr_bits = btf_int_bits(t);
  

•   if (bits_offset)
  

•           return btf_dump_int_bits(d, t, data, bits_offset, 0);
  

This is always non-bitfield case, right? This means bits_offset is always a multiple of 8, but I'd argue that it should always be zero instead and data has to be adjusted by the called (which is the case except what I pointed out above).

But struct might be packed and on some architectures accessed below might be problematic. So instead this should be check for whether data is naturally aligned for the desired int type. Isn't that right? So it's something like below

int sz = t->size;

if (sz == 0) return -EINVAL;

if (((uintptr_t)data) % sz) return btf_dump_int_bits(d, t, data, 0, 0);

switch (sz) { ... }

•   switch (nr_bits) {
  

•   case 128:
  

•           btf_dump_int128(d, t, data);
  

•           break;
  

•   case 64:
  

•           if (sign)
  

•                   btf_dump_type_values(d, "%lld", *(long long *)data);
  

•           else
  

•                   btf_dump_type_values(d, "%llu", *(unsigned long long *)data);
  

•           break;
  

•   case 32:
  

•           if (sign)
  

•                   btf_dump_type_values(d, "%d", *(__s32 *)data);
  

•           else
  

•                   btf_dump_type_values(d, "%u", *(__u32 *)data);
  

•           break;
  

•   case 16:
  

•           if (sign)
  

•                   btf_dump_type_values(d, "%d", *(__s16 *)data);
  

•           else
  

•                   btf_dump_type_values(d, "%u", *(__u16 *)data);
  

•           break;
  

•   case 8:
  

•           if (d->typed_dump->state.array_ischar) {
  

•                   /* check for null terminator */
  

•                   if (d->typed_dump->state.array_terminated)
  

•                           break;
  

•                   if (*(char *)data == '\0') {
  

•                           d->typed_dump->state.array_terminated = 1;
  

•                           break;
  

•                   }
  

•                   if (isprint(*(char *)data)) {
  

•                           btf_dump_type_values(d, "'%c'", *(char *)data);
  

•                           break;
  

•                   }
  

•           }
  

•           if (sign)
  

•                   btf_dump_type_values(d, "%d", *(__s8 *)data);
  

•           else
  

•                   btf_dump_type_values(d, "%u", *(__u8 *)data);
  

•           break;
  

•   default:
  

•           pr_warn("unexpected nr_bits %d for id [%u]\n", nr_bits, type_id);
  

•           return -EINVAL;
  

•   }
  

•   return 0;
  

+}

+static int btf_dump_float_check_zero(struct btf_dump *d,

•                                const struct btf_type *t,
  

•                                const void *data)
  

+{

•   static __u8 bytecmp[16] = {};
  

•   int nr_bytes = t->size;
  

•   if (nr_bytes > 16 || nr_bytes < 2) {
  

•           pr_warn("unexpected size %d for float\n", nr_bytes);
  

•           return -EINVAL;
  

•   }
  

•   if (memcmp(data,  bytecmp, nr_bytes) == 0)
  

extra space before bytecmp

•           return -ENOMSG;
  

I think you used -ENODATA in other places?...

•   return 0;
  

+}

+static int btf_dump_float_data(struct btf_dump *d,

•                          const struct btf_type *t,
  

•                          __u32 type_id,
  

•                          const void *data,
  

•                          __u8 bits_offset)
  

bits_offset is not used and it can't be non-zero, right?

+{

•   int nr_bytes = t->size;
  

•   switch (nr_bytes) {
  

•   case 16:
  

•           btf_dump_type_values(d, "%Lf", *(long double *)data);
  

•           break;
  

•   case 8:
  

•           btf_dump_type_values(d, "%f", *(double *)data);
  

%lf?

•           break;
  

•   case 4:
  

•           btf_dump_type_values(d, "%f", *(float *)data);
  

•           break;
  

•   default:
  

•           pr_warn("unexpected size %d for id [%u]\n", nr_bytes, type_id);
  

•           return -EINVAL;
  

•   }
  

•   return 0;
  

+}

+static int btf_dump_var_data(struct btf_dump *d,

•                        const struct btf_type *v,
  

•                        __u32 id,
  

•                        const void *data)
  

+{

•   enum btf_func_linkage linkage = btf_var(v)->linkage;
  

•   const struct btf_type *t;
  

•   const char *l;
  

•   __u32 type_id;
  

•   switch (linkage) {
  

•   case BTF_FUNC_STATIC:
  

•           l = "static ";
  

•           break;
  

•   case BTF_FUNC_EXTERN:
  

•           l = "extern ";
  

•           break;
  

•   case BTF_FUNC_GLOBAL:
  

•   default:
  

•           l = "";
  

•           break;
  

•   }
  

•   /* format of output here is [linkage] [type] [varname] = (type)value,
  

•    * for example "static int cpu_profile_flip = (int)1"
  

•    */
  

•   btf_dump_printf(d, "%s", l);
  

•   type_id = v->type;
  

•   t = btf__type_by_id(d->btf, type_id);
  

•   btf_dump_emit_type_cast(d, type_id, false);
  

•   btf_dump_printf(d, " %s = ", btf_name_of(d, v->name_off));
  

•   return btf_dump_dump_type_data(d, NULL, t, type_id, data, 0, 0);
  

+}

+static int btf_dump_array_data(struct btf_dump *d,

•                          const struct btf_type *t,
  

•                          __u32 id,
  

•                          const void *data)
  

+{

•   const struct btf_array *array = btf_array(t);
  

•   const struct btf_type *elem_type;
  

•   __u32 i, elem_size = 0, elem_type_id;
  

•   int array_member;
  

•   elem_type_id = array->type;
  

•   elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
  

•   elem_size = btf__resolve_size(d->btf, elem_type_id);
  

•   if (elem_size <= 0) {
  

•           pr_warn("unexpected elem size %d for array type [%u]\n", elem_size, id);
  

•           return -EINVAL;
  

•   }
  

•   if (btf_is_int(elem_type)) {
  

•           /*
  

•            * BTF_INT_CHAR encoding never seems to be set for
  

•            * char arrays, so if size is 1 and element is
  

•            * printable as a char, we'll do that.
  

•            */
  

•           if (elem_size == 1)
  

•                   d->typed_dump->state.array_ischar = true;
  

•   }
  

•   d->typed_dump->state.depth++;
  

•   btf_dump_printf(d, "[%s", btf_dump_data_newline(d));
  

•   /* may be a multidimensional array, so store current "is array member"
  

•    * status so we can restore it correctly later.
  

•    */
  

•   array_member = d->typed_dump->state.array_member;
  

•   d->typed_dump->state.array_member = 1;
  

•   for (i = 0;
  

•        i < array->nelems && !d->typed_dump->state.array_terminated;
  

•        i++) {
  

stylistic nit: this multi-line for is quite complicated, let's do a separate

if (d->typed_dump->state.array_terminated) break;

inside the loop and keep the for loop itself short and straightforward

•           btf_dump_dump_type_data(d, NULL, elem_type, elem_type_id, data, 0, 0);
  

•           data += elem_size;
  

data += elem_size actually might go into the for along the i++

•   }
  

•   d->typed_dump->state.array_member = array_member;
  

•   d->typed_dump->state.depth--;
  

•   btf_dump_data_pfx(d);
  

•   btf_dump_printf(d, "]%s%s", btf_dump_data_delim(d), btf_dump_data_newline(d));
  

with ## suggestion above, this should be just

btf_dump_type_values(d, "]")

right?

•   return 0;
  

+}

+static int btf_dump_struct_data(struct btf_dump *d,

•                           const struct btf_type *t,
  

•                           __u32 id,
  

•                           const void *data)
  

+{

•   const struct btf_member *m = btf_members(t);
  

•   __u16 n = btf_vlen(t);
  

•   int i, err;
  

•   d->typed_dump->state.depth++;
  

•   btf_dump_printf(d, "{%s", btf_dump_data_newline(d));
  

subjective nit, but logically you open { and then depth increases, so I'd swap these two lines

•   for (i = 0; i < n; i++, m++) {
  

•           const struct btf_type *mtype;
  

•           const char *mname;
  

•           __u32 moffset;
  

•           __u8 bit_sz;
  

•           mtype = btf__type_by_id(d->btf, m->type);
  

•           mname = btf_name_of(d, m->name_off);
  

•           moffset = btf_member_bit_offset(t, i);
  

•           bit_sz = btf_member_bitfield_size(t, i);
  

•           err = btf_dump_dump_type_data(d, mname, mtype, m->type, data + moffset / 8,
  

•                                         moffset % 8, bit_sz);
  

•           if (err < 0)
  

•                   return err;
  

•   }
  

•   d->typed_dump->state.depth--;
  

•   btf_dump_data_pfx(d);
  

•   btf_dump_printf(d, "}%s%s", btf_dump_data_delim(d), btf_dump_data_newline(d));
  

same as above, could be btf_dump_type_values(), if I'm not mistaken

btw, as opposed to above here the sequence makes sense, depth is decreased first, then we emit closing }

•   return err;
  

+}

+static int btf_dump_ptr_data(struct btf_dump *d,

•                         const struct btf_type *t,
  

•                         __u32 id,
  

•                         const void *data)
  

+{

•   btf_dump_type_values(d, "%p", *(void **)data);
  

see below about making assumptions about pointer size, in general case this is wrong

•   return 0;
  

+}

+static int btf_dump_get_enum_value(const struct btf_type *t,

•                              const void *data,
  

•                              __u32 id,
  

•                              __s64 *value)
  

+{

•   switch (t->size) {
  

•   case 8:
  

•           *value = *(__s64 *)data;
  

•           return 0;
  

•   case 4:
  

•           *value = *(__s32 *)data;
  

•           return 0;
  

•   case 2:
  

•           *value = *(__s16 *)data;
  

•           return 0;
  

•   case 1:
  

•           *value = *(__s8 *)data;
  

•   default:
  

•           pr_warn("unexpected size %d for enum, id:[%u]\n", t->size, id);
  

•           return -EINVAL;
  

•   }
  

similar to handling floats and ints, we need to account for packed unaligned data

+}

+static int btf_dump_enum_data(struct btf_dump *d,

•                         const struct btf_type *t,
  

•                         __u32 id,
  

•                         const void *data)
  

+{

•   const struct btf_enum *e;
  

•   __s64 value;
  

•   int i, err;
  

•   err = btf_dump_get_enum_value(t, data, id, &value);
  

•   if (err)
  

•           return err;
  

•   for (i = 0, e = btf_enum(t); i < btf_vlen(t); i++, e++) {
  

•           if (value != e->val)
  

•                   continue;
  

•           btf_dump_type_values(d, "%s", btf_name_of(d, e->name_off));
  

•           return 0;
  

•   }
  

•   btf_dump_type_values(d, "%d", value);
  

•   return 0;
  

+}

+static int btf_dump_datasec_data(struct btf_dump *d,

•                            const struct btf_type *t,
  

•                            __u32 id,
  

•                            const void *data)
  

+{

•   const struct btf_var_secinfo *vsi;
  

•   const struct btf_type *var;
  

•   __u32 i;
  

•   int err;
  

•   btf_dump_type_values(d, "SEC(\"%s\") ",
  

•                        btf_name_of(d, t->name_off));
  

nit: single line

•   for (i = 0, vsi = btf_var_secinfos(t); i < btf_vlen(t); i++, vsi++) {
  

•           var = btf__type_by_id(d->btf, vsi->type);
  

•           err = btf_dump_dump_type_data(d, NULL, var, vsi->type, data + vsi->offset, 0, 0);
  

•           if (err < 0)
  

•                   return err;
  

•           btf_dump_printf(d, ";");
  

•   }
  

•   return 0;
  

+}

+/* return size of type, or if base type overflows, return -E2BIG. */ +static int btf_dump_type_data_check_overflow(struct btf_dump *d,

•                                        const struct btf_type *t,
  

•                                        __u32 id,
  

•                                        const void *data,
  

•                                        __u8 bits_offset)
  

+{

•   __s64 size = btf__resolve_size(d->btf, id);
  

•   if (size < 0 || size >= INT_MAX) {
  

•           pr_warn("unexpected size [%lld] for id [%u]\n",
  

•                   size, id);
  

•           return -EINVAL;
  

•   }
  

•   /* Only do overflow checking for base types; we do not want to
  

•    * avoid showing part of a struct, union or array, even if we
  

•    * do not have enough data to show the full object.  By
  

•    * restricting overflow checking to base types we can ensure
  

•    * that partial display succeeds, while avoiding overflowing
  

•    * and using bogus data for display.
  

•    */
  

•   t = skip_mods_and_typedefs(d->btf, id, NULL);
  

•   if (!t) {
  

•           pr_warn("unexpected error skipping mods/typedefs for id [%u]\n",
  

•                   id);
  

•           return -EINVAL;
  

•   }
  

•   switch (BTF_INFO_KIND(t->info)) {
  

btf_kind(t)

•   case BTF_KIND_INT:
  

•   case BTF_KIND_PTR:
  

•   case BTF_KIND_ENUM:
  

BTF_KIND_FLOAT as well, right?

•           if (data + bits_offset / 8 + size > d->typed_dump->data_end)
  

•                   return -E2BIG;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   return (int)size;
  

+}

+static int btf_dump_type_data_check_zero(struct btf_dump *d,

•                                    const struct btf_type *t,
  

•                                    __u32 id,
  

•                                    const void *data,
  

•                                    __u8 bits_offset,
  

•                                    __u8 bit_sz)
  

+{

•   __s64 value;
  

•   int i, err;
  

•   /* toplevel exceptions; we show zero values if
  

•    * - we ask for them (emit_zeros)
  

•    * - if we are at top-level so we see "struct empty { }"
  

•    * - or if we are an array member and the array is non-empty and
  

•    *   not a char array; we don't want to be in a situation where we
  

•    *   have an integer array 0, 1, 0, 1 and only show non-zero values.
  

•    *   If the array contains zeroes only, or is a char array starting
  

•    *   with a '\0', the array-level check_zero() will prevent showing it;
  

•    *   we are concerned with determining zero value at the array member
  

•    *   level here.
  

•    */
  

•   if (d->typed_dump->emit_zeroes || d->typed_dump->state.depth == 0 ||
  

•       (d->typed_dump->state.array_member &&
  

•        !d->typed_dump->state.array_ischar))
  

•           return 0;
  

•   t = skip_mods_and_typedefs(d->btf, id, NULL);
  

•   if (!t) {
  

see below, btf_dump_type_data_check_overflow already validated everything, no need to re-check this, let's keep the code clean and succinct

•           pr_warn("unexpected error skipping mods/typedefs for id [%u]\n",
  

•                   id);
  

•           return -EINVAL;
  

•   }
  

•   switch (BTF_INFO_KIND(t->info)) {
  

btf_kind(t)

•   case BTF_KIND_INT:
  

•           if (bit_sz)
  

•                   return btf_dump_int_bits_check_zero(d, t, data,
  

•                                                       bits_offset, bit_sz);
  

single line is ok here

•           return btf_dump_int_check_zero(d, t, data, bits_offset);
  

•   case BTF_KIND_FLOAT:
  

•           return btf_dump_float_check_zero(d, t, data);
  

•   case BTF_KIND_PTR:
  

•           if (*((void **)data) == NULL)
  

so this is wrong when BTF's pointer size differs from host pointer size (e.g., 64-bit BTF vs 32-bit host and vice versa). It's better to do byte-by-byte comparison based on ptr type's size

•                   return -ENODATA;
  

•           return 0;
  

•   case BTF_KIND_ARRAY: {
  

•           const struct btf_array *array = btf_array(t);
  

•           const struct btf_type *elem_type;
  

•           __u32 elem_type_id, elem_size;
  

•           bool ischar;
  

•           elem_type_id = array->type;
  

•           elem_size = btf__resolve_size(d->btf, elem_type_id);
  

•           elem_type =  btf__type_by_id(d->btf, elem_type_id);
  

•           ischar = btf_is_int(elem_type) && elem_size == 1;
  

this won't work if element type is const int or is a typedef. So need to skip_mods_and_typedefs before checking this.

•           /* check all elements; if _any_ element is nonzero, all
  

•            * of array is displayed.  We make an exception however
  

•            * for char arrays where the first element is 0; these
  

•            * are considered zeroed also, even if later elements are
  

•            * non-zero because the string is terminated.
  

•            */
  

•           for (i = 0; i < array->nelems; i++) {
  

•                   if (i == 0 && ischar && *(char *)data == 0)
  

•                           return -ENODATA;
  

•                   err = btf_dump_type_data_check_zero(d, elem_type,
  

•                                                       elem_type_id,
  

•                                                       data +
  

•                                                       (i * elem_size),
  

•                                                       bits_offset, 0);
  

•                   if (err != -ENODATA)
  

•                           return err;
  

•           }
  

•           return -ENODATA;
  

•   }
  

•   case BTF_KIND_STRUCT:
  

•   case BTF_KIND_UNION: {
  

•           const struct btf_member *m = btf_members(t);
  

•           __u16 n = btf_vlen(t);
  

•           /* if any struct/union member is non-zero, the struct/union
  

•            * is considered non-zero and dumped.
  

•            */
  

•           for (i = 0; i < n; i++, m++) {
  

•                   const struct btf_type *mtype;
  

•                   __u32 moffset;
  

•                   __u8 bit_sz;
  

•                   mtype = btf__type_by_id(d->btf, m->type);
  

•                   moffset = btf_member_bit_offset(t, i);
  

•                   /* btf_int_bits() does not store member bitfield size;
  

•                    * bitfield size needs to be stored here so int display
  

•                    * of member can retrieve it.
  

•                    */
  

•                   bit_sz = btf_member_bitfield_size(t, i);
  

•                   err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
  

•                                                       moffset % 8, bit_sz);
  

•                   if (err != ENODATA)
  

•                           return err;
  

•           }
  

•           return -ENODATA;
  

•   }
  

•   case BTF_KIND_ENUM:
  

•           if (btf_dump_get_enum_value(t, data, id, &value))
  

•                   return 0;
  

•           if (value == 0)
  

•                   return -ENODATA;
  

•           return 0;
  

•   default:
  

•           return 0;
  

•   }
  

+}

+/* returns size of data dumped, or error. */ +static int btf_dump_dump_type_data(struct btf_dump *d,

•                              const char *fname,
  

•                              const struct btf_type *t,
  

•                              __u32 id,
  

•                              const void *data,
  

•                              __u8 bits_offset,
  

•                              __u8 bit_sz)
  

+{

•   int size, err;
  

•   size = btf_dump_type_data_check_overflow(d, t, id, data, bits_offset);
  

•   if (size < 0)
  

•           return size;
  

•   err = btf_dump_type_data_check_zero(d, t, id, data, bits_offset, bit_sz);
  

•   if (err) {
  

•           /* zeroed data is expected and not an error, so simply skip
  

•            * dumping such data.  Record other errors however.
  

•            */
  

•           if (err == -ENODATA)
  

•                   return size;
  

•           return err;
  

•   }
  

•   btf_dump_data_pfx(d);
  

•   if (!d->typed_dump->skip_names) {
  

•           if (fname && strlen(fname) > 0)
  

•                   btf_dump_printf(d, ".%s = ", fname);
  

•           btf_dump_emit_type_cast(d, id, true);
  

•   }
  

•   t = skip_mods_and_typedefs(d->btf, id, NULL);
  

•   if (!t) {
  

btf_dump_type_data_check_overflow already ensured this can't happen, let's drop unnecessary check

•           pr_warn("unexpected error skipping mods/typedefs for id [%u]\n",
  

•                   id);
  

•           return -EINVAL;
  

•   }
  

•   switch (BTF_INFO_KIND(t->info)) {
  

btf_kind(t)

•   case BTF_KIND_UNKN:
  

•   case BTF_KIND_FWD:
  

•   case BTF_KIND_FUNC:
  

•   case BTF_KIND_FUNC_PROTO:
  

•           err = btf_dump_unsupported_data(d, t, id);
  

•           break;
  

•   case BTF_KIND_INT:
  

•           if (bit_sz)
  

•                   err = btf_dump_bitfield_data(d, t, data, bits_offset, bit_sz);
  

•           else
  

•                   err = btf_dump_int_data(d, t, id, data, bits_offset);
  

•           break;
  

•   case BTF_KIND_FLOAT:
  

•           err = btf_dump_float_data(d, t, id, data, bits_offset);
  

•           break;
  

•   case BTF_KIND_PTR:
  

•           err = btf_dump_ptr_data(d, t, id, data);
  

•           break;
  

•   case BTF_KIND_ARRAY:
  

•           err = btf_dump_array_data(d, t, id, data);
  

•           break;
  

•   case BTF_KIND_STRUCT:
  

•   case BTF_KIND_UNION:
  

•           err = btf_dump_struct_data(d, t, id, data);
  

•           break;
  

•   case BTF_KIND_ENUM:
  

•           /* handle bitfield and int enum values */
  

•           if (bit_sz) {
  

•                   unsigned __int128 print_num;
  

•                   __s64 enum_val;
  

•                   print_num = btf_dump_bitfield_get_data(d, data, bits_offset, bit_sz);
  

•                   enum_val = (__s64)print_num;
  

•                   err = btf_dump_enum_data(d, t, id, &enum_val);
  

•           } else
  

•                   err = btf_dump_enum_data(d, t, id, data);
  

•           break;
  

•   case BTF_KIND_VAR:
  

•           err = btf_dump_var_data(d, t, id, data);
  

•           break;
  

•   case BTF_KIND_DATASEC:
  

•           err = btf_dump_datasec_data(d, t, id, data);
  

•           break;
  

•   default:
  

•           pr_warn("unexpected kind [%u] for id [%u]\n",
  

•                   BTF_INFO_KIND(t->info), id);
  

•           return -EINVAL;
  

•   }
  

•   if (err < 0)
  

•           return err;
  

•   return size;
  

+}

+int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,

•                        const void *data, size_t data_sz,
  

•                        const struct btf_dump_type_data_opts *opts)
  

+{

•   const struct btf_type *t;
  

•   int ret;
  

•   if (!OPTS_VALID(opts, btf_dump_type_data_opts))
  

•           return libbpf_err(-EINVAL);
  

•   t = btf__type_by_id(d->btf, id);
  

•   if (!t)
  

•           return libbpf_err(-ENOENT);
  

•   d->typed_dump = calloc(1, sizeof(struct btf_dump_data));
  

•   if (!d->typed_dump)
  

•           return libbpf_err(-ENOMEM);
  

•   d->typed_dump->data_end = data + data_sz;
  

•   d->typed_dump->indent_lvl = OPTS_GET(opts, indent_level, 0);
  

•   /* default indent string is a tab */
  

•   if (!opts->indent_str)
  

•           d->typed_dump->indent_str[0] = '\t';
  

both sides of if have to have have {} or not, but not mixed

•   else {
  

•           if (strlen(opts->indent_str) >
  

•               sizeof(d->typed_dump->indent_str) - 1)
  

•                   return libbpf_err(-EINVAL);
  

gotta free d->typed_dump here, so goto cleanup of some kind is needed here. I'd probably just silently truncate the provided string, though.

•           strncpy(d->typed_dump->indent_str, opts->indent_str,
  

•                   sizeof(d->typed_dump->indent_str) - 1);
  

GCC will probably complain about potentially non-zero terminated indent_str. Let's simplify all this with s/strncpy/strncat/ and dropping the strlen check above. It will silently truncate and ensure zero termination.

•   }
  

•   d->typed_dump->compact = OPTS_GET(opts, compact, false);
  

•   d->typed_dump->skip_names = OPTS_GET(opts, skip_names, false);
  

•   d->typed_dump->emit_zeroes = OPTS_GET(opts, emit_zeroes, false);
  

•   ret = btf_dump_dump_type_data(d, NULL, t, id, data, 0, 0);
  

•   free(d->typed_dump);
  

•   return libbpf_err(ret);
  

+} diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index 944c99d..5bfc107 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -373,5 +373,6 @@ LIBBPF_0.5.0 { bpf_map__initial_value; bpf_map_lookup_and_delete_elem_flags; bpf_object__gen_loader;

•           btf_dump__dump_type_data;
            libbpf_set_strict_mode;
  

} LIBBPF_0.4.0;

1.8.3.1

On Sat, Jun 19, 2021 at 1:56 AM Alan Maguire alan.maguire@oracle.com wrote:

It will support strncmp()-style string comparisons.

Suggested-by: Andrii Nakryiko andrii.nakryiko@gmail.com Signed-off-by: Alan Maguire alan.maguire@oracle.com

---

tools/testing/selftests/bpf/test_progs.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h index 8ef7f33..d2944da 100644 --- a/tools/testing/selftests/bpf/test_progs.h +++ b/tools/testing/selftests/bpf/test_progs.h @@ -221,6 +221,18 @@ struct test_env { ___ok; \ })

+#define ASSERT_STRNEQ(actual, expected, len, name) ({ \

•   static int duration = 0;                                        \
  

•   const char *___act = actual;                                    \
  

•   const char *___exp = expected;                                  \
  

•   size_t ___len = len;                                            \
  

•   bool ___ok = strncmp(___act, ___exp, ___len) == 0;              \
  

•   CHECK(!___ok, (name),                                           \
  

•         "unexpected %s: actual '%s' != expected '%s'\n",          \
  

•         (name), ___act, ___exp);                                  \
  

it would be nice to only emit what we are actually comparing - first n characters of each string. Luckily, printf is cool enough to support this:

printf("actual '%.*s' != expected '%.*s'\n", ___len, ___act, ___len, ___exp);

•   ___ok;                                                          \
  

+})

#define ASSERT_OK(res, name) ({ \ static int duration = 0; \ long long ___res = (res); \ -- 1.8.3.1

On Sat, Jun 19, 2021 at 1:56 AM Alan Maguire alan.maguire@oracle.com wrote:

Test various type data dumping operations by comparing expected format with the dumped string; an snprintf-style printf function is used to record the string dumped. Also verify overflow handling where the data passed does not cover the full size of a type, such as would occur if a tracer has a portion of the 8k "struct task_struct".

Signed-off-by: Alan Maguire alan.maguire@oracle.com

tools/testing/selftests/bpf/prog_tests/btf_dump.c | 644 ++++++++++++++++++++++ 1 file changed, 644 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c index 1b90e68..c894201 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c @@ -232,7 +232,621 @@ void test_btf_dump_incremental(void) btf__free(btf); }

+#define STRSIZE 4096

+static void btf_dump_snprintf(void *ctx, const char *fmt, va_list args) +{

•   char *s = ctx, new[STRSIZE];
  

•   vsnprintf(new, STRSIZE, fmt, args);
  

•   strncat(s, new, STRSIZE);
  

this can cause stack corruption, because strncat doesn't take into account the length of string in s and might copy all STRSIZE bytes from new. What you want here is actually strlcat() variant, but I'm not sure it's available in Linux glibc. Instead, you have to pass STRSIZE - strlen(s) - 1 to strncat.

+}

+/* skip "enum "/"struct " prefixes */ +#define SKIP_PREFIX(_typestr, _prefix) \

•   do {                                                            \
  

•           if (strncmp(_typestr, _prefix, strlen(_prefix)) == 0)   \
  

•                   _typestr += strlen(_prefix) + 1;                \
  

If you expect "enum " or "struct " (not, say, "enum/"), then the test should just pass that in explicitly instead of SKIP_PREFIX silently ignoring an extra *any* character.

•   } while (0)
  

[...]

+/* overflow test; pass typesize < expected type size, ensure E2BIG returned */ +#define TEST_BTF_DUMP_DATA_OVER(_b, _d, _str, _type, _type_sz, _expected, ...)\

•   do {                                                            \
  

•           char __ptrtype[64] = #_type;                            \
  

•           char *_ptrtype = (char *)__ptrtype;                     \
  

•           _type _ptrdata = __VA_ARGS__;                           \
  

•           void *_ptr = &_ptrdata;                                 \
  

•           int _err;                                               \
  

•                                                                   \
  

•           _err = btf_dump_data(_b, _d, _ptrtype, 0, _ptr,         \
  

•                                _type_sz, _str, _expected);        \
  

•           if (_err < 0)                                           \
  

•                   return;                                         \
  

don't return, let all the validation run. It's better to see all the failures than fix one by one, recompile, rerun, then fix another one. Same for TEST_BTF_DUMP_DATA above.

•   } while (0)
  

+#define TEST_BTF_DUMP_VAR(_b, _d, _str, _var, _type, _flags, _expected, ...) \

•   do {                                                            \
  

•           _type _ptrdata = __VA_ARGS__;                           \
  

•           void *_ptr = &_ptrdata;                                 \
  

•           int _err;                                               \
  

•                                                                   \
  

•           _err = btf_dump_data(_b, _d, _var, _flags, _ptr,        \
  

•                                sizeof(_type), _str, _expected);   \
  

•           if (_err < 0)                                           \
  

•                   return;                                         \
  

same, don't return early

•   } while (0)
  

+static void test_btf_dump_int_data(struct btf *btf, struct btf_dump *d,

•                              char *str)
  

+{

•   /* simple int */
  

•   TEST_BTF_DUMP_DATA_C(btf, d, str, int, BTF_F_COMPACT, 1234);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, BTF_F_COMPACT | BTF_F_NONAME,
  

•                      "1234", 1234);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, 0, "(int)1234", 1234);
  

•   /* zero value should be printed at toplevel */
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, BTF_F_COMPACT, "(int)0", 0);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, BTF_F_COMPACT | BTF_F_NONAME,
  

•                      "0", 0);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, BTF_F_COMPACT | BTF_F_ZERO,
  

•                      "(int)0", 0);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int,
  

•                      BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
  

•                      "0", 0);
  

•   TEST_BTF_DUMP_DATA_C(btf, d, str, int, BTF_F_COMPACT, -4567);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, BTF_F_COMPACT | BTF_F_NONAME,
  

•                      "-4567", -4567);
  

•   TEST_BTF_DUMP_DATA(btf, d, str, int, 0, "(int)-4567", -4567);
  

•   TEST_BTF_DUMP_DATA_OVER(btf, d, str, int, sizeof(int)-1, "", 1);
  

all of these validations are independent of each other, so there is no need to return early if any one fails (see above)

+}

+static void test_btf_dump_float_data(struct btf *btf, struct btf_dump *d,

•                                char *str)
  

+{

•   float t1 = 1.234567;
  

•   float t2 = -1.234567;
  

•   float t3 = 0.0;
  

•   double t4 = 5.678912;
  

•   double t5 = -5.678912;
  

•   double t6 = 0.0;
  

•   long double t7 = 9.876543;
  

•   long double t8 = -9.876543;
  

•   long double t9 = 0.0;
  

•   /* since the kernel does not likely have any float types in its BTF, we
  

•    * will need to add some of various sizes.
  

•    */
  

•   if (!ASSERT_GT(btf__add_float(btf, "test_float", 4), 0, "add float"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_float", 0, &t1, 4, str,
  

•                                "(test_float)1.234567"), "dump float"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_float", 0, &t2, 4, str,
  

•                                "(test_float)-1.234567"), "dump float"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_float", 0, &t3, 4, str,
  

•                                "(test_float)0.000000"), "dump float"))
  

•           return;
  

•   if (!ASSERT_GT(btf__add_float(btf, "test_double", 8), 0, "add_double"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_double", 0, &t4, 8, str,
  

•                                "(test_double)5.678912"), "dump double"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_double", 0, &t5, 8, str,
  

•                                "(test_double)-5.678912"), "dump double"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_double", 0, &t6, 8, str,
  

•                                "(test_double)0.000000"), "dump double"))
  

•           return;
  

•   if (!ASSERT_GT(btf__add_float(btf, "test_long_double", 16), 0,
  

•                  "add_long_double"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_long_double", 0, &t7, 16,
  

•                                str, "(test_long_double)9.876543"),
  

•                                "dump long_double"))
  

•           return;
  

•   if (!ASSERT_OK(btf_dump_data(btf, d, "test_long_double", 0, &t8, 16,
  

•                                str, "(test_long_double)-9.876543"),
  

•                                "dump long_double"))
  

•           return;
  

•   ASSERT_OK(btf_dump_data(btf, d, "test_long_double", 0, &t9, 16,
  

•                           str, "(test_long_double)0.000000"),
  

•                           "dump long_double");
  

same, don't return, just have a list of assertions

+}

[...]

•                      { .next = (struct list_head *)1 });
  

•   /* NULL pointer should not be displayed */
  

•   TEST_BTF_DUMP_DATA(btf, d, str, struct list_head, BTF_F_COMPACT,
  

•                      "(struct list_head){}",
  

•                      { .next = (struct list_head *)0 });
  

•   TEST_BTF_DUMP_DATA(btf, d, str, struct list_head, 0,
  

+"(struct list_head){\n" +"}",

•                      { .next = (struct list_head *)0 });
  

•   /* struct with function pointers */
  

•   type_id = btf__find_by_name(btf, "file_operations");
  

•   if (CHECK(type_id <= 0, "find type id",
  

some more CHECK leftovers, please switch all CHECKs to ASSERT_xxx

•             "no 'struct file_operations' in BTF: %d\n", type_id))
  

•           return;
  

•   type_sz = btf__resolve_size(btf, type_id);
  

•   str[0] = '\0';
  

•   ret = btf_dump__dump_type_data(d, type_id, fops, type_sz, &opts);
  

•   if (CHECK(ret != type_sz,
  

•             "dump file_operations is successful",
  

•             "unexpected return value dumping file_operations '%s': %d\n",
  

•             str, ret))
  

•           return;
  

•   cmpstr =
  

+"(struct file_operations){\n" +" .owner = (struct module *)0xffffffffffffffff,\n" +" .llseek = (loff_t (*)(struct file *, loff_t, int))0xffffffffffffffff,";

•   if (!ASSERT_STRNEQ(str, cmpstr, strlen(cmpstr), "file_operations"))
  

•           return;
  

same as above, even if this fails, we can still run all the other validations safely

•   /* struct with char array */
  

•   TEST_BTF_DUMP_DATA(btf, d, str, struct bpf_prog_info, BTF_F_COMPACT,
  

•                      "(struct bpf_prog_info){.name = (char[16])['f','o','o',],}",
  

•                      { .name = "foo",});
  

•   TEST_BTF_DUMP_DATA(btf, d, str, struct bpf_prog_info,
  

•                      BTF_F_COMPACT | BTF_F_NONAME,
  

•                      "{['f','o','o',],}",
  

•                      {.name = "foo",});
  

[...]

void test_btf_dump() {

•   char str[STRSIZE];
  

•   struct btf_dump_opts opts = { .ctx = str };
  

•   struct btf_dump *d;
  

•   struct btf *btf;
    int i;
  
    for (i = 0; i < ARRAY_SIZE(btf_dump_test_cases); i++) {
  

@@ -245,4 +859,34 @@ void test_btf_dump() { } if (test__start_subtest("btf_dump: incremental")) test_btf_dump_incremental();

•   btf = libbpf_find_kernel_btf();
  

•   if (CHECK(!btf, "get kernel BTF", "no kernel BTF found"))
  

•           return;
  

•   d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf);
  

•   if (CHECK(!d, "new dump", "could not create BTF dump"))
  

•           return;
  

goto clean and free dumper and btf?

•   /* Verify type display for various types. */
  

•   if (test__start_subtest("btf_dump: int_data"))
  

•           test_btf_dump_int_data(btf, d, str);
  

•   if (test__start_subtest("btf_dump: float_data"))
  

•           test_btf_dump_float_data(btf, d, str);
  

•   if (test__start_subtest("btf_dump: char_data"))
  

•           test_btf_dump_char_data(btf, d, str);
  

•   if (test__start_subtest("btf_dump: typedef_data"))
  

•           test_btf_dump_typedef_data(btf, d, str);
  

•   if (test__start_subtest("btf_dump: enum_data"))
  

•           test_btf_dump_enum_data(btf, d, str);
  

•   if (test__start_subtest("btf_dump: struct_data"))
  

•           test_btf_dump_struct_data(btf, d, str);
  

•   if (test__start_subtest("btf_dump: var_data"))
  

•           test_btf_dump_var_data(btf, d, str);
  

•   btf_dump__free(d);
  

•   btf__free(btf);
  

•   if (test__start_subtest("btf_dump: datasec_data"))
  

•           test_btf_dump_datasec_data(str);
  

}

1.8.3.1