[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160126145850.GA3493@redhat.com>
Date: Tue, 26 Jan 2016 12:58:50 -0200
From: Arnaldo Carvalho de Melo <acme@...hat.com>
To: Wang Nan <wangnan0@...wei.com>
Cc: Alexei Starovoitov <ast@...nel.org>, acme@...nel.org,
Brendan Gregg <brendan.d.gregg@...il.com>,
Daniel Borkmann <daniel@...earbox.net>,
"David S. Miller" <davem@...emloft.net>,
He Kuang <hekuang@...wei.com>, Jiri Olsa <jolsa@...nel.org>,
Li Zefan <lizefan@...wei.com>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, pi3orama@....com,
Will Deacon <will.deacon@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/54] perf test: Add libbpf relocation checker
Em Mon, Jan 25, 2016 at 09:55:48AM +0000, Wang Nan escreveu:
> There's a bug in LLVM that it can generate unneeded relocation
> information. See [1] and [2]. Libbpf should check the target section
> of a relocation symbol.
>
> This patch adds a testcase which reference a global variable (BPF
> doesn't support global variable). Before fixing libbpf, the new test
> case can be loaded into kernel, the global variable acts like the first
> map. It is incorrect.
>
> Result:
> # ~/perf test BPF
> 37: Test BPF filter :
> 37.1: Test basic BPF filtering : Ok
> 37.2: Test BPF prologue generation : Ok
> 37.3: Test BPF relocation checker : FAILED!
>
> # ~/perf test -v BPF
> ...
> libbpf: loading object '[bpf_relocation_test]' from buffer
> libbpf: section .strtab, size 126, link 0, flags 0, type=3
> libbpf: section .text, size 0, link 0, flags 6, type=1
> libbpf: section .data, size 0, link 0, flags 3, type=1
> libbpf: section .bss, size 0, link 0, flags 3, type=8
> libbpf: section func=sys_write, size 104, link 0, flags 6, type=1
> libbpf: found program func=sys_write
> libbpf: section .relfunc=sys_write, size 16, link 10, flags 0, type=9
> libbpf: section maps, size 16, link 0, flags 3, type=1
> libbpf: maps in [bpf_relocation_test]: 16 bytes
> libbpf: section license, size 4, link 0, flags 3, type=1
> libbpf: license of [bpf_relocation_test] is GPL
> libbpf: section version, size 4, link 0, flags 3, type=1
> libbpf: kernel version of [bpf_relocation_test] is 40400
> libbpf: section .symtab, size 144, link 1, flags 0, type=2
> libbpf: map 0 is "my_table"
> libbpf: collecting relocating info for: 'func=sys_write'
> libbpf: relocation: insn_idx=7
> Success unexpectedly: libbpf error when dealing with relocation
"Success unexpectedly?" Reading the code to try to grok this message...
- Arnaldo
> test child finished with -1
> ---- end ----
> Test BPF filter subtest 2: FAILED!
>
> [1] https://llvm.org/bugs/show_bug.cgi?id=26243
> [2] https://patchwork.ozlabs.org/patch/571385/
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: Li Zefan <lizefan@...wei.com>
> Cc: pi3orama@....com
> ---
> tools/perf/Makefile.perf | 2 +-
> tools/perf/tests/.gitignore | 1 +
> tools/perf/tests/Build | 9 ++++-
> tools/perf/tests/bpf-script-test-relocation.c | 50 +++++++++++++++++++++++++++
> tools/perf/tests/bpf.c | 26 +++++++++++---
> tools/perf/tests/llvm.c | 17 ++++++---
> tools/perf/tests/llvm.h | 5 ++-
> 7 files changed, 98 insertions(+), 12 deletions(-)
> create mode 100644 tools/perf/tests/bpf-script-test-relocation.c
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 5d34815..97ce869 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -618,7 +618,7 @@ clean: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean
> $(call QUIET_CLEAN, core-progs) $(RM) $(ALL_PROGRAMS) perf perf-read-vdso32 perf-read-vdsox32
> $(call QUIET_CLEAN, core-gen) $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)FEATURE-DUMP $(OUTPUT)util/*-bison* $(OUTPUT)util/*-flex* \
> $(OUTPUT)util/intel-pt-decoder/inat-tables.c $(OUTPUT)fixdep \
> - $(OUTPUT)tests/llvm-src-{base,kbuild,prologue}.c
> + $(OUTPUT)tests/llvm-src-{base,kbuild,prologue,relocation}.c
> $(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) clean
> $(python-clean)
>
> diff --git a/tools/perf/tests/.gitignore b/tools/perf/tests/.gitignore
> index bf016c4..8cc30e7 100644
> --- a/tools/perf/tests/.gitignore
> +++ b/tools/perf/tests/.gitignore
> @@ -1,3 +1,4 @@
> llvm-src-base.c
> llvm-src-kbuild.c
> llvm-src-prologue.c
> +llvm-src-relocation.c
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 614899b..1ba628e 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -31,7 +31,7 @@ perf-y += sample-parsing.o
> perf-y += parse-no-sample-id-all.o
> perf-y += kmod-path.o
> perf-y += thread-map.o
> -perf-y += llvm.o llvm-src-base.o llvm-src-kbuild.o llvm-src-prologue.o
> +perf-y += llvm.o llvm-src-base.o llvm-src-kbuild.o llvm-src-prologue.o llvm-src-relocation.o
> perf-y += bpf.o
> perf-y += topology.o
> perf-y += cpumap.o
> @@ -59,6 +59,13 @@ $(OUTPUT)tests/llvm-src-prologue.c: tests/bpf-script-test-prologue.c tests/Build
> $(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
> $(Q)echo ';' >> $@
>
> +$(OUTPUT)tests/llvm-src-relocation.c: tests/bpf-script-test-relocation.c tests/Build
> + $(call rule_mkdir)
> + $(Q)echo '#include <tests/llvm.h>' > $@
> + $(Q)echo 'const char test_llvm__bpf_test_relocation[] =' >> $@
> + $(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
> + $(Q)echo ';' >> $@
> +
> ifeq ($(ARCH),$(filter $(ARCH),x86 arm arm64))
> perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
> endif
> diff --git a/tools/perf/tests/bpf-script-test-relocation.c b/tools/perf/tests/bpf-script-test-relocation.c
> new file mode 100644
> index 0000000..93af774
> --- /dev/null
> +++ b/tools/perf/tests/bpf-script-test-relocation.c
> @@ -0,0 +1,50 @@
> +/*
> + * bpf-script-test-relocation.c
> + * Test BPF loader checking relocation
> + */
> +#ifndef LINUX_VERSION_CODE
> +# error Need LINUX_VERSION_CODE
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
> +#endif
> +#define BPF_ANY 0
> +#define BPF_MAP_TYPE_ARRAY 2
> +#define BPF_FUNC_map_lookup_elem 1
> +#define BPF_FUNC_map_update_elem 2
> +
> +static void *(*bpf_map_lookup_elem)(void *map, void *key) =
> + (void *) BPF_FUNC_map_lookup_elem;
> +static void *(*bpf_map_update_elem)(void *map, void *key, void *value, int flags) =
> + (void *) BPF_FUNC_map_update_elem;
> +
> +struct bpf_map_def {
> + unsigned int type;
> + unsigned int key_size;
> + unsigned int value_size;
> + unsigned int max_entries;
> +};
> +
> +#define SEC(NAME) __attribute__((section(NAME), used))
> +struct bpf_map_def SEC("maps") my_table = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(int),
> + .value_size = sizeof(int),
> + .max_entries = 1,
> +};
> +
> +int this_is_a_global_val;
> +
> +SEC("func=sys_write")
> +int bpf_func__sys_write(void *ctx)
> +{
> + int key = 0;
> + int value = 0;
> +
> + /*
> + * Incorrect relocation. Should not allow this program be
> + * loaded into kernel.
> + */
> + bpf_map_update_elem(&this_is_a_global_val, &key, &value, 0);
> + return 0;
> +}
> +char _license[] SEC("license") = "GPL";
> +int _version SEC("version") = LINUX_VERSION_CODE;
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 33689a0..952ca99 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -71,6 +71,15 @@ static struct {
> (NR_ITERS + 1) / 4,
> },
> #endif
> + {
> + LLVM_TESTCASE_BPF_RELOCATION,
> + "Test BPF relocation checker",
> + "[bpf_relocation_test]",
> + "fix 'perf test LLVM' first",
> + "libbpf error when dealing with relocation",
> + NULL,
> + 0,
> + },
> };
>
> static int do_test(struct bpf_object *obj, int (*func)(void),
> @@ -190,7 +199,7 @@ static int __test__bpf(int idx)
>
> ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
> bpf_testcase_table[idx].prog_id,
> - true);
> + true, NULL);
> if (ret != TEST_OK || !obj_buf || !obj_buf_sz) {
> pr_debug("Unable to get BPF object, %s\n",
> bpf_testcase_table[idx].msg_compile_fail);
> @@ -202,14 +211,21 @@ static int __test__bpf(int idx)
>
> obj = prepare_bpf(obj_buf, obj_buf_sz,
> bpf_testcase_table[idx].name);
> - if (!obj) {
> + if ((!!bpf_testcase_table[idx].target_func) != (!!obj)) {
> + if (!obj)
> + pr_debug("Fail to load BPF object: %s\n",
> + bpf_testcase_table[idx].msg_load_fail);
> + else
> + pr_debug("Success unexpectedly: %s\n",
> + bpf_testcase_table[idx].msg_load_fail);
> ret = TEST_FAIL;
> goto out;
> }
>
> - ret = do_test(obj,
> - bpf_testcase_table[idx].target_func,
> - bpf_testcase_table[idx].expect_result);
> + if (obj)
> + ret = do_test(obj,
> + bpf_testcase_table[idx].target_func,
> + bpf_testcase_table[idx].expect_result);
> out:
> bpf__clear();
> return ret;
> diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
> index 06f45c1..70edcdf 100644
> --- a/tools/perf/tests/llvm.c
> +++ b/tools/perf/tests/llvm.c
> @@ -35,6 +35,7 @@ static int test__bpf_parsing(void *obj_buf __maybe_unused,
> static struct {
> const char *source;
> const char *desc;
> + bool should_load_fail;
> } bpf_source_table[__LLVM_TESTCASE_MAX] = {
> [LLVM_TESTCASE_BASE] = {
> .source = test_llvm__bpf_base_prog,
> @@ -48,14 +49,19 @@ static struct {
> .source = test_llvm__bpf_test_prologue_prog,
> .desc = "Compile source for BPF prologue generation test",
> },
> + [LLVM_TESTCASE_BPF_RELOCATION] = {
> + .source = test_llvm__bpf_test_relocation,
> + .desc = "Compile source for BPF relocation test",
> + .should_load_fail = true,
> + },
> };
>
> -
> int
> test_llvm__fetch_bpf_obj(void **p_obj_buf,
> size_t *p_obj_buf_sz,
> enum test_llvm__testcase idx,
> - bool force)
> + bool force,
> + bool *should_load_fail)
> {
> const char *source;
> const char *desc;
> @@ -68,6 +74,8 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf,
>
> source = bpf_source_table[idx].source;
> desc = bpf_source_table[idx].desc;
> + if (should_load_fail)
> + *should_load_fail = bpf_source_table[idx].should_load_fail;
>
> perf_config(perf_config_cb, NULL);
>
> @@ -136,14 +144,15 @@ int test__llvm(int subtest)
> int ret;
> void *obj_buf = NULL;
> size_t obj_buf_sz = 0;
> + bool should_load_fail = false;
>
> if ((subtest < 0) || (subtest >= __LLVM_TESTCASE_MAX))
> return TEST_FAIL;
>
> ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
> - subtest, false);
> + subtest, false, &should_load_fail);
>
> - if (ret == TEST_OK) {
> + if (ret == TEST_OK && !should_load_fail) {
> ret = test__bpf_parsing(obj_buf, obj_buf_sz);
> if (ret != TEST_OK) {
> pr_debug("Failed to parse test case '%s'\n",
> diff --git a/tools/perf/tests/llvm.h b/tools/perf/tests/llvm.h
> index 5150b4d..0eaa604 100644
> --- a/tools/perf/tests/llvm.h
> +++ b/tools/perf/tests/llvm.h
> @@ -7,14 +7,17 @@
> extern const char test_llvm__bpf_base_prog[];
> extern const char test_llvm__bpf_test_kbuild_prog[];
> extern const char test_llvm__bpf_test_prologue_prog[];
> +extern const char test_llvm__bpf_test_relocation[];
>
> enum test_llvm__testcase {
> LLVM_TESTCASE_BASE,
> LLVM_TESTCASE_KBUILD,
> LLVM_TESTCASE_BPF_PROLOGUE,
> + LLVM_TESTCASE_BPF_RELOCATION,
> __LLVM_TESTCASE_MAX,
> };
>
> int test_llvm__fetch_bpf_obj(void **p_obj_buf, size_t *p_obj_buf_sz,
> - enum test_llvm__testcase index, bool force);
> + enum test_llvm__testcase index, bool force,
> + bool *should_load_fail);
> #endif
> --
> 1.8.3.4
Powered by blists - more mailing lists