On 17/10/18 08:24, Yonghong Song wrote:
> This patch added support to print function signature
> if btf func_info is available. Note that ksym
> now uses function name instead of prog_name as
> prog_name has a limit of 16 bytes including
> ending '\0'.
>
> The following is a sample output for selftests
> test_btf with file test_btf_haskv.o:
>
> $ bpftool prog dump jited id 1
> int _dummy_tracepoint(struct dummy_tracepoint_args * ):
> bpf_prog_b07ccb89267cf242__dummy_tracepoint:
> 0: push %rbp
> 1: mov %rsp,%rbp
> ......
> 3c: add $0x28,%rbp
> 40: leaveq
> 41: retq
>
> int test_long_fname_1(struct dummy_tracepoint_args * ):
> bpf_prog_2dcecc18072623fc_test_long_fname_1:
> 0: push %rbp
> 1: mov %rsp,%rbp
> ......
> 3a: add $0x28,%rbp
> 3e: leaveq
> 3f: retq
>
> int test_long_fname_2(struct dummy_tracepoint_args * ):
> bpf_prog_89d64e4abf0f0126_test_long_fname_2:
> 0: push %rbp
> 1: mov %rsp,%rbp
> ......
> 80: add $0x28,%rbp
> 84: leaveq
> 85: retq
>
> Signed-off-by: Yonghong Song <y...@fb.com>
> ---
> tools/bpf/bpftool/btf_dumper.c | 96 ++++++++++++++++++++++++++++++++++
> tools/bpf/bpftool/main.h | 2 +
> tools/bpf/bpftool/prog.c | 54 +++++++++++++++++++
> 3 files changed, 152 insertions(+)
>
> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index 55bc512a1831..a31df4202335 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c
> @@ -249,3 +249,99 @@ int btf_dumper_type(const struct btf_dumper *d, __u32
> type_id,
> {
> return btf_dumper_do_type(d, type_id, 0, data);
> }
> +
> +#define BTF_PRINT_STRING(str)
> \
> + { \
> + pos += snprintf(func_sig + pos, size - pos, str); \
> + if (pos >= size) \
> + return -1; \
> + }
Usual kernel practice for this sort of macro is to use
do { \
} while(0)
to ensure correct behaviour if the macro is used within another control
flow statement, e.g.
if (x)
BTF_PRINT_STRING(x);
else
do_something_else();
will not compile with the bare braces as the else will be detached.
> +#define BTF_PRINT_ONE_ARG(fmt, arg) \
> + { \
> + pos += snprintf(func_sig + pos, size - pos, fmt, arg); \
> + if (pos >= size) \
> + return -1; \
> + }
Any reason for not just using a variadic macro?
> +#define BTF_PRINT_TYPE_ONLY(type) \
> + { \
> + pos = __btf_dumper_type_only(btf, type, func_sig, \
> + pos, size); \
> + if (pos == -1) \
> + return -1; \
> + }
> +
> +static int __btf_dumper_type_only(struct btf *btf, __u32 type_id,
> + char *func_sig, int pos, int size)
> +{
> + const struct btf_type *t = btf__type_by_id(btf, type_id);
> + const struct btf_array *array;
> + int i, vlen;
> +
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_INT:
> + BTF_PRINT_ONE_ARG("%s ",
> + btf__name_by_offset(btf, t->name_off));
> + break;
> + case BTF_KIND_STRUCT:
> + BTF_PRINT_ONE_ARG("struct %s ",
> + btf__name_by_offset(btf, t->name_off));
> + break;
> + case BTF_KIND_UNION:
> + BTF_PRINT_ONE_ARG("union %s ",
> + btf__name_by_offset(btf, t->name_off));
> + break;
> + case BTF_KIND_ENUM:
> + BTF_PRINT_ONE_ARG("enum %s ",
> + btf__name_by_offset(btf, t->name_off));
> + break;
> + case BTF_KIND_ARRAY:
> + array = (struct btf_array *)(t + 1);
> + BTF_PRINT_TYPE_ONLY(array->type);
> + BTF_PRINT_ONE_ARG("[%d]", array->nelems);
> + break;
> + case BTF_KIND_PTR:
> + BTF_PRINT_TYPE_ONLY(t->type);
> + BTF_PRINT_STRING("* ");
> + break;
> + case BTF_KIND_UNKN:
> + case BTF_KIND_FWD:
> + case BTF_KIND_TYPEDEF:
> + return -1;
> + case BTF_KIND_VOLATILE:
> + BTF_PRINT_STRING("volatile ");
> + BTF_PRINT_TYPE_ONLY(t->type);
> + break;
> + case BTF_KIND_CONST:
> + BTF_PRINT_STRING("const ");
> + BTF_PRINT_TYPE_ONLY(t->type);
> + break;
> + case BTF_KIND_RESTRICT:
> + BTF_PRINT_STRING("restrict ");
> + BTF_PRINT_TYPE_ONLY(t->type);
> + break;
> + case BTF_KIND_FUNC:
> + case BTF_KIND_FUNC_PROTO:
> + BTF_PRINT_TYPE_ONLY(t->type);
> + BTF_PRINT_ONE_ARG("%s(", btf__name_by_offset(btf, t->name_off));
> + vlen = BTF_INFO_VLEN(t->info);
> + for (i = 0; i < vlen; i++) {
> + __u32 arg_type = ((__u32 *)(t + 1))[i];
> +
> + BTF_PRINT_TYPE_ONLY(arg_type);
> + if (i != (vlen - 1))
> + BTF_PRINT_STRING(", ");
> + }
In this kind of loop I find it cleaner to print the comma before the item;
that way the test becomes i != 0. Thus:
for (i = 0; i < vlen; i++) {
__u32 arg_type = ((__u32 *)(t + 1))[i];
if (i)
BTF_PRINT_STRING(", ");
BTF_PRINT_TYPE_ONLY(arg_type);
}
-Ed
