On Fri, Feb 15, 2019 at 01:53:53PM -0800, Song Liu wrote:

SNIP

> +static int symbol__disassemble_bpf(struct symbol *sym,
> +                                struct annotate_args *args)
> +{
> +     struct annotation *notes = symbol__annotation(sym);
> +     struct annotation_options *opts = args->options;
> +     struct bpf_prog_info_linear *info_linear;
> +     struct bpf_prog_linfo *prog_linfo = NULL;
> +     struct bpf_prog_info_node *info_node;
> +     int len = sym->end - sym->start;
> +     disassembler_ftype disassemble;
> +     struct map *map = args->ms.map;
> +     struct disassemble_info info;
> +     struct dso *dso = map->dso;
> +     int pc = 0, count, sub_id;
> +     struct btf *btf = NULL;
> +     char tpath[PATH_MAX];
> +     size_t buf_size;
> +     int nr_skip = 0;
> +     __u64 arrays;
> +     char *buf;
> +     bfd *bfdf;
> +     FILE *s;
> +
> +     if (dso->binary_type != DSO_BINARY_TYPE__BPF_PROG_INFO)
> +             return -1;
> +
> +     pr_debug("%s: handling sym %s addr %lx len %lx\n", __func__,
> +              sym->name, sym->start, sym->end - sym->start);
> +
> +     memset(tpath, 0, sizeof(tpath));
> +     get_exec_path(tpath, sizeof(tpath));
> +
> +     bfdf = bfd_openr(tpath, NULL);
> +     assert(bfdf);
> +     assert(bfd_check_format(bfdf, bfd_object));
> +
> +     s = open_memstream(&buf, &buf_size);

what if open_memstream fails?


> +     init_disassemble_info(&info, s,
> +                           (fprintf_ftype) fprintf);
> +
> +     info.arch = bfd_get_arch(bfdf);
> +     info.mach = bfd_get_mach(bfdf);
> +
> +     arrays = 1UL << BPF_PROG_INFO_JITED_INSNS;
> +     arrays |= 1UL << BPF_PROG_INFO_JITED_KSYMS;
> +     arrays |= 1UL << BPF_PROG_INFO_LINE_INFO;
> +     arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;

what's the arrays for?

> +
> +     info_node = perf_env__find_bpf_prog_info(dso->bpf_prog.env,
> +                                              dso->bpf_prog.id);
> +     if (!info_node)
> +             return -1;
> +     info_linear = info_node->info_linear;
> +     sub_id = dso->bpf_prog.sub_id;
> +
> +     info.buffer = (void *)(info_linear->info.jited_prog_insns);
> +     info.buffer_length = info_linear->info.jited_prog_len;
> +
> +     if (info_linear->info.nr_line_info)
> +             prog_linfo = bpf_prog_linfo__new(&info_linear->info);
> +     prog_linfo = prog_linfo;
> +
> +     if (info_linear->info.btf_id) {
> +             struct btf_node *node;
> +
> +             node = perf_env__find_btf(dso->bpf_prog.env,
> +                                       info_linear->info.btf_id);
> +             if (node)
> +                     btf = btf__new((__u8 *)(node->data),
> +                                    node->data_size);
> +     }

what if btf__new fails? the btf__name_by_offset
does not check btf != NULL

> +
> +     disassemble_init_for_target(&info);
> +
> +#ifdef DISASM_FOUR_ARGS_SIGNATURE
> +     disassemble = disassembler(info.arch,
> +                                bfd_big_endian(bfdf),
> +                                info.mach,
> +                                bfdf);
> +#else
> +     disassemble = disassembler(bfdf);
> +#endif
> +     assert(disassemble);
> +
> +     fflush(s);

any chance this function could be split into some logical
pieces/fucntions?

thanks,
jirka

> +     do {
> +             const struct bpf_line_info *linfo = NULL;
> +             struct disasm_line *dl;
> +             size_t prev_buf_size;
> +             const char *srcline;
> +             u64 addr;
> +
> +             addr = pc + ((u64 *)(info_linear->info.jited_ksyms))[sub_id];
> +             count = disassemble(pc, &info);
> +
> +             linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo, addr, 
> sub_id,
> +                                                     nr_skip);
> +
> +             if (linfo) {
> +                     srcline = btf__name_by_offset(btf, linfo->line_off);
> +                     nr_skip++;
> +             } else
> +                     srcline = NULL;
> +
> +             fprintf(s, "\n");
> +             prev_buf_size = buf_size;
> +             fflush(s);
> +
> +             if (!opts->hide_src_code && srcline) {
> +                     args->offset = -1;
> +                     args->line = strdup(srcline);
> +                     args->line_nr = 0;
> +                     args->ms.sym  = sym;
> +                     dl = disasm_line__new(args);
> +                     annotation_line__add(&dl->al, &notes->src->source);
> +             }
> +
> +             args->offset = pc;
> +             args->line = buf + prev_buf_size;
> +             args->line_nr = 0;
> +             args->ms.sym  = sym;
> +             dl = disasm_line__new(args);
> +             annotation_line__add(&dl->al, &notes->src->source);
> +
> +             pc += count;
> +     } while (count > 0 && pc < len);
> +
> +     bfd_close(bfdf);
> +     return 0;
> +}

SNIP

Reply via email to