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.
Signed-off-by: Namhyung Kim <[email protected]>
---
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
On Fri, Apr 15, 2022 at 8:40 PM Namhyung Kim <[email protected]> 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/[email protected]/
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 <[email protected]>
> ---
> 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
>
Hi Ian,
On Sat, Apr 16, 2022 at 7:59 AM Ian Rogers <[email protected]> wrote:
>
> On Fri, Apr 15, 2022 at 8:40 PM Namhyung Kim <[email protected]> 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/[email protected]/
> 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?
Yeah, IIUC the whole point of the symbol end fixup is because the
kallsyms doesn't have the symbol size info. Every ELF binaries
should have the size except for some hand-written asm functions
which missed adding it manually. I guess that's the reason it was
added to other DSO loading paths.
Also considering "[" (and "]") in the symbol name is specific to
kallsyms which has both kernel and module symbols together.
Thanks,
Namhyung