Skip to content

Commit

Permalink
bpftool: Remove asserts from JIT disassembler
Browse files Browse the repository at this point in the history
The JIT disassembler in bpftool is the only components (with the JSON
writer) using asserts to check the return values of functions. But it
does not do so in a consistent way, and diasm_print_insn() returns no
value, although sometimes the operation failed.

Remove the asserts, and instead check the return values, print messages
on errors, and propagate the error to the caller from prog.c.

Remove the inclusion of assert.h from jit_disasm.c, and also from map.c
where it is unused.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Tested-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Acked-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20221025150329.97371-3-quentin@isovalent.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
qmonnet authored and Alexei Starovoitov committed Oct 25, 2022
1 parent b3d84af commit 55b4de5
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 35 deletions.
51 changes: 35 additions & 16 deletions tools/bpf/bpftool/jit_disasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <stdarg.h>
#include <stdint.h>
#include <stdlib.h>
#include <assert.h>
#include <unistd.h>
#include <string.h>
#include <bfd.h>
Expand All @@ -31,14 +30,18 @@
#include "json_writer.h"
#include "main.h"

static void get_exec_path(char *tpath, size_t size)
static int get_exec_path(char *tpath, size_t size)
{
const char *path = "/proc/self/exe";
ssize_t len;

len = readlink(path, tpath, size - 1);
assert(len > 0);
if (len <= 0)
return -1;

tpath[len] = 0;

return 0;
}

static int oper_count;
Expand Down Expand Up @@ -99,30 +102,39 @@ static int fprintf_json_styled(void *out,
return r;
}

void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options,
const struct btf *btf,
const struct bpf_prog_linfo *prog_linfo,
__u64 func_ksym, unsigned int func_idx,
bool linum)
int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options,
const struct btf *btf,
const struct bpf_prog_linfo *prog_linfo,
__u64 func_ksym, unsigned int func_idx,
bool linum)
{
const struct bpf_line_info *linfo = NULL;
disassembler_ftype disassemble;
int count, i, pc = 0, err = -1;
struct disassemble_info info;
unsigned int nr_skip = 0;
int count, i, pc = 0;
char tpath[PATH_MAX];
bfd *bfdf;

if (!len)
return;
return -1;

memset(tpath, 0, sizeof(tpath));
get_exec_path(tpath, sizeof(tpath));
if (get_exec_path(tpath, sizeof(tpath))) {
p_err("failed to create disasembler (get_exec_path)");
return -1;
}

bfdf = bfd_openr(tpath, NULL);
assert(bfdf);
assert(bfd_check_format(bfdf, bfd_object));
if (!bfdf) {
p_err("failed to create disassembler (bfd_openr)");
return -1;
}
if (!bfd_check_format(bfdf, bfd_object)) {
p_err("failed to create disassembler (bfd_check_format)");
goto exit_close;
}

if (json_output)
init_disassemble_info_compat(&info, stdout,
Expand All @@ -141,7 +153,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
bfdf->arch_info = inf;
} else {
p_err("No libbfd support for %s", arch);
return;
goto exit_close;
}
}

Expand All @@ -162,7 +174,10 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
#else
disassemble = disassembler(bfdf);
#endif
assert(disassemble);
if (!disassemble) {
p_err("failed to create disassembler");
goto exit_close;
}

if (json_output)
jsonw_start_array(json_wtr);
Expand Down Expand Up @@ -226,7 +241,11 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
if (json_output)
jsonw_end_array(json_wtr);

err = 0;

exit_close:
bfd_close(bfdf);
return err;
}

int disasm_init(void)
Expand Down
25 changes: 13 additions & 12 deletions tools/bpf/bpftool/main.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,22 +173,23 @@ int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);

struct bpf_prog_linfo;
#ifdef HAVE_LIBBFD_SUPPORT
void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options,
const struct btf *btf,
const struct bpf_prog_linfo *prog_linfo,
__u64 func_ksym, unsigned int func_idx,
bool linum);
int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options,
const struct btf *btf,
const struct bpf_prog_linfo *prog_linfo,
__u64 func_ksym, unsigned int func_idx,
bool linum);
int disasm_init(void);
#else
static inline
void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options,
const struct btf *btf,
const struct bpf_prog_linfo *prog_linfo,
__u64 func_ksym, unsigned int func_idx,
bool linum)
int disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options,
const struct btf *btf,
const struct bpf_prog_linfo *prog_linfo,
__u64 func_ksym, unsigned int func_idx,
bool linum)
{
return 0;
}
static inline int disasm_init(void)
{
Expand Down
1 change: 0 additions & 1 deletion tools/bpf/bpftool/map.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
/* Copyright (C) 2017-2018 Netronome Systems, Inc. */

#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <linux/err.h>
Expand Down
15 changes: 9 additions & 6 deletions tools/bpf/bpftool/prog.c
Original file line number Diff line number Diff line change
Expand Up @@ -822,10 +822,11 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode,
printf("%s:\n", sym_name);
}

disasm_print_insn(img, lens[i], opcodes,
name, disasm_opt, btf,
prog_linfo, ksyms[i], i,
linum);
if (disasm_print_insn(img, lens[i], opcodes,
name, disasm_opt, btf,
prog_linfo, ksyms[i], i,
linum))
goto exit_free;

img += lens[i];

Expand All @@ -838,8 +839,10 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode,
if (json_output)
jsonw_end_array(json_wtr);
} else {
disasm_print_insn(buf, member_len, opcodes, name,
disasm_opt, btf, NULL, 0, 0, false);
if (disasm_print_insn(buf, member_len, opcodes, name,
disasm_opt, btf, NULL, 0, 0,
false))
goto exit_free;
}
} else if (visual) {
if (json_output)
Expand Down

0 comments on commit 55b4de5

Please sign in to comment.