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 <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> Cc: Li Zefan <[email protected]>
> Cc: [email protected]
> ---
>  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

Reply via email to