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: <4E0BFF2D.6040108@hitachi.com>
Date:	Thu, 30 Jun 2011 13:44:29 +0900
From:	Akihiro Nagai <akihiro.nagai.hw@...achi.com>
To:	David Ahern <dsahern@...il.com>
Cc:	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	linux-kernel@...r.kernel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	yrl.pp-manager.tt@...achi.com, Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH -tip 3/4] perf script: add the option to show the offset
 of symbols

(2011/06/28 23:55), David Ahern wrote:
> On 06/28/2011 08:52 AM, David Ahern wrote:
>> On 06/28/2011 01:40 AM, Akihiro Nagai wrote:
>>> Add the option '--show-symbol-offset' to show the offset from
>>> the symbols in the output of perf-script. We can get the more
>>> detailed address information.
>>
>> I like the offset option; added that locally last week as well. more below.
>
> one more comment: move the --show-symbol-offset option to a field
> parameter - like ip, addr, dso and sym. For parameter checking make it
> depend on the sym field.
>
> Update the Documentation file as well.
OK, I'm going to add 'offs' field specifier instead of
the option '--show-symbol-offset', and update documents.

>
> David
>
>>
>>>
>>> Output sample:
>>> # perf script -f ip,addr,sym --show-symbol-offset
>>> 301ec016b0 ffffffff81467612 irq_return+0x0
>>> 301ec016b0            _start+0x0 ffffffff81467612 irq_return+0x0
>>> 301ec04b70         _dl_start+0x0       301ec016b3 _start+0x3
>>> 301ec04b70         _dl_start+0x0 ffffffff81467612 irq_return+0x0
>>> 301ec04b96        _dl_start+0x26 ffffffff81467612 irq_return+0x0
>>> 301ec04b9d        _dl_start+0x2d ffffffff81467612 irq_return+0x0
>>> 301ec04c0d        _dl_start+0x9d       301ec04beb _dl_start+0x7b
>>> 301ec04bf0        _dl_start+0x80       301ec04c11 _dl_start+0xa1
>>> [snip]
>>>
>>> Signed-off-by: Akihiro Nagai<akihiro.nagai.hw@...achi.com>
>>> Cc: Peter Zijlstra<peterz@...radead.org>
>>> Cc: Frederic Weisbecker<fweisbec@...il.com>
>>> Cc: Paul Mackerras<paulus@...ba.org>
>>> Cc: Ingo Molnar<mingo@...e.hu>
>>> Cc: Arnaldo Carvalho de Melo<acme@...radead.org>
>>> Cc: David Ahern<dsahern@...il.com>
>>> Cc: Masami Hiramatsu<masami.hiramatsu.pt@...achi.com>
>>> ---
>>>
>>>   tools/perf/Documentation/perf-script.txt |    3 +++
>>>   tools/perf/builtin-script.c              |   20 ++++++++++++++------
>>>   tools/perf/util/session.c                |   16 ++++++++++------
>>>   tools/perf/util/session.h                |    3 ++-
>>>   4 files changed, 29 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
>>> index c6068cb..39c4721 100644
>>> --- a/tools/perf/Documentation/perf-script.txt
>>> +++ b/tools/perf/Documentation/perf-script.txt
>>> @@ -182,6 +182,9 @@ OPTIONS
>>>   --hide-call-graph::
>>>           When printing symbols do not display call chain.
>>>
>>> +--show-symbol-offset::
>>> +	Show the offset of the symbols.
>>> +
>>>   SEE ALSO
>>>   --------
>>>   linkperf:perf-record[1], linkperf:perf-script-perl[1],
>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>> index 2d68086..f9b7047 100644
>>> --- a/tools/perf/builtin-script.c
>>> +++ b/tools/perf/builtin-script.c
>>> @@ -21,6 +21,7 @@ static u64			last_timestamp;
>>>   static u64			nr_unordered;
>>>   extern const struct option	record_options[];
>>>   static bool			no_callchain;
>>> +static bool			show_symbol_offset;
>>>
>>>   enum perf_output_field {
>>>   	PERF_OUTPUT_COMM            = 1U<<  0,
>>> @@ -324,6 +325,7 @@ static void print_sample_addr(union perf_event *event,
>>>   	struct addr_location al;
>>>   	u8 cpumode = event->header.misc&  PERF_RECORD_MISC_CPUMODE_MASK;
>>>   	const char *symname, *dsoname;
>>> +	unsigned long offset;
>>>
>>>   	printf("%16" PRIx64, sample->addr);
>>>
>>> @@ -346,12 +348,15 @@ static void print_sample_addr(union perf_event *event,
>>>   		al.sym = map__find_symbol(al.map, al.addr, NULL);
>>>
>>>   	if (PRINT_FIELD(SYM)) {
>>> -		if (al.sym&&  al.sym->name)
>>> +		printf(" ");
>>> +		if (al.sym&&  al.sym->name) {
>>>   			symname = al.sym->name;
>>> -		else
>>> -			symname = "";
>>
>> dropping the else path will cause columns to misalign when ip's do not
>> resolve.
I see.
I'd like to add a magic word that indicates perf-script failed to resolve the symbol.
For example, '[unknown]'.

>>
>>> -
>>> -		printf(" %16s", symname);
>>> +			offset = al.addr - al.sym->start;
>>
>> move the offset calculation under the if (show_symbol_offset)
OK.

>>
>>> +			if (show_symbol_offset)
>>> +				printf("%16s+0x%lx", symname, offset);
>>> +			else
>>> +				printf("%16s", symname);
>>> +		}
>>>   	}
>>>
>>>   	if (PRINT_FIELD(DSO)) {
>>> @@ -390,7 +395,8 @@ static void process_event(union perf_event *event __unused,
>>>   		else
>>>   			printf("\n");
>>>   		perf_session__print_ip(event, sample, session,
>>> -					      PRINT_FIELD(SYM), PRINT_FIELD(DSO));
>>> +				       PRINT_FIELD(SYM), PRINT_FIELD(DSO),
>>> +				       show_symbol_offset);
>>>   	}
>>>
>>>   	printf("\n");
>>> @@ -1084,6 +1090,8 @@ static const struct option options[] = {
>>>   	OPT_CALLBACK('f', "fields", NULL, "str",
>>>   		     "comma separated output fields prepend with 'type:'. Valid types: hw,sw,trace,raw. Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,addr",
>>>   		     parse_output_fields),
>>> +	OPT_BOOLEAN('\0', "show-symbol-offset",&show_symbol_offset,
>>> +		    "Show the offset of the symbol."),
>>>
>>>   	OPT_END()
>>>   };
>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>> index 632266f..bb522ed 100644
>>> --- a/tools/perf/util/session.c
>>> +++ b/tools/perf/util/session.c
>>> @@ -1205,10 +1205,11 @@ struct perf_evsel *perf_session__find_first_evtype(struct perf_session *session,
>>>   void perf_session__print_ip(union perf_event *event,
>>>   			    struct perf_sample *sample,
>>>   			    struct perf_session *session,
>>> -			    int print_sym, int print_dso)
>>> +			    int print_sym, int print_dso, int print_offset)
>>>   {
>>>   	struct addr_location al;
>>>   	const char *symname, *dsoname;
>>> +	unsigned long offset;
>>>   	struct callchain_cursor *cursor =&session->callchain_cursor;
>>>   	struct callchain_cursor_node *node;
>>>
>>> @@ -1259,12 +1260,15 @@ void perf_session__print_ip(union perf_event *event,
>>>   	} else {
>>>   		printf("%16" PRIx64, sample->ip);
>>>   		if (print_sym) {
>>> -			if (al.sym&&  al.sym->name)
>>> +			printf(" ");
>>> +			if (al.sym&&  al.sym->name) {
>>>   				symname = al.sym->name;
>>> -			else
>>> -				symname = "";
>>> -
>>> -			printf(" %s", symname);
>>> +				offset = al.addr - al.sym->start;
>>> +				if (print_offset)
>>> +					printf("%s+0x%lx", symname, offset);
>>
>> same comments for this code path.
OK. I'm going to move the code.


Thank you.

>>
>> David
>>
>>
>>> +				else
>>> +					printf("%s", symname);
>>> +			}
>>>   		}
>>>
>>>   		if (print_dso) {
>>> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
>>> index de4178d..8187e68 100644
>>> --- a/tools/perf/util/session.h
>>> +++ b/tools/perf/util/session.h
>>> @@ -170,6 +170,7 @@ struct perf_evsel *perf_session__find_first_evtype(struct perf_session *session,
>>>   void perf_session__print_ip(union perf_event *event,
>>>   				 struct perf_sample *sample,
>>>   				 struct perf_session *session,
>>> -				 int print_sym, int print_dso);
>>> +				 int print_sym, int print_dso,
>>> +				 int print_offset);
>>>
>>>   #endif /* __PERF_SESSION_H */
>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