On Tue, Mar 03, 2020 at 10:00:02AM -0800, Andrii Nakryiko wrote:
> On Tue, Mar 3, 2020 at 9:33 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >
> > On Tue, Mar 03, 2020 at 09:09:38AM -0800, Andrii Nakryiko wrote:
> > > On Tue, Mar 3, 2020 at 6:12 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > > >
> > > > hi,
> > > > for bpftrace I'd like to print BTF functions (BTF_KIND_FUNC)
> > > > declarations together with their names.
> > > >
> > > > I saw we have btf_dump__emit_type_decl and added BTF_KIND_FUNC,
> > > > where it seemed to be missing, so it prints out something now
> > > > (not sure it's the right fix though).
> > > >
> > > > Anyway, would you be ok with adding some flag/bool to struct
> > > > btf_dump_emit_type_decl_opts, so I could get output like:
> > > >
> > > > kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> > > > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> > > >
> > > > ... to be able to the arguments and return type separated,
> > > > so I could easily get to something like above?
> > > >
> > > > Current interface is just vfprintf callback and I'm not sure
> > > > I can rely that it will allywas be called with same arguments,
> > > > like having separated calls for parsed atoms like 'return type',
> > > > '(', ')', '(', 'arg type', 'arg name', ...
> > > >
> > > > I'm open to any suggestion ;-)
> > >
> > > Hey Jiri!
> > >
> > > Can you please elaborate on the use case and problem you are trying to solve?
> > >
> > > I think we can (and probably even should) add such option and support
> > > to dump functions, but whatever we do it should be a valid C syntax
> > > and should be compilable.
> > > Example above:
> > >
> > > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> > >
> > > Is this really the syntax you need to get? I think btf_dump, when
> > > (optionally) emitting function declaration, will have to emit that
> > > particular one as:
> > >
> > > size_t ksys_read(unsigned int fd, char buf, long unsigned int count);
> > >
> > > But I'd like to hear the use case before we add this. Thanks!
> >
> > the use case is just for the 'bpftrace -l' output, which displays
> > the probe names that could be used.. for kernel BTF kernel functions
> > it's 'kfunc:function(args)'
> >
> > software:task-clock:
> > hardware:backend-stalls:
> > hardware:branch-instructions:
> > ...
> > tracepoint:kvmmmu:kvm_mmu_pagetable_walk
> > tracepoint:kvmmmu:kvm_mmu_paging_element
> > ...
> > kprobe:console_on_rootfs
> > kprobe:trace_initcall_start_cb
> > kprobe:run_init_process
> > kprobe:try_to_run_init_process
> > ...
> > kfunc:x86_reserve_hardware
> > kfunc:hw_perf_lbr_event_destroy
> > kfunc:x86_perf_event_update
> >
> > I dont want to print the return type as is in C, because it would
> > mess up the whole output, hence the '= <return type>'
> >
> > kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> >
> > also possible only in verbose mode ;-)
> >
> > the final shape of the format will be decided in a bpftrace review,
> > but in any case I think I'll need some way to get these bits:
> > <args> <return type>
> >
>
> Ok, I think for your use case it's better for you to implement it
> customly, I don't think this fits btf_dump() C output as is. But you
> have all the right high-level APIs anyways. There is nothing irregular
> about function declarations, thankfully. Pointers to functions are way
> more involved, syntactically, which is already abstracted from you in
> btf_dump__emit_type_decl(). Here's the code:
>
> static int dump_funcs(const struct btf *btf, struct btf_dump *d)
> {
> int err = 0, i, j, cnt = btf__get_nr_types(btf);
> const struct btf_type *t;
> const struct btf_param *p;
> const char *name;
>
> for (i = 1; i <= cnt; i++) {
> t = btf__type_by_id(btf, i);
> if (!btf_is_func(t))
> continue;
>
> name = btf__name_by_offset(btf, t->name_off);
> t = btf__type_by_id(btf, t->type);
> if (!btf_is_func_proto(t))
> return -EINVAL;
>
> printf("kfunc:%s(", name);
> for (j = 0, p = btf_params(t); j < btf_vlen(t); j++, p++) {
> err = btf_dump__emit_type_decl(d, p->type, NULL);
> if (err)
> return err;
> }
> printf(") = ");
>
> err = btf_dump__emit_type_decl(d, t->type, NULL);
> if (err)
> return err;
aaah right, we could move it one level down ;-) ok, that will do
>
> printf(";\n");
> }
> return 0;
> }
>
> Beware, this will crash right now due to NULL field_name, but I'm
> fixing that with a tiny patch in just a second.
>
> Also beware, there are no argument names captures for func_protos...
>
> So with the above (and btf_dump__emit_type_decl() fix for NULL
> field_name), this will produce output:
>
> kfunc:num_digits(int) = int;
> kfunc:copy_from_user_nmi(void *const void *long unsigned int) = long
> unsigned int;
> kfunc:arch_wb_cache_pmem(void *size_t) = void;
> kfunc:__clear_user(void *long unsigned int) = long unsigned int;
thanks, I'll use that
jirka
