From: Quentin Monnet <quentin@isovalent.com>
To: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>
Cc: "Martin KaFai Lau" <kafai@fb.com>,
"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
"John Fastabend" <john.fastabend@gmail.com>,
"KP Singh" <kpsingh@kernel.org>,
"Stanislav Fomichev" <sdf@google.com>,
"Hao Luo" <haoluo@google.com>, "Jiri Olsa" <jolsa@kernel.org>,
bpf@vger.kernel.org,
"Niklas Söderlund" <niklas.soderlund@corigine.com>,
"Simon Horman" <simon.horman@corigine.com>,
"Quentin Monnet" <quentin@isovalent.com>,
"Song Liu" <song@kernel.org>
Subject: [PATCH bpf-next v4 2/8] bpftool: Remove asserts from JIT disassembler
Date: Tue, 25 Oct 2022 16:03:23 +0100 [thread overview]
Message-ID: <20221025150329.97371-3-quentin@isovalent.com> (raw)
In-Reply-To: <20221025150329.97371-1-quentin@isovalent.com>
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>
---
tools/bpf/bpftool/jit_disasm.c | 51 +++++++++++++++++++++++-----------
tools/bpf/bpftool/main.h | 25 +++++++++--------
tools/bpf/bpftool/map.c | 1 -
tools/bpf/bpftool/prog.c | 15 ++++++----
4 files changed, 57 insertions(+), 35 deletions(-)
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 71cb258ab0ee..723a9b799a0c 100644
--- 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 5e5060c2ac04..c9e171082cf6 100644
--- 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 9a6ca9f31133..3087ced658ad 100644
--- 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 dbf5489b8fde..93dfb89b85e3 100644
--- 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)
--
2.34.1
next prev parent reply other threads:[~2022-10-25 15:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-25 15:03 [PATCH bpf-next v4 0/8] bpftool: Add LLVM as default library for disassembling JIT-ed programs Quentin Monnet
2022-10-25 15:03 ` [PATCH bpf-next v4 1/8] bpftool: Define _GNU_SOURCE only once Quentin Monnet
2022-10-25 15:03 ` Quentin Monnet [this message]
2022-10-25 15:03 ` [PATCH bpf-next v4 3/8] bpftool: Split FEATURE_TESTS/FEATURE_DISPLAY definitions in Makefile Quentin Monnet
2022-10-25 15:03 ` [PATCH bpf-next v4 4/8] bpftool: Group libbfd defs in Makefile, only pass them if we use libbfd Quentin Monnet
2022-10-25 15:03 ` [PATCH bpf-next v4 5/8] bpftool: Refactor disassembler for JIT-ed programs Quentin Monnet
2022-10-25 15:03 ` [PATCH bpf-next v4 6/8] bpftool: Add LLVM as default library for disassembling " Quentin Monnet
2022-10-25 15:03 ` [PATCH bpf-next v4 7/8] bpftool: Support setting alternative arch for JIT disasm with LLVM Quentin Monnet
2022-10-25 15:03 ` [PATCH bpf-next v4 8/8] bpftool: Add llvm feature to "bpftool version" Quentin Monnet
2022-10-25 17:20 ` [PATCH bpf-next v4 0/8] bpftool: Add LLVM as default library for disassembling JIT-ed programs patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221025150329.97371-3-quentin@isovalent.com \
--to=quentin@isovalent.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=niklas.soderlund@corigine.com \
--cc=sdf@google.com \
--cc=simon.horman@corigine.com \
--cc=song@kernel.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox