[v1] perf script: add --addr2line option

[PATCH] perf script: add --addr2line option

Posted by m.liska@foxlink.cz 1 year, 5 months ago

From: Martin Liska <martin.liska@hey.com>

Similarly to other subcommands (like report, top), it would be handy to
provide a path for addr2line command.
---
 tools/perf/Documentation/perf-script.txt |  3 +++
 tools/perf/builtin-script.c              | 10 +++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index ff086ef05a0c..5abb960c4960 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -369,6 +369,9 @@ OPTIONS
 --demangle-kernel::
 	Demangle kernel symbol names to human readable form (for C++ kernels).
 
+--addr2line=<path>::
+	Path to addr2line binary.
+
 --header
 	Show perf.data header.
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index c16224b1fef3..301ea1c98e36 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3897,7 +3897,7 @@ int cmd_script(int argc, const char **argv)
 	};
 	struct utsname uts;
 	char *script_path = NULL;
-	const char *dlfilter_file = NULL;
+	const char *dlfilter_file = NULL, *addr2line_path = NULL;
 	const char **__argv;
 	int i, j, err = 0;
 	struct perf_script script = {
@@ -4052,6 +4052,8 @@ int cmd_script(int argc, const char **argv)
 			"Enable symbol demangling"),
 	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
 			"Enable kernel symbol demangling"),
+	OPT_STRING(0, "addr2line", &addr2line_path, "path",
+			"addr2line binary to use for line numbers"),
 	OPT_STRING(0, "time", &script.time_str, "str",
 		   "Time span of interest (start,stop)"),
 	OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name,
@@ -4138,6 +4140,12 @@ int cmd_script(int argc, const char **argv)
 	    itrace_synth_opts.callchain_sz > scripting_max_stack)
 		scripting_max_stack = itrace_synth_opts.callchain_sz;
 
+	if (addr2line_path) {
+		symbol_conf.addr2line_path = strdup(addr2line_path);
+		if (!symbol_conf.addr2line_path)
+			return -ENOMEM;
+	}
+
 	/* make sure PERF_EXEC_PATH is set for scripts */
 	set_argv_exec_path(get_argv_exec_path());
 
-- 
2.45.2

Re: [PATCH] perf script: add --addr2line option

Posted by Ian Rogers 1 year, 5 months ago

On Thu, Jul 18, 2024 at 7:30 AM <m.liska@foxlink.cz> wrote:
>
> From: Martin Liska <martin.liska@hey.com>
>
> Similarly to other subcommands (like report, top), it would be handy to
> provide a path for addr2line command.

Thanks Martin, lgtm but you did put a Signed-off-by tag in your commit
message. There is an option in git commit and git format-patch to add
this for you.

> ---
>  tools/perf/Documentation/perf-script.txt |  3 +++
>  tools/perf/builtin-script.c              | 10 +++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index ff086ef05a0c..5abb960c4960 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -369,6 +369,9 @@ OPTIONS
>  --demangle-kernel::
>         Demangle kernel symbol names to human readable form (for C++ kernels).
>
> +--addr2line=<path>::
> +       Path to addr2line binary.
> +
>  --header
>         Show perf.data header.
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index c16224b1fef3..301ea1c98e36 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -3897,7 +3897,7 @@ int cmd_script(int argc, const char **argv)
>         };
>         struct utsname uts;
>         char *script_path = NULL;
> -       const char *dlfilter_file = NULL;
> +       const char *dlfilter_file = NULL, *addr2line_path = NULL;
>         const char **__argv;
>         int i, j, err = 0;
>         struct perf_script script = {
> @@ -4052,6 +4052,8 @@ int cmd_script(int argc, const char **argv)
>                         "Enable symbol demangling"),
>         OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
>                         "Enable kernel symbol demangling"),
> +       OPT_STRING(0, "addr2line", &addr2line_path, "path",

Thinking out loud. I'm kind of wondering why we use a local variable
and not just &symbol_conf.addr2line_path here. I see you've used the
same logic that is elsewhere like builtin-top.c, so I think it is
okay.

Thanks,
Ian

> +                       "addr2line binary to use for line numbers"),
>         OPT_STRING(0, "time", &script.time_str, "str",
>                    "Time span of interest (start,stop)"),
>         OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name,
> @@ -4138,6 +4140,12 @@ int cmd_script(int argc, const char **argv)
>             itrace_synth_opts.callchain_sz > scripting_max_stack)
>                 scripting_max_stack = itrace_synth_opts.callchain_sz;
>
> +       if (addr2line_path) {
> +               symbol_conf.addr2line_path = strdup(addr2line_path);
> +               if (!symbol_conf.addr2line_path)
> +                       return -ENOMEM;
> +       }
> +
>         /* make sure PERF_EXEC_PATH is set for scripts */
>         set_argv_exec_path(get_argv_exec_path());
>
> --
> 2.45.2
>

