[PATCH 0/2] perf: support specify vdso path in cmdline

[PATCH 0/2] perf: support specify vdso path in cmdline

[Changbin Du]

The vdso dumped from process memory (in buildid-cache) lacks debugging
info. To annotate vdso symbols with source lines we need specify a
debugging version.

For x86, we can find them from your local build as
arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may resides in
/lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that the
builid has to match.

$ sudo perf record -a
$ sudo perf report --objdump=llvm-objdump \
      --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg

Samples: 17K of event 'cycles:P', 4000 Hz, Event count (approx.): 1760
__vdso_clock_gettime  /work/linux-host/arch/x86/entry/vdso/vdso64.so.d
Percent│       movq    -48(%rbp),%rsi                                ▒
       │       testq   %rax,%rax                                     ▒
       │     ;               return vread_hvclock();                 ▒
       │       movq    %rax,%rdx                                     ▒
       │     ;               if (unlikely(!vdso_cycles_ok(cycles)))  ▒
       │     ↑ js      eb                                            ▒
       │     ↑ jmp     74                                            ▒
       │     ;               ts->tv_sec = vdso_ts->sec;              ▒
  0.02 │147:   leaq    2(%rbx),%rax                                  ▒
       │       shlq    $4, %rax                                      ▒
       │       addq    %r10,%rax                                     ▒
       │     ;               while ((seq = READ_ONCE(vd->seq)) & 1) {▒
  9.38 │152:   movl    (%r10),%ecx                                   ▒


When doing cross platform analysis, we also need specify the vdso path if
we are interested in its symbols.

Changbin Du (2):
  perf: support specify vdso path in cmdline
  perf: disasm: prefer symsrc_filename for filename

 tools/perf/builtin-annotate.c |  2 +
 tools/perf/builtin-c2c.c      |  2 +
 tools/perf/builtin-inject.c   |  2 +
 tools/perf/builtin-report.c   |  2 +
 tools/perf/builtin-script.c   |  2 +
 tools/perf/builtin-top.c      |  2 +
 tools/perf/util/disasm.c      |  5 +++
 tools/perf/util/symbol.c      | 82 ++++++++++++++++++++++++++++++++++-
 tools/perf/util/symbol_conf.h |  5 +++
 9 files changed, 102 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/2] perf: support specify vdso path in cmdline

[Changbin Du]

The vdso dumped from process memory (in buildid-cache) lacks debugging
info. To annotate vdso symbols with source lines we need specify a
debugging version.

For x86, we can find them from your local build as
arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may reside in
/lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that
the buildid has to match.

$ sudo perf record -a
$ sudo perf report --objdump=llvm-objdump \
  --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg

When doing cross platform analysis, we also need specify the vdso path if
we are interested in its symbols.

Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
 tools/perf/builtin-annotate.c |  2 +
 tools/perf/builtin-c2c.c      |  2 +
 tools/perf/builtin-inject.c   |  2 +
 tools/perf/builtin-report.c   |  2 +
 tools/perf/builtin-script.c   |  2 +
 tools/perf/builtin-top.c      |  2 +
 tools/perf/util/symbol.c      | 74 +++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol_conf.h |  5 +++
 8 files changed, 91 insertions(+)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 50d2fb222d48..ff466882065d 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -742,6 +742,8 @@ int cmd_annotate(int argc, const char **argv)
 		   "file", "vmlinux pathname"),
 	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
 		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
+	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
+		     parse_vdso_pathnames),
 	OPT_BOOLEAN('l', "print-line", &annotate_opts.print_lines,
 		    "print matching source lines (may be slow)"),
 	OPT_BOOLEAN('P', "full-paths", &annotate_opts.full_path,
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index c157bd31f2e5..4764f9139661 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -3018,6 +3018,8 @@ static int perf_c2c__report(int argc, const char **argv)
 	const struct option options[] = {
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
+	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
+		     parse_vdso_pathnames),
 	OPT_STRING('i', "input", &input_name, "file",
 		   "the input file to process"),
 	OPT_INCR('N', "node-info", &c2c.node_info,
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index a212678d47be..e774e83d0a0f 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -2247,6 +2247,8 @@ int cmd_inject(int argc, const char **argv)
 			    "don't load vmlinux even if found"),
 		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
 			   "kallsyms pathname"),
+		OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
+		     parse_vdso_pathnames),
 		OPT_BOOLEAN('f', "force", &data.force, "don't complain, do it"),
 		OPT_CALLBACK_OPTARG(0, "itrace", &inject.itrace_synth_opts,
 				    NULL, "opts", "Instruction Tracing options\n"
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 69618fb0110b..a64b48460dce 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1324,6 +1324,8 @@ int cmd_report(int argc, const char **argv)
                     "don't load vmlinux even if found"),
 	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
 		   "file", "kallsyms pathname"),
+	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
+		     parse_vdso_pathnames),
 	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
 	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
 		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index c16224b1fef3..2e358922a8d1 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3965,6 +3965,8 @@ int cmd_script(int argc, const char **argv)
 		   "file", "vmlinux pathname"),
 	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
 		   "file", "kallsyms pathname"),
+	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
+		     parse_vdso_pathnames),
 	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
 		    "When printing symbols do not display call chain"),
 	OPT_CALLBACK(0, "symfs", NULL, "directory",
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 1d6aef51c122..a3cce4e76eb9 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1479,6 +1479,8 @@ int cmd_top(int argc, const char **argv)
 		   "file", "kallsyms pathname"),
 	OPT_BOOLEAN('K', "hide_kernel_symbols", &top.hide_kernel_symbols,
 		    "hide kernel symbols"),
+	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
+		     parse_vdso_pathnames),
 	OPT_CALLBACK('m', "mmap-pages", &opts->mmap_pages, "pages",
 		     "number of mmap data pages", evlist__parse_mmap_pages),
 	OPT_INTEGER('r', "realtime", &top.realtime_prio,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9e5940b5bc59..8d040039a7ce 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -19,6 +19,7 @@
 #include "build-id.h"
 #include "cap.h"
 #include "dso.h"
+#include "vdso.h"
 #include "util.h" // lsdir()
 #include "debug.h"
 #include "event.h"
@@ -44,6 +45,7 @@
 
 static int dso__load_kernel_sym(struct dso *dso, struct map *map);
 static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map);
