From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966327AbcAZO66 (ORCPT ); Tue, 26 Jan 2016 09:58:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59637 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965184AbcAZO6z (ORCPT ); Tue, 26 Jan 2016 09:58:55 -0500 Date: Tue, 26 Jan 2016 12:58:50 -0200 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: Alexei Starovoitov , acme@kernel.org, Brendan Gregg , Daniel Borkmann , "David S. Miller" , He Kuang , Jiri Olsa , Li Zefan , Masami Hiramatsu , Namhyung Kim , Peter Zijlstra , pi3orama@163.com, Will Deacon , linux-kernel@vger.kernel.org Subject: Re: [PATCH 01/54] perf test: Add libbpf relocation checker Message-ID: <20160126145850.GA3493@redhat.com> References: <1453715801-7732-1-git-send-email-wangnan0@huawei.com> <1453715801-7732-2-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1453715801-7732-2-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.20 (2009-12-10) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Alexei Starovoitov > Cc: Arnaldo Carvalho de Melo > Cc: Daniel Borkmann > Cc: Li Zefan > Cc: pi3orama@163.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 ' > $@ > + $(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