Re: [PATCH] perf script: add --addr2line option

Posted by Martin Liška 1 year, 4 months ago


On 7/18/24 17:44, Ian Rogers wrote:
> On Thu, Jul 18, 2024 at 7:30 AM <m.liska@foxlink.cz> wrote:
>>
>> From: Martin Liska <martin.liska@hey.com>
>>
>> Similarly to other subcommands (like report, top), it would be handy to
>> provide a path for addr2line command.
> 
> Thanks Martin, lgtm but you did put a Signed-off-by tag in your commit

Hello.

You likely meant "did not", right? I'm going to fix it in V2.

> message. There is an option in git commit and git format-patch to add
> this for you.
> 
>> ---
>>  tools/perf/Documentation/perf-script.txt |  3 +++
>>  tools/perf/builtin-script.c              | 10 +++++++++-
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
>> index ff086ef05a0c..5abb960c4960 100644
>> --- a/tools/perf/Documentation/perf-script.txt
>> +++ b/tools/perf/Documentation/perf-script.txt
>> @@ -369,6 +369,9 @@ OPTIONS
>>  --demangle-kernel::
>>         Demangle kernel symbol names to human readable form (for C++ kernels).
>>
>> +--addr2line=<path>::
>> +       Path to addr2line binary.
>> +
>>  --header
>>         Show perf.data header.
>>
>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>> index c16224b1fef3..301ea1c98e36 100644
>> --- a/tools/perf/builtin-script.c
>> +++ b/tools/perf/builtin-script.c
>> @@ -3897,7 +3897,7 @@ int cmd_script(int argc, const char **argv)
>>         };
>>         struct utsname uts;
>>         char *script_path = NULL;
>> -       const char *dlfilter_file = NULL;
>> +       const char *dlfilter_file = NULL, *addr2line_path = NULL;
>>         const char **__argv;
>>         int i, j, err = 0;
>>         struct perf_script script = {
>> @@ -4052,6 +4052,8 @@ int cmd_script(int argc, const char **argv)
>>                         "Enable symbol demangling"),
>>         OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
>>                         "Enable kernel symbol demangling"),
>> +       OPT_STRING(0, "addr2line", &addr2line_path, "path",
> 
> Thinking out loud. I'm kind of wondering why we use a local variable
> and not just &symbol_conf.addr2line_path here. I see you've used the
> same logic that is elsewhere like builtin-top.c, so I think it is
> okay.

Yeah, you are right, we can simplify the code as suggested!

Martin

> 
> Thanks,
> Ian
> 
>> +                       "addr2line binary to use for line numbers"),
>>         OPT_STRING(0, "time", &script.time_str, "str",
>>                    "Time span of interest (start,stop)"),
>>         OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name,
>> @@ -4138,6 +4140,12 @@ int cmd_script(int argc, const char **argv)
>>             itrace_synth_opts.callchain_sz > scripting_max_stack)
>>                 scripting_max_stack = itrace_synth_opts.callchain_sz;
>>
>> +       if (addr2line_path) {
>> +               symbol_conf.addr2line_path = strdup(addr2line_path);
>> +               if (!symbol_conf.addr2line_path)
>> +                       return -ENOMEM;
>> +       }
>> +
>>         /* make sure PERF_EXEC_PATH is set for scripts */
>>         set_argv_exec_path(get_argv_exec_path());
>>
>> --
>> 2.45.2
>>

[PATCH v2] perf script: add --addr2line option

Posted by Martin Liška 1 year, 4 months ago

Similarly to other subcommands (like report, top), it would be handy to
provide a path for addr2line command.

Signed-off-by: Martin Liska <martin.liska@hey.com>
---
 tools/perf/Documentation/perf-script.txt | 3 +++
 tools/perf/builtin-script.c              | 2 ++
 tools/perf/util/symbol_conf.h            | 2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index ff086ef05a0c..5abb960c4960 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -369,6 +369,9 @@ OPTIONS
 --demangle-kernel::
 	Demangle kernel symbol names to human readable form (for C++ kernels).
 
+--addr2line=<path>::
+	Path to addr2line binary.
+
 --header
 	Show perf.data header.
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index c16224b1fef3..932167b2362b 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -4052,6 +4052,8 @@ int cmd_script(int argc, const char **argv)
 			"Enable symbol demangling"),
 	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
 			"Enable kernel symbol demangling"),
