bpftool: Remove asserts from JIT disassembler

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>

diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 71cb258ab0eed3c51f75b20f6240c7ed5b5ea000..723a9b799a0c3758a1de0fe9b97b8a9cbc9680b5 100644 (file)
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -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>
@@ -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;
@@ -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,
@@ -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;
                }
        }
 
@@ -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);
@@ -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)

diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 5e5060c2ac047ba6a5ee01a375d7bc00dd437f8d..c9e171082cf61094b8413b599781bc09db36ffd5 100644 (file)
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -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)
 {

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 9a6ca9f311338ddffa25278b1c8d87838068253e..3087ced658adc1fcec4395c0280912ab83a1a316 100644 (file)
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -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>

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index dbf5489b8fde259bb33612c30f66ddd099e7a845..93dfb89b85e38e85f5701d18761db736b7cad0f4 100644 (file)
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -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];
 
@@ -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)