On 10/17/18 4:11 AM, Edward Cree wrote: > 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.
Thanks for the review! Will change to use the "do while" format as you suggested. >> +#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? No particular reason. I will try to use it in the next revision. >> +#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); > } Good suggestion. Will make change in the next revision. > > -Ed >