+	OPT_STRING(0, "addr2line", &symbol_conf.addr2line_path, "path",
+			"addr2line binary to use for line numbers"),
 	OPT_STRING(0, "time", &script.time_str, "str",
 		   "Time span of interest (start,stop)"),
 	OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name,
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index c114bbceef40..d41b25e10f1b 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -63,7 +63,7 @@ struct symbol_conf {
 			*sym_list_str,
 			*col_width_list_str,
 			*bt_stop_list_str;
-	char		*addr2line_path;
+	const char		*addr2line_path;
 	unsigned long	time_quantum;
        struct strlist	*dso_list,
 			*comm_list,
-- 
2.45.2

Re: [PATCH v2] perf script: add --addr2line option

Posted by Martin Liška 1 year, 4 months ago

@acme: May I please ping this patch?

Thanks,
Martin

On 7/19/24 12:57, Martin Liška wrote:
> Similarly to other subcommands (like report, top), it would be handy to
> provide a path for addr2line command.
> 
> Signed-off-by: Martin Liska <martin.liska@hey.com>
> ---
>  tools/perf/Documentation/perf-script.txt | 3 +++
>  tools/perf/builtin-script.c              | 2 ++
>  tools/perf/util/symbol_conf.h            | 2 +-
>  3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index ff086ef05a0c..5abb960c4960 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -369,6 +369,9 @@ OPTIONS
>  --demangle-kernel::
>  	Demangle kernel symbol names to human readable form (for C++ kernels).
>  
> +--addr2line=<path>::
> +	Path to addr2line binary.
> +
>  --header
>  	Show perf.data header.
>  
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index c16224b1fef3..932167b2362b 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -4052,6 +4052,8 @@ int cmd_script(int argc, const char **argv)
>  			"Enable symbol demangling"),
>  	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
>  			"Enable kernel symbol demangling"),
> +	OPT_STRING(0, "addr2line", &symbol_conf.addr2line_path, "path",
> +			"addr2line binary to use for line numbers"),
>  	OPT_STRING(0, "time", &script.time_str, "str",
>  		   "Time span of interest (start,stop)"),
>  	OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name,
> diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> index c114bbceef40..d41b25e10f1b 100644
> --- a/tools/perf/util/symbol_conf.h
> +++ b/tools/perf/util/symbol_conf.h
> @@ -63,7 +63,7 @@ struct symbol_conf {
>  			*sym_list_str,
>  			*col_width_list_str,
>  			*bt_stop_list_str;
> -	char		*addr2line_path;
> +	const char		*addr2line_path;
>  	unsigned long	time_quantum;
>         struct strlist	*dso_list,
>  			*comm_list,

Re: [PATCH v2] perf script: add --addr2line option

Posted by Arnaldo Carvalho de Melo 1 year, 4 months ago

On Mon, Aug 12, 2024 at 11:42:31AM +0200, Martin Liška wrote:
> @acme: May I please ping this patch?

Simple enough, merged, thanks for the ping.

- Arnaldo
 
> Thanks,
> Martin
> 
> On 7/19/24 12:57, Martin Liška wrote:
> > Similarly to other subcommands (like report, top), it would be handy to
> > provide a path for addr2line command.
> > 
> > Signed-off-by: Martin Liska <martin.liska@hey.com>
> > ---
> >  tools/perf/Documentation/perf-script.txt | 3 +++
> >  tools/perf/builtin-script.c              | 2 ++
> >  tools/perf/util/symbol_conf.h            | 2 +-
> >  3 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index ff086ef05a0c..5abb960c4960 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -369,6 +369,9 @@ OPTIONS
> >  --demangle-kernel::
> >  	Demangle kernel symbol names to human readable form (for C++ kernels).
> >  
> > +--addr2line=<path>::
> > +	Path to addr2line binary.
> > +
> >  --header
> >  	Show perf.data header.
> >  
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index c16224b1fef3..932167b2362b 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -4052,6 +4052,8 @@ int cmd_script(int argc, const char **argv)
> >  			"Enable symbol demangling"),
> >  	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
> >  			"Enable kernel symbol demangling"),
> > +	OPT_STRING(0, "addr2line", &symbol_conf.addr2line_path, "path",
> > +			"addr2line binary to use for line numbers"),
> >  	OPT_STRING(0, "time", &script.time_str, "str",
> >  		   "Time span of interest (start,stop)"),
> >  	OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name,
> > diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> > index c114bbceef40..d41b25e10f1b 100644
> > --- a/tools/perf/util/symbol_conf.h
> > +++ b/tools/perf/util/symbol_conf.h
> > @@ -63,7 +63,7 @@ struct symbol_conf {
> >  			*sym_list_str,
> >  			*col_width_list_str,
> >  			*bt_stop_list_str;
> > -	char		*addr2line_path;
> > +	const char		*addr2line_path;
> >  	unsigned long	time_quantum;
> >         struct strlist	*dso_list,
> >  			*comm_list,