+static int dso__load_vdso_sym(struct dso *dso, struct map *map);
 static bool symbol__is_idle(const char *name);
 
 int vmlinux_path__nr_entries;
@@ -1833,6 +1835,12 @@ int dso__load(struct dso *dso, struct map *map)
 		goto out;
 	}
 
+	if (dso__is_vdso(dso)) {
+		ret = dso__load_vdso_sym(dso, map);
+		if (ret > 0)
+			goto out;
+	}
+
 	dso__set_adjust_symbols(dso, false);
 
 	if (perfmap) {
@@ -2349,6 +2357,72 @@ static int vmlinux_path__init(struct perf_env *env)
 	return -1;
 }
 
+int parse_vdso_pathnames(const struct option *opt __maybe_unused,
+			 const char *arg, int unset __maybe_unused)
+{
+	char *tmp, *tok, *str = strdup(arg);
+	unsigned int i = 0;
+
+	for (tok = strtok_r(str, ",", &tmp); tok && i < ARRAY_SIZE(symbol_conf.vdso_name);
+	     tok = strtok_r(NULL, ",", &tmp)) {
+		symbol_conf.vdso_name[i++] = strdup(tok);
+	}
+
+	free(str);
+	return 0;
+}
+
+static int dso__load_vdso(struct dso *dso, struct map *map,
+			  const char *vdso)
+{
+	int err = -1;
+	struct symsrc ss;
+	char symfs_vdso[PATH_MAX];
+
+	if (vdso[0] == '/')
+		snprintf(symfs_vdso, sizeof(symfs_vdso), "%s", vdso);
+	else
+		symbol__join_symfs(symfs_vdso, vdso);
+
+	if (symsrc__init(&ss, dso, symfs_vdso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO))
+		return -1;
+
+	/*
+	 * dso__load_sym() may copy 'dso' which will result in the copies having
+	 * an incorrect long name unless we set it here first.
+	 */
+	dso__set_long_name(dso, vdso, false);
+	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
+
+	err = dso__load_sym(dso, map, &ss, &ss, 0);
+	symsrc__destroy(&ss);
+
+	if (err > 0) {
+		dso__set_loaded(dso);
+		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
+	}
+
+	return err;
+}
+
+static int dso__load_vdso_sym(struct dso *dso, struct map *map)
+{
+	int ret;
+
+	if (!dso__is_vdso(dso))
+		return -1;
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(symbol_conf.vdso_name); i++) {
+		if (symbol_conf.vdso_name[i] != NULL) {
+			ret = dso__load_vdso(dso, map, symbol_conf.vdso_name[i]);
+			if (ret > 0)
+				return ret;
+		}
+	}
+
+	return -1;
+}
+
 int setup_list(struct strlist **list, const char *list_str,
 		      const char *list_name)
 {
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index c114bbceef40..108356e3c981 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -3,6 +3,7 @@
 #define __PERF_SYMBOL_CONF 1
 
 #include <stdbool.h>
+#include <subcmd/parse-options.h>
 
 struct strlist;
 struct intlist;
@@ -55,6 +56,7 @@ struct symbol_conf {
 	const char	*default_guest_vmlinux_name,
 			*default_guest_kallsyms,
 			*default_guest_modules;
+	const char	*vdso_name[2];
 	const char	*guestmount;
 	const char	*dso_list_str,
 			*comm_list_str,
@@ -85,4 +87,7 @@ struct symbol_conf {
 
 extern struct symbol_conf symbol_conf;
 
+int parse_vdso_pathnames(const struct option *opt __maybe_unused,
+			 const char *arg, int unset __maybe_unused);
+
 #endif // __PERF_SYMBOL_CONF
-- 
2.34.1

[PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

[Changbin Du]

If we already found a debugging version when loading symbols for that dso,
then use the same file for disasm instead of looking up in buildid-cache.

Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
 tools/perf/util/disasm.c |  5 +++++
 tools/perf/util/symbol.c | 12 ++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 72aec8f61b94..75cfc6fbd11d 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -1103,6 +1103,11 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 	    !dso__is_kcore(dso))
 		return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX;
 
+	if (dso__symsrc_filename(dso)) {
+		strlcpy(filename, dso__symsrc_filename(dso), filename_size);
+		return 0;
+	}
+
 	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
 	if (build_id_filename) {
 		__symbol__join_symfs(filename, filename_size, build_id_filename);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8d040039a7ce..a90c647d37e1 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2025,12 +2025,14 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 		dso__set_binary_type(dso, DSO_BINARY_TYPE__VMLINUX);
 
 	err = dso__load_sym(dso, map, &ss, &ss, 0);
-	symsrc__destroy(&ss);
-
 	if (err > 0) {
 		dso__set_loaded(dso);
 		pr_debug("Using %s for symbols\n", symfs_vmlinux);
+
+		if (symsrc__has_symtab(&ss) && !dso__symsrc_filename(dso))
+			dso__set_symsrc_filename(dso, strdup(symfs_vmlinux));
 	}
+	symsrc__destroy(&ss);
 
 	return err;
 }
@@ -2395,12 +2397,14 @@ static int dso__load_vdso(struct dso *dso, struct map *map,
 	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
 
 	err = dso__load_sym(dso, map, &ss, &ss, 0);
-	symsrc__destroy(&ss);
-
 	if (err > 0) {
 		dso__set_loaded(dso);
 		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
+
+		if (symsrc__has_symtab(&ss) && !dso__symsrc_filename(dso))
+			dso__set_symsrc_filename(dso, strdup(symfs_vdso));
 	}
+	symsrc__destroy(&ss);
 
 	return err;
 }
-- 
2.34.1

Re: [PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

[Adrian Hunter]

On 13/06/24 09:35, Changbin Du wrote:
> If we already found a debugging version when loading symbols for that dso,
> then use the same file for disasm instead of looking up in buildid-cache.

In the past, there have been cases where the debugging version has not
worked for reading object code.  I don't remember the details, but the
symbols and debugging information was OK while the object code was not.

In general, using anything other than the file that was actually executed
for reading object code seems like a bad idea.

> 
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
>  tools/perf/util/disasm.c |  5 +++++
>  tools/perf/util/symbol.c | 12 ++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 72aec8f61b94..75cfc6fbd11d 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -1103,6 +1103,11 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  	    !dso__is_kcore(dso))
>  		return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX;
>  
> +	if (dso__symsrc_filename(dso)) {
> +		strlcpy(filename, dso__symsrc_filename(dso), filename_size);
> +		return 0;
> +	}
> +
>  	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
>  	if (build_id_filename) {
>  		__symbol__join_symfs(filename, filename_size, build_id_filename);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 8d040039a7ce..a90c647d37e1 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2025,12 +2025,14 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
>  		dso__set_binary_type(dso, DSO_BINARY_TYPE__VMLINUX);
>  
>  	err = dso__load_sym(dso, map, &ss, &ss, 0);
> -	symsrc__destroy(&ss);
> -
>  	if (err > 0) {
>  		dso__set_loaded(dso);
>  		pr_debug("Using %s for symbols\n", symfs_vmlinux);
> +
> +		if (symsrc__has_symtab(&ss) && !dso__symsrc_filename(dso))
> +			dso__set_symsrc_filename(dso, strdup(symfs_vmlinux));
>  	}
> +	symsrc__destroy(&ss);
>  
>  	return err;
>  }
> @@ -2395,12 +2397,14 @@ static int dso__load_vdso(struct dso *dso, struct map *map,
>  	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
>  
>  	err = dso__load_sym(dso, map, &ss, &ss, 0);
> -	symsrc__destroy(&ss);
> -
>  	if (err > 0) {
>  		dso__set_loaded(dso);
>  		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
> +
> +		if (symsrc__has_symtab(&ss) && !dso__symsrc_filename(dso))
> +			dso__set_symsrc_filename(dso, strdup(symfs_vdso));
>  	}
> +	symsrc__destroy(&ss);
>  
>  	return err;
>  }

Re: [PATCH 1/2] perf: support specify vdso path in cmdline

[Adrian Hunter]

On 13/06/24 09:35, Changbin Du wrote:
> The vdso dumped from process memory (in buildid-cache) lacks debugging
> info. To annotate vdso symbols with source lines we need specify a
> debugging version.
> 
> For x86, we can find them from your local build as
> arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may reside in
> /lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that
> the buildid has to match.
> 
> $ sudo perf record -a
> $ sudo perf report --objdump=llvm-objdump \
>   --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg
> 
> When doing cross platform analysis, we also need specify the vdso path if
> we are interested in its symbols.

We already have logic to find debug files, and to deal with a symbol source
(syms_ss) and runtime symbol source (runtime_ss).

Can't we make that work transparently for vdso so that the user does
not have to.

> 
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
>  tools/perf/builtin-annotate.c |  2 +
>  tools/perf/builtin-c2c.c      |  2 +
>  tools/perf/builtin-inject.c   |  2 +
>  tools/perf/builtin-report.c   |  2 +
>  tools/perf/builtin-script.c   |  2 +
>  tools/perf/builtin-top.c      |  2 +
>  tools/perf/util/symbol.c      | 74 +++++++++++++++++++++++++++++++++++
>  tools/perf/util/symbol_conf.h |  5 +++
>  8 files changed, 91 insertions(+)
> 
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 50d2fb222d48..ff466882065d 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -742,6 +742,8 @@ int cmd_annotate(int argc, const char **argv)
>  		   "file", "vmlinux pathname"),
>  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
>  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  	OPT_BOOLEAN('l', "print-line", &annotate_opts.print_lines,
>  		    "print matching source lines (may be slow)"),
>  	OPT_BOOLEAN('P', "full-paths", &annotate_opts.full_path,
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index c157bd31f2e5..4764f9139661 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -3018,6 +3018,8 @@ static int perf_c2c__report(int argc, const char **argv)
>  	const struct option options[] = {
>  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
>  		   "file", "vmlinux pathname"),
> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  	OPT_STRING('i', "input", &input_name, "file",
>  		   "the input file to process"),
>  	OPT_INCR('N', "node-info", &c2c.node_info,
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index a212678d47be..e774e83d0a0f 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -2247,6 +2247,8 @@ int cmd_inject(int argc, const char **argv)
>  			    "don't load vmlinux even if found"),
>  		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
>  			   "kallsyms pathname"),
> +		OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  		OPT_BOOLEAN('f', "force", &data.force, "don't complain, do it"),
>  		OPT_CALLBACK_OPTARG(0, "itrace", &inject.itrace_synth_opts,
>  				    NULL, "opts", "Instruction Tracing options\n"
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 69618fb0110b..a64b48460dce 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1324,6 +1324,8 @@ int cmd_report(int argc, const char **argv)
>                      "don't load vmlinux even if found"),
>  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
>  		   "file", "kallsyms pathname"),
> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
>  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
>  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index c16224b1fef3..2e358922a8d1 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -3965,6 +3965,8 @@ int cmd_script(int argc, const char **argv)
>  		   "file", "vmlinux pathname"),
>  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
>  		   "file", "kallsyms pathname"),
> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
>  		    "When printing symbols do not display call chain"),
>  	OPT_CALLBACK(0, "symfs", NULL, "directory",
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 1d6aef51c122..a3cce4e76eb9 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1479,6 +1479,8 @@ int cmd_top(int argc, const char **argv)
>  		   "file", "kallsyms pathname"),
>  	OPT_BOOLEAN('K', "hide_kernel_symbols", &top.hide_kernel_symbols,
>  		    "hide kernel symbols"),
> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> +		     parse_vdso_pathnames),
>  	OPT_CALLBACK('m', "mmap-pages", &opts->mmap_pages, "pages",
>  		     "number of mmap data pages", evlist__parse_mmap_pages),
>  	OPT_INTEGER('r', "realtime", &top.realtime_prio,
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 9e5940b5bc59..8d040039a7ce 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -19,6 +19,7 @@
>  #include "build-id.h"
>  #include "cap.h"
>  #include "dso.h"
> +#include "vdso.h"
>  #include "util.h" // lsdir()
>  #include "debug.h"
>  #include "event.h"
> @@ -44,6 +45,7 @@
>  
>  static int dso__load_kernel_sym(struct dso *dso, struct map *map);
>  static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map);
> +static int dso__load_vdso_sym(struct dso *dso, struct map *map);
>  static bool symbol__is_idle(const char *name);
>  
>  int vmlinux_path__nr_entries;
> @@ -1833,6 +1835,12 @@ int dso__load(struct dso *dso, struct map *map)
>  		goto out;
>  	}
>  
> +	if (dso__is_vdso(dso)) {
> +		ret = dso__load_vdso_sym(dso, map);
> +		if (ret > 0)
> +			goto out;
> +	}
> +
>  	dso__set_adjust_symbols(dso, false);
>  
>  	if (perfmap) {
> @@ -2349,6 +2357,72 @@ static int vmlinux_path__init(struct perf_env *env)
>  	return -1;
>  }
>  
> +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
> +			 const char *arg, int unset __maybe_unused)
> +{
> +	char *tmp, *tok, *str = strdup(arg);
> +	unsigned int i = 0;
> +
> +	for (tok = strtok_r(str, ",", &tmp); tok && i < ARRAY_SIZE(symbol_conf.vdso_name);
> +	     tok = strtok_r(NULL, ",", &tmp)) {
> +		symbol_conf.vdso_name[i++] = strdup(tok);
> +	}
> +
> +	free(str);
> +	return 0;
> +}
> +
> +static int dso__load_vdso(struct dso *dso, struct map *map,
> +			  const char *vdso)
> +{
> +	int err = -1;
> +	struct symsrc ss;
> +	char symfs_vdso[PATH_MAX];
> +
> +	if (vdso[0] == '/')
> +		snprintf(symfs_vdso, sizeof(symfs_vdso), "%s", vdso);
> +	else
> +		symbol__join_symfs(symfs_vdso, vdso);
> +
> +	if (symsrc__init(&ss, dso, symfs_vdso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO))
> +		return -1;
> +
> +	/*
> +	 * dso__load_sym() may copy 'dso' which will result in the copies having
> +	 * an incorrect long name unless we set it here first.
> +	 */
> +	dso__set_long_name(dso, vdso, false);
> +	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
> +
> +	err = dso__load_sym(dso, map, &ss, &ss, 0);
> +	symsrc__destroy(&ss);
> +
> +	if (err > 0) {
> +		dso__set_loaded(dso);
> +		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
> +	}
> +
> +	return err;
> +}
> +
> +static int dso__load_vdso_sym(struct dso *dso, struct map *map)
> +{
> +	int ret;
> +
> +	if (!dso__is_vdso(dso))
> +		return -1;
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(symbol_conf.vdso_name); i++) {
> +		if (symbol_conf.vdso_name[i] != NULL) {
> +			ret = dso__load_vdso(dso, map, symbol_conf.vdso_name[i]);
> +			if (ret > 0)
> +				return ret;
> +		}
> +	}
> +
> +	return -1;
> +}
> +
>  int setup_list(struct strlist **list, const char *list_str,
>  		      const char *list_name)
>  {
> diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> index c114bbceef40..108356e3c981 100644
> --- a/tools/perf/util/symbol_conf.h
> +++ b/tools/perf/util/symbol_conf.h
> @@ -3,6 +3,7 @@
>  #define __PERF_SYMBOL_CONF 1
>  
>  #include <stdbool.h>
> +#include <subcmd/parse-options.h>
>  
>  struct strlist;
>  struct intlist;
> @@ -55,6 +56,7 @@ struct symbol_conf {
>  	const char	*default_guest_vmlinux_name,
>  			*default_guest_kallsyms,
>  			*default_guest_modules;
> +	const char	*vdso_name[2];
>  	const char	*guestmount;
>  	const char	*dso_list_str,
>  			*comm_list_str,
> @@ -85,4 +87,7 @@ struct symbol_conf {
>  
>  extern struct symbol_conf symbol_conf;
>  
> +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
> +			 const char *arg, int unset __maybe_unused);
> +
>  #endif // __PERF_SYMBOL_CONF

Re: [PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

[duchangbin]

On Thu, Jun 13, 2024 at 11:15:28AM +0300, Adrian Hunter wrote:
> On 13/06/24 09:35, Changbin Du wrote:
> > If we already found a debugging version when loading symbols for that dso,
> > then use the same file for disasm instead of looking up in buildid-cache.
> 
> In the past, there have been cases where the debugging version has not
> worked for reading object code.  I don't remember the details, but the
> symbols and debugging information was OK while the object code was not.
> 
> In general, using anything other than the file that was actually executed
> for reading object code seems like a bad idea.
> 
Is this a platform specific issue? AFAIK, the binary code in debugging and
non-debugging version should be identical. 

> 
> 

-- 
Cheers,
Changbin Du

Re: [PATCH 1/2] perf: support specify vdso path in cmdline

[duchangbin]

On Thu, Jun 13, 2024 at 11:23:26AM +0300, Adrian Hunter wrote:
> On 13/06/24 09:35, Changbin Du wrote:
> > The vdso dumped from process memory (in buildid-cache) lacks debugging
> > info. To annotate vdso symbols with source lines we need specify a
> > debugging version.
> > 
> > For x86, we can find them from your local build as
> > arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may reside in
> > /lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that
> > the buildid has to match.
> > 
> > $ sudo perf record -a
> > $ sudo perf report --objdump=llvm-objdump \
> >   --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg
> > 
> > When doing cross platform analysis, we also need specify the vdso path if
> > we are interested in its symbols.
> 
> We already have logic to find debug files, and to deal with a symbol source
> (syms_ss) and runtime symbol source (runtime_ss).
> 
> Can't we make that work transparently for vdso so that the user does
> not have to.
>
For Ubuntu, we can find the debug files of vdso in
/lib/modules/<version>/vdso/vdso{32,64}.so. These two are debug version.

For local build, seems vdso{32,64}.so.dbg are not installed in /lib/modules/ nor
other locations.

> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> >  tools/perf/builtin-annotate.c |  2 +
> >  tools/perf/builtin-c2c.c      |  2 +
> >  tools/perf/builtin-inject.c   |  2 +
> >  tools/perf/builtin-report.c   |  2 +
> >  tools/perf/builtin-script.c   |  2 +
> >  tools/perf/builtin-top.c      |  2 +
> >  tools/perf/util/symbol.c      | 74 +++++++++++++++++++++++++++++++++++
> >  tools/perf/util/symbol_conf.h |  5 +++
> >  8 files changed, 91 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> > index 50d2fb222d48..ff466882065d 100644
> > --- a/tools/perf/builtin-annotate.c
> > +++ b/tools/perf/builtin-annotate.c
> > @@ -742,6 +742,8 @@ int cmd_annotate(int argc, const char **argv)
> >  		   "file", "vmlinux pathname"),
> >  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
> >  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_BOOLEAN('l', "print-line", &annotate_opts.print_lines,
> >  		    "print matching source lines (may be slow)"),
> >  	OPT_BOOLEAN('P', "full-paths", &annotate_opts.full_path,
> > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> > index c157bd31f2e5..4764f9139661 100644
> > --- a/tools/perf/builtin-c2c.c
> > +++ b/tools/perf/builtin-c2c.c
> > @@ -3018,6 +3018,8 @@ static int perf_c2c__report(int argc, const char **argv)
> >  	const struct option options[] = {
> >  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
> >  		   "file", "vmlinux pathname"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_STRING('i', "input", &input_name, "file",
> >  		   "the input file to process"),
> >  	OPT_INCR('N', "node-info", &c2c.node_info,
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index a212678d47be..e774e83d0a0f 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -2247,6 +2247,8 @@ int cmd_inject(int argc, const char **argv)
> >  			    "don't load vmlinux even if found"),
> >  		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
> >  			   "kallsyms pathname"),
> > +		OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  		OPT_BOOLEAN('f', "force", &data.force, "don't complain, do it"),
> >  		OPT_CALLBACK_OPTARG(0, "itrace", &inject.itrace_synth_opts,
> >  				    NULL, "opts", "Instruction Tracing options\n"
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 69618fb0110b..a64b48460dce 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -1324,6 +1324,8 @@ int cmd_report(int argc, const char **argv)
> >                      "don't load vmlinux even if found"),
> >  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
> >  		   "file", "kallsyms pathname"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
> >  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
> >  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index c16224b1fef3..2e358922a8d1 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -3965,6 +3965,8 @@ int cmd_script(int argc, const char **argv)
> >  		   "file", "vmlinux pathname"),
> >  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
> >  		   "file", "kallsyms pathname"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
> >  		    "When printing symbols do not display call chain"),
> >  	OPT_CALLBACK(0, "symfs", NULL, "directory",
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index 1d6aef51c122..a3cce4e76eb9 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -1479,6 +1479,8 @@ int cmd_top(int argc, const char **argv)
> >  		   "file", "kallsyms pathname"),
> >  	OPT_BOOLEAN('K', "hide_kernel_symbols", &top.hide_kernel_symbols,
> >  		    "hide kernel symbols"),
> > +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
> > +		     parse_vdso_pathnames),
> >  	OPT_CALLBACK('m', "mmap-pages", &opts->mmap_pages, "pages",
> >  		     "number of mmap data pages", evlist__parse_mmap_pages),
> >  	OPT_INTEGER('r', "realtime", &top.realtime_prio,
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 9e5940b5bc59..8d040039a7ce 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -19,6 +19,7 @@
> >  #include "build-id.h"
> >  #include "cap.h"
> >  #include "dso.h"
> > +#include "vdso.h"
> >  #include "util.h" // lsdir()
> >  #include "debug.h"
> >  #include "event.h"
> > @@ -44,6 +45,7 @@
> >  
> >  static int dso__load_kernel_sym(struct dso *dso, struct map *map);
> >  static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map);
> > +static int dso__load_vdso_sym(struct dso *dso, struct map *map);
> >  static bool symbol__is_idle(const char *name);
> >  
> >  int vmlinux_path__nr_entries;
> > @@ -1833,6 +1835,12 @@ int dso__load(struct dso *dso, struct map *map)
> >  		goto out;
> >  	}
> >  
> > +	if (dso__is_vdso(dso)) {
> > +		ret = dso__load_vdso_sym(dso, map);
> > +		if (ret > 0)
> > +			goto out;
> > +	}
> > +
> >  	dso__set_adjust_symbols(dso, false);
> >  
> >  	if (perfmap) {
> > @@ -2349,6 +2357,72 @@ static int vmlinux_path__init(struct perf_env *env)
> >  	return -1;
> >  }
> >  
> > +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
> > +			 const char *arg, int unset __maybe_unused)
> > +{
> > +	char *tmp, *tok, *str = strdup(arg);
> > +	unsigned int i = 0;
> > +
> > +	for (tok = strtok_r(str, ",", &tmp); tok && i < ARRAY_SIZE(symbol_conf.vdso_name);
> > +	     tok = strtok_r(NULL, ",", &tmp)) {
> > +		symbol_conf.vdso_name[i++] = strdup(tok);
> > +	}
> > +
> > +	free(str);
> > +	return 0;
> > +}
> > +
> > +static int dso__load_vdso(struct dso *dso, struct map *map,
> > +			  const char *vdso)
> > +{
> > +	int err = -1;
> > +	struct symsrc ss;
> > +	char symfs_vdso[PATH_MAX];
> > +
> > +	if (vdso[0] == '/')
> > +		snprintf(symfs_vdso, sizeof(symfs_vdso), "%s", vdso);
> > +	else
> > +		symbol__join_symfs(symfs_vdso, vdso);
> > +
> > +	if (symsrc__init(&ss, dso, symfs_vdso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO))
> > +		return -1;
> > +
> > +	/*
> > +	 * dso__load_sym() may copy 'dso' which will result in the copies having
> > +	 * an incorrect long name unless we set it here first.
> > +	 */
> > +	dso__set_long_name(dso, vdso, false);
> > +	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
> > +
> > +	err = dso__load_sym(dso, map, &ss, &ss, 0);
> > +	symsrc__destroy(&ss);
> > +
> > +	if (err > 0) {
> > +		dso__set_loaded(dso);
> > +		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int dso__load_vdso_sym(struct dso *dso, struct map *map)
> > +{
> > +	int ret;
> > +
> > +	if (!dso__is_vdso(dso))
> > +		return -1;
> > +
> > +	for (unsigned int i = 0; i < ARRAY_SIZE(symbol_conf.vdso_name); i++) {
> > +		if (symbol_conf.vdso_name[i] != NULL) {
> > +			ret = dso__load_vdso(dso, map, symbol_conf.vdso_name[i]);
> > +			if (ret > 0)
> > +				return ret;
> > +		}
> > +	}
> > +
> > +	return -1;
> > +}
> > +
> >  int setup_list(struct strlist **list, const char *list_str,
> >  		      const char *list_name)
> >  {
> > diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> > index c114bbceef40..108356e3c981 100644
> > --- a/tools/perf/util/symbol_conf.h
> > +++ b/tools/perf/util/symbol_conf.h
> > @@ -3,6 +3,7 @@
> >  #define __PERF_SYMBOL_CONF 1
> >  
> >  #include <stdbool.h>
> > +#include <subcmd/parse-options.h>
> >  
> >  struct strlist;
> >  struct intlist;
> > @@ -55,6 +56,7 @@ struct symbol_conf {
> >  	const char	*default_guest_vmlinux_name,
> >  			*default_guest_kallsyms,
> >  			*default_guest_modules;
> > +	const char	*vdso_name[2];
> >  	const char	*guestmount;
> >  	const char	*dso_list_str,
> >  			*comm_list_str,
> > @@ -85,4 +87,7 @@ struct symbol_conf {
> >  
> >  extern struct symbol_conf symbol_conf;
> >  
> > +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
> > +			 const char *arg, int unset __maybe_unused);
> > +
> >  #endif // __PERF_SYMBOL_CONF
> 
> 

-- 
Cheers,
Changbin Du

Re: [PATCH 1/2] perf: support specify vdso path in cmdline

[Adrian Hunter]

On 13/06/24 12:49, duchangbin wrote:
> On Thu, Jun 13, 2024 at 11:23:26AM +0300, Adrian Hunter wrote:
>> On 13/06/24 09:35, Changbin Du wrote:
>>> The vdso dumped from process memory (in buildid-cache) lacks debugging
>>> info. To annotate vdso symbols with source lines we need specify a
>>> debugging version.
>>>
>>> For x86, we can find them from your local build as
>>> arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may reside in
>>> /lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that
>>> the buildid has to match.
>>>
>>> $ sudo perf record -a
>>> $ sudo perf report --objdump=llvm-objdump \
>>>   --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg
>>>
>>> When doing cross platform analysis, we also need specify the vdso path if
>>> we are interested in its symbols.
>>
>> We already have logic to find debug files, and to deal with a symbol source
>> (syms_ss) and runtime symbol source (runtime_ss).
>>
>> Can't we make that work transparently for vdso so that the user does
>> not have to.
>>
> For Ubuntu, we can find the debug files of vdso in
> /lib/modules/<version>/vdso/vdso{32,64}.so. These two are debug version.
> 
> For local build, seems vdso{32,64}.so.dbg are not installed in /lib/modules/ nor
> other locations.

Isn't there /lib/modules/<version>/build symbolic link to follow?

> 
>>>
>>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
>>> ---
>>>  tools/perf/builtin-annotate.c |  2 +
>>>  tools/perf/builtin-c2c.c      |  2 +
>>>  tools/perf/builtin-inject.c   |  2 +
>>>  tools/perf/builtin-report.c   |  2 +
>>>  tools/perf/builtin-script.c   |  2 +
>>>  tools/perf/builtin-top.c      |  2 +
>>>  tools/perf/util/symbol.c      | 74 +++++++++++++++++++++++++++++++++++
>>>  tools/perf/util/symbol_conf.h |  5 +++
>>>  8 files changed, 91 insertions(+)
>>>
>>> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
>>> index 50d2fb222d48..ff466882065d 100644
>>> --- a/tools/perf/builtin-annotate.c
>>> +++ b/tools/perf/builtin-annotate.c
>>> @@ -742,6 +742,8 @@ int cmd_annotate(int argc, const char **argv)
>>>  		   "file", "vmlinux pathname"),
>>>  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
>>>  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
>>> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
>>> +		     parse_vdso_pathnames),
>>>  	OPT_BOOLEAN('l', "print-line", &annotate_opts.print_lines,
>>>  		    "print matching source lines (may be slow)"),
>>>  	OPT_BOOLEAN('P', "full-paths", &annotate_opts.full_path,
>>> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
>>> index c157bd31f2e5..4764f9139661 100644
>>> --- a/tools/perf/builtin-c2c.c
>>> +++ b/tools/perf/builtin-c2c.c
>>> @@ -3018,6 +3018,8 @@ static int perf_c2c__report(int argc, const char **argv)
>>>  	const struct option options[] = {
>>>  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
>>>  		   "file", "vmlinux pathname"),
>>> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
>>> +		     parse_vdso_pathnames),
>>>  	OPT_STRING('i', "input", &input_name, "file",
>>>  		   "the input file to process"),
>>>  	OPT_INCR('N', "node-info", &c2c.node_info,
>>> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
>>> index a212678d47be..e774e83d0a0f 100644
>>> --- a/tools/perf/builtin-inject.c
>>> +++ b/tools/perf/builtin-inject.c
>>> @@ -2247,6 +2247,8 @@ int cmd_inject(int argc, const char **argv)
>>>  			    "don't load vmlinux even if found"),
>>>  		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
>>>  			   "kallsyms pathname"),
>>> +		OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
>>> +		     parse_vdso_pathnames),
>>>  		OPT_BOOLEAN('f', "force", &data.force, "don't complain, do it"),
>>>  		OPT_CALLBACK_OPTARG(0, "itrace", &inject.itrace_synth_opts,
>>>  				    NULL, "opts", "Instruction Tracing options\n"
>>> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
>>> index 69618fb0110b..a64b48460dce 100644
>>> --- a/tools/perf/builtin-report.c
>>> +++ b/tools/perf/builtin-report.c
>>> @@ -1324,6 +1324,8 @@ int cmd_report(int argc, const char **argv)
>>>                      "don't load vmlinux even if found"),
>>>  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
>>>  		   "file", "kallsyms pathname"),
>>> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
>>> +		     parse_vdso_pathnames),
>>>  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
>>>  	OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
>>>  		    "load module symbols - WARNING: use only with -k and LIVE kernel"),
>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>> index c16224b1fef3..2e358922a8d1 100644
>>> --- a/tools/perf/builtin-script.c
>>> +++ b/tools/perf/builtin-script.c
>>> @@ -3965,6 +3965,8 @@ int cmd_script(int argc, const char **argv)
>>>  		   "file", "vmlinux pathname"),
>>>  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
>>>  		   "file", "kallsyms pathname"),
>>> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
>>> +		     parse_vdso_pathnames),
>>>  	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
>>>  		    "When printing symbols do not display call chain"),
>>>  	OPT_CALLBACK(0, "symfs", NULL, "directory",
>>> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
>>> index 1d6aef51c122..a3cce4e76eb9 100644
>>> --- a/tools/perf/builtin-top.c
>>> +++ b/tools/perf/builtin-top.c
>>> @@ -1479,6 +1479,8 @@ int cmd_top(int argc, const char **argv)
>>>  		   "file", "kallsyms pathname"),
>>>  	OPT_BOOLEAN('K', "hide_kernel_symbols", &top.hide_kernel_symbols,
>>>  		    "hide kernel symbols"),
>>> +	OPT_CALLBACK(0, "vdso", NULL, "vdso1[,vdso2]", "vdso pathnames",
>>> +		     parse_vdso_pathnames),
>>>  	OPT_CALLBACK('m', "mmap-pages", &opts->mmap_pages, "pages",
>>>  		     "number of mmap data pages", evlist__parse_mmap_pages),
>>>  	OPT_INTEGER('r', "realtime", &top.realtime_prio,
>>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>>> index 9e5940b5bc59..8d040039a7ce 100644
>>> --- a/tools/perf/util/symbol.c
>>> +++ b/tools/perf/util/symbol.c
>>> @@ -19,6 +19,7 @@
>>>  #include "build-id.h"
>>>  #include "cap.h"
>>>  #include "dso.h"
>>> +#include "vdso.h"
>>>  #include "util.h" // lsdir()
>>>  #include "debug.h"
>>>  #include "event.h"
>>> @@ -44,6 +45,7 @@
>>>  
>>>  static int dso__load_kernel_sym(struct dso *dso, struct map *map);
>>>  static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map);
>>> +static int dso__load_vdso_sym(struct dso *dso, struct map *map);
>>>  static bool symbol__is_idle(const char *name);
>>>  
>>>  int vmlinux_path__nr_entries;
>>> @@ -1833,6 +1835,12 @@ int dso__load(struct dso *dso, struct map *map)
>>>  		goto out;
>>>  	}
>>>  
>>> +	if (dso__is_vdso(dso)) {
>>> +		ret = dso__load_vdso_sym(dso, map);
>>> +		if (ret > 0)
>>> +			goto out;
>>> +	}
>>> +
>>>  	dso__set_adjust_symbols(dso, false);
>>>  
>>>  	if (perfmap) {
>>> @@ -2349,6 +2357,72 @@ static int vmlinux_path__init(struct perf_env *env)
>>>  	return -1;
>>>  }
>>>  
>>> +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
>>> +			 const char *arg, int unset __maybe_unused)
>>> +{
>>> +	char *tmp, *tok, *str = strdup(arg);
>>> +	unsigned int i = 0;
>>> +
>>> +	for (tok = strtok_r(str, ",", &tmp); tok && i < ARRAY_SIZE(symbol_conf.vdso_name);
>>> +	     tok = strtok_r(NULL, ",", &tmp)) {
>>> +		symbol_conf.vdso_name[i++] = strdup(tok);
>>> +	}
>>> +
>>> +	free(str);
>>> +	return 0;
>>> +}
>>> +
>>> +static int dso__load_vdso(struct dso *dso, struct map *map,
>>> +			  const char *vdso)
>>> +{
>>> +	int err = -1;
>>> +	struct symsrc ss;
>>> +	char symfs_vdso[PATH_MAX];
>>> +
>>> +	if (vdso[0] == '/')
>>> +		snprintf(symfs_vdso, sizeof(symfs_vdso), "%s", vdso);
>>> +	else
>>> +		symbol__join_symfs(symfs_vdso, vdso);
>>> +
>>> +	if (symsrc__init(&ss, dso, symfs_vdso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO))
>>> +		return -1;
>>> +
>>> +	/*
>>> +	 * dso__load_sym() may copy 'dso' which will result in the copies having
>>> +	 * an incorrect long name unless we set it here first.
>>> +	 */
>>> +	dso__set_long_name(dso, vdso, false);
>>> +	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
>>> +
>>> +	err = dso__load_sym(dso, map, &ss, &ss, 0);
>>> +	symsrc__destroy(&ss);
>>> +
>>> +	if (err > 0) {
>>> +		dso__set_loaded(dso);
>>> +		pr_debug("Using %s for %s symbols\n", symfs_vdso, dso__short_name(dso));
>>> +	}
>>> +
>>> +	return err;
>>> +}
>>> +
>>> +static int dso__load_vdso_sym(struct dso *dso, struct map *map)
>>> +{
>>> +	int ret;
>>> +
>>> +	if (!dso__is_vdso(dso))
>>> +		return -1;
>>> +
>>> +	for (unsigned int i = 0; i < ARRAY_SIZE(symbol_conf.vdso_name); i++) {
>>> +		if (symbol_conf.vdso_name[i] != NULL) {
>>> +			ret = dso__load_vdso(dso, map, symbol_conf.vdso_name[i]);
>>> +			if (ret > 0)
>>> +				return ret;
>>> +		}
>>> +	}
>>> +
>>> +	return -1;
>>> +}
>>> +
>>>  int setup_list(struct strlist **list, const char *list_str,
>>>  		      const char *list_name)
>>>  {
>>> diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
>>> index c114bbceef40..108356e3c981 100644
>>> --- a/tools/perf/util/symbol_conf.h
>>> +++ b/tools/perf/util/symbol_conf.h
>>> @@ -3,6 +3,7 @@
>>>  #define __PERF_SYMBOL_CONF 1
>>>  
>>>  #include <stdbool.h>
>>> +#include <subcmd/parse-options.h>
>>>  
>>>  struct strlist;
>>>  struct intlist;
>>> @@ -55,6 +56,7 @@ struct symbol_conf {
>>>  	const char	*default_guest_vmlinux_name,
>>>  			*default_guest_kallsyms,
>>>  			*default_guest_modules;
>>> +	const char	*vdso_name[2];
>>>  	const char	*guestmount;
>>>  	const char	*dso_list_str,
>>>  			*comm_list_str,
>>> @@ -85,4 +87,7 @@ struct symbol_conf {
>>>  
>>>  extern struct symbol_conf symbol_conf;
>>>  
>>> +int parse_vdso_pathnames(const struct option *opt __maybe_unused,
>>> +			 const char *arg, int unset __maybe_unused);
>>> +
>>>  #endif // __PERF_SYMBOL_CONF
>>
>>
> 

Re: [PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

[Adrian Hunter]

On 13/06/24 12:43, duchangbin wrote:
> On Thu, Jun 13, 2024 at 11:15:28AM +0300, Adrian Hunter wrote:
>> On 13/06/24 09:35, Changbin Du wrote:
>>> If we already found a debugging version when loading symbols for that dso,
>>> then use the same file for disasm instead of looking up in buildid-cache.
>>
>> In the past, there have been cases where the debugging version has not
>> worked for reading object code.  I don't remember the details, but the
>> symbols and debugging information was OK while the object code was not.
>>
>> In general, using anything other than the file that was actually executed
>> for reading object code seems like a bad idea.
>>
> Is this a platform specific issue? AFAIK, the binary code in debugging and
> non-debugging version should be identical. 

"should be" != "guaranteed to be".  Simpler to avoid the issue and
stick with the file that was actually executed.  We already support
having separate symbol sources, so there should not really be a
problem.

Re: [PATCH 2/2] perf: disasm: prefer symsrc_filename for filename

[duchangbin]

On Thu, Jun 13, 2024 at 01:26:39PM +0300, Adrian Hunter wrote:
> On 13/06/24 12:43, duchangbin wrote:
> > On Thu, Jun 13, 2024 at 11:15:28AM +0300, Adrian Hunter wrote:
> >> On 13/06/24 09:35, Changbin Du wrote:
> >>> If we already found a debugging version when loading symbols for that dso,
> >>> then use the same file for disasm instead of looking up in buildid-cache.
> >>
> >> In the past, there have been cases where the debugging version has not
> >> worked for reading object code.  I don't remember the details, but the
> >> symbols and debugging information was OK while the object code was not.
> >>
> >> In general, using anything other than the file that was actually executed
> >> for reading object code seems like a bad idea.
> >>
> > Is this a platform specific issue? AFAIK, the binary code in debugging and
> > non-debugging version should be identical. 
> 
> "should be" != "guaranteed to be".  Simpler to avoid the issue and
> stick with the file that was actually executed.  We already support
> having separate symbol sources, so there should not really be a
> problem.
>
ok, so for vdso I think we can flow the kernel part in dso__disassemble_filename.
I'll add vdso processing here.

> 

-- 
Cheers,
Changbin Du

Re: [PATCH 1/2] perf: support specify vdso path in cmdline

[duchangbin]

On Thu, Jun 13, 2024 at 01:19:20PM +0300, Adrian Hunter wrote:
> On 13/06/24 12:49, duchangbin wrote:
> > On Thu, Jun 13, 2024 at 11:23:26AM +0300, Adrian Hunter wrote:
> >> On 13/06/24 09:35, Changbin Du wrote:
> >>> The vdso dumped from process memory (in buildid-cache) lacks debugging
> >>> info. To annotate vdso symbols with source lines we need specify a
> >>> debugging version.
> >>>
> >>> For x86, we can find them from your local build as
> >>> arch/x86/entry/vdso/vdso{32,64}.so.dbg. Or they may reside in
> >>> /lib/modules/<version>/vdso/vdso{32,64}.so on Ubuntu. But notice that
> >>> the buildid has to match.
> >>>
> >>> $ sudo perf record -a
> >>> $ sudo perf report --objdump=llvm-objdump \
> >>>   --vdso arch/x86/entry/vdso/vdso64.so.dbg,arch/x86/entry/vdso/vdso32.so.dbg
> >>>
> >>> When doing cross platform analysis, we also need specify the vdso path if
> >>> we are interested in its symbols.
> >>
> >> We already have logic to find debug files, and to deal with a symbol source
> >> (syms_ss) and runtime symbol source (runtime_ss).
> >>
> >> Can't we make that work transparently for vdso so that the user does
> >> not have to.
> >>
> > For Ubuntu, we can find the debug files of vdso in
> > /lib/modules/<version>/vdso/vdso{32,64}.so. These two are debug version.
> > 
> > For local build, seems vdso{32,64}.so.dbg are not installed in /lib/modules/ nor
> > other locations.
> 
> Isn't there /lib/modules/<version>/build symbolic link to follow?
> 
hm, we can try /lib/modules/<version>/build/arch/x86/entry/vdso/vdso*.so.dbg.
Other arches are similar.

So, we will try these pathes internally. And keep '--vdso' if someone really
want a special path. (just like vmlinux)

-- 
Cheers,
Changbin Du