linux-kernel - Re: [PATCH 1/3] perf symbol: Pass is_kallsyms to symbols__fixup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fX5S2CR58REzAhXH4s8_27=D2LbKksS7mHoSoxYOOmN4g@mail.gmail.com>
Date:   Sat, 16 Apr 2022 10:59:27 -0400
From:   Ian Rogers <irogers@...gle.com>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Michael Petlan <mpetlan@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
        Song Liu <songliubraving@...com>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        John Garry <john.garry@...wei.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Leo Yan <leo.yan@...aro.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Heiko Carstens <hca@...ux.ibm.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-perf-users@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-s390@...r.kernel.org
Subject: Re: [PATCH 1/3] perf symbol: Pass is_kallsyms to symbols__fixup_end()

On Fri, Apr 15, 2022 at 8:40 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> The symbol fixup is necessary for symbols in kallsyms since they don't
> have size info.  So we use the next symbol's address to calculate the
> size.  Now it's also used for user binaries because sometimes they
> miss size for hand-written asm functions.
>
> There's a arch-specific function to handle kallsyms differently but
> currently it cannot distinguish kallsyms from others.  Pass this
> information explicitly to handle it properly.  Note that those arch
> functions will be moved to the generic function so I didn't added it
> to the arch-functions.

Thanks Namhyung, in:
https://lore.kernel.org/linux-perf-users/20220412154817.2728324-3-irogers@google.com/
I used "dso->kernel != DSO_SPACE__USER" in symbol-elf to make this
more than just kallsyms as presumably kernel code is the issue. Do we
know elf kernel code has correctly sized symbols?

Thanks,
Ian

> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/symbol-elf.c | 2 +-
>  tools/perf/util/symbol.c     | 7 ++++---
>  tools/perf/util/symbol.h     | 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 31cd59a2b66e..ecd377938eea 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1290,7 +1290,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>          * For misannotated, zeroed, ASM function sizes.
>          */
>         if (nr > 0) {
> -               symbols__fixup_end(&dso->symbols);
> +               symbols__fixup_end(&dso->symbols, false);
>                 symbols__fixup_duplicate(&dso->symbols);
>                 if (kmap) {
>                         /*
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index dea0fc495185..1b85cc1422a9 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -217,7 +217,8 @@ void symbols__fixup_duplicate(struct rb_root_cached *symbols)
>         }
>  }
>
> -void symbols__fixup_end(struct rb_root_cached *symbols)
> +void symbols__fixup_end(struct rb_root_cached *symbols,
> +                       bool is_kallsyms __maybe_unused)
>  {
>         struct rb_node *nd, *prevnd = rb_first_cached(symbols);
>         struct symbol *curr, *prev;
> @@ -1467,7 +1468,7 @@ int __dso__load_kallsyms(struct dso *dso, const char *filename,
>         if (kallsyms__delta(kmap, filename, &delta))
>                 return -1;
>
> -       symbols__fixup_end(&dso->symbols);
> +       symbols__fixup_end(&dso->symbols, true);
>         symbols__fixup_duplicate(&dso->symbols);
>
>         if (dso->kernel == DSO_SPACE__KERNEL_GUEST)
> @@ -1659,7 +1660,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
>  #undef bfd_asymbol_section
>  #endif
>
> -       symbols__fixup_end(&dso->symbols);
> +       symbols__fixup_end(&dso->symbols, false);
>         symbols__fixup_duplicate(&dso->symbols);
>         dso->adjust_symbols = 1;
>
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index fbf866d82dcc..5fcdd1f94c56 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -203,7 +203,7 @@ void __symbols__insert(struct rb_root_cached *symbols, struct symbol *sym,
>                        bool kernel);
>  void symbols__insert(struct rb_root_cached *symbols, struct symbol *sym);
>  void symbols__fixup_duplicate(struct rb_root_cached *symbols);
> -void symbols__fixup_end(struct rb_root_cached *symbols);
> +void symbols__fixup_end(struct rb_root_cached *symbols, bool is_kallsyms);
>  void maps__fixup_end(struct maps *maps);
>
>  typedef int (*mapfn_t)(u64 start, u64 len, u64 pgoff, void *data);
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>