* [PATCH] perf tools: Fix segfault in 'perf top'
@ 2015-09-11 4:59 Wang Nan
2015-09-11 16:22 ` Namhyung Kim
0 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2015-09-11 4:59 UTC (permalink / raw)
To: acme; +Cc: lizefan, linux-kernel, pi3orama, Wang Nan, Namhyung Kim
'perf top' segfaults with following operation:
# perf top -e page-faults -p 15173 # 15173 never generate page-fault
Then on the resulting empty interface, press right key:
# ./perf top -e page-faults -p 11400
perf: Segmentation fault
-------- backtrace --------
./perf[0x535428]
/lib64/libc.so.6(+0x3545f)[0x7f0dd360745f]
./perf[0x531d46]
./perf(perf_evlist__tui_browse_hists+0x96)[0x5340d6]
./perf[0x44ba2f]
/lib64/libpthread.so.0(+0x81d0)[0x7f0dd49dc1d0]
/lib64/libc.so.6(clone+0x6c)[0x7f0dd36b90dc]
The bug reside in perf_evsel__hists_browse() that, in the above circumstance
browser->selection can be NULL, but code after skip_annotation doesn't consider
it.
This patch fix it by checing browser->selection before fetching
browser->selection->map and browser->selection->sym.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e4fd40f..b00fa92 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2017,7 +2017,8 @@ skip_annotation:
&options[nr_options], dso);
nr_options += add_map_opt(browser, &actions[nr_options],
&options[nr_options],
- browser->selection->map);
+ browser->selection ?
+ browser->selection->map : NULL);
nr_options += add_socket_opt(browser, &actions[nr_options],
&options[nr_options],
socket);
@@ -2030,7 +2031,10 @@ skip_annotation:
nr_options += add_script_opt(browser,
&actions[nr_options],
&options[nr_options],
- NULL, browser->selection->sym);
+ NULL,
+ browser->selection ?
+ browser->selection->sym :
+ NULL);
}
nr_options += add_script_opt(browser, &actions[nr_options],
&options[nr_options], NULL, NULL);
--
1.8.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tools: Fix segfault in 'perf top'
2015-09-11 4:59 [PATCH] perf tools: Fix segfault in 'perf top' Wang Nan
@ 2015-09-11 16:22 ` Namhyung Kim
2015-09-14 10:15 ` Wangnan (F)
2015-09-14 10:23 ` [PATCH v2] " Wang Nan
0 siblings, 2 replies; 7+ messages in thread
From: Namhyung Kim @ 2015-09-11 16:22 UTC (permalink / raw)
To: Wang Nan; +Cc: acme, lizefan, linux-kernel, pi3orama
On Fri, Sep 11, 2015 at 04:59:11AM +0000, Wang Nan wrote:
> 'perf top' segfaults with following operation:
>
> # perf top -e page-faults -p 15173 # 15173 never generate page-fault
>
> Then on the resulting empty interface, press right key:
>
> # ./perf top -e page-faults -p 11400
> perf: Segmentation fault
> -------- backtrace --------
> ./perf[0x535428]
> /lib64/libc.so.6(+0x3545f)[0x7f0dd360745f]
> ./perf[0x531d46]
> ./perf(perf_evlist__tui_browse_hists+0x96)[0x5340d6]
> ./perf[0x44ba2f]
> /lib64/libpthread.so.0(+0x81d0)[0x7f0dd49dc1d0]
> /lib64/libc.so.6(clone+0x6c)[0x7f0dd36b90dc]
>
> The bug reside in perf_evsel__hists_browse() that, in the above circumstance
> browser->selection can be NULL, but code after skip_annotation doesn't consider
> it.
>
> This patch fix it by checing browser->selection before fetching
> browser->selection->map and browser->selection->sym.
I'm ok with this change, just a nitpick below..
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/hists.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index e4fd40f..b00fa92 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2017,7 +2017,8 @@ skip_annotation:
> &options[nr_options], dso);
> nr_options += add_map_opt(browser, &actions[nr_options],
> &options[nr_options],
> - browser->selection->map);
> + browser->selection ?
> + browser->selection->map : NULL);
> nr_options += add_socket_opt(browser, &actions[nr_options],
> &options[nr_options],
> socket);
> @@ -2030,7 +2031,10 @@ skip_annotation:
> nr_options += add_script_opt(browser,
> &actions[nr_options],
> &options[nr_options],
> - NULL, browser->selection->sym);
> + NULL,
> + browser->selection ?
> + browser->selection->sym :
> + NULL);
Is this really needed? IIUC browser->selection is not NULL if
browser->he_selection is not NULL. Did you see a case it's NULL?
Also passing a NULL thread and a NULL sym is meaningless here since
it's a duplication..
Thanks,
Namhyung
> }
> nr_options += add_script_opt(browser, &actions[nr_options],
> &options[nr_options], NULL, NULL);
> --
> 1.8.3.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tools: Fix segfault in 'perf top'
2015-09-11 16:22 ` Namhyung Kim
@ 2015-09-14 10:15 ` Wangnan (F)
2015-09-14 10:23 ` [PATCH v2] " Wang Nan
1 sibling, 0 replies; 7+ messages in thread
From: Wangnan (F) @ 2015-09-14 10:15 UTC (permalink / raw)
To: Namhyung Kim; +Cc: acme, lizefan, linux-kernel, pi3orama
On 2015/9/12 0:22, Namhyung Kim wrote:
> On Fri, Sep 11, 2015 at 04:59:11AM +0000, Wang Nan wrote:
>> 'perf top' segfaults with following operation:
>>
>> # perf top -e page-faults -p 15173 # 15173 never generate page-fault
>>
>> Then on the resulting empty interface, press right key:
>>
>> # ./perf top -e page-faults -p 11400
>> perf: Segmentation fault
>> -------- backtrace --------
>> ./perf[0x535428]
>> /lib64/libc.so.6(+0x3545f)[0x7f0dd360745f]
>> ./perf[0x531d46]
>> ./perf(perf_evlist__tui_browse_hists+0x96)[0x5340d6]
>> ./perf[0x44ba2f]
>> /lib64/libpthread.so.0(+0x81d0)[0x7f0dd49dc1d0]
>> /lib64/libc.so.6(clone+0x6c)[0x7f0dd36b90dc]
>>
>> The bug reside in perf_evsel__hists_browse() that, in the above circumstance
>> browser->selection can be NULL, but code after skip_annotation doesn't consider
>> it.
>>
>> This patch fix it by checing browser->selection before fetching
>> browser->selection->map and browser->selection->sym.
> I'm ok with this change, just a nitpick below..
>
>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> ---
>> tools/perf/ui/browsers/hists.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index e4fd40f..b00fa92 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -2017,7 +2017,8 @@ skip_annotation:
>> &options[nr_options], dso);
>> nr_options += add_map_opt(browser, &actions[nr_options],
>> &options[nr_options],
>> - browser->selection->map);
>> + browser->selection ?
>> + browser->selection->map : NULL);
>> nr_options += add_socket_opt(browser, &actions[nr_options],
>> &options[nr_options],
>> socket);
>> @@ -2030,7 +2031,10 @@ skip_annotation:
>> nr_options += add_script_opt(browser,
>> &actions[nr_options],
>> &options[nr_options],
>> - NULL, browser->selection->sym);
>> + NULL,
>> + browser->selection ?
>> + browser->selection->sym :
>> + NULL);
> Is this really needed? IIUC browser->selection is not NULL if
> browser->he_selection is not NULL. Did you see a case it's NULL?
> Also passing a NULL thread and a NULL sym is meaningless here since
> it's a duplication..
Actually I didn't see the case when browser->he_selection is not NULL but
browser->selection is NULL. I add the testing only for safety reason because
we have
if (browser->selection == NULL)
goto skip_annotation;
So I guess we'd better to check browser->selection in the codeblock
leading by
skip_annotation, so if someone breaks the assumption the code still work.
But yes, we can add the checker back when the assumption *does* be broken.
Currently let's add a comment here for code readers.
Please have a look for v2 patch.
Thank you.
> Thanks,
> Namhyung
>
>
>> }
>> nr_options += add_script_opt(browser, &actions[nr_options],
>> &options[nr_options], NULL, NULL);
>> --
>> 1.8.3.4
>>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] perf tools: Fix segfault in 'perf top'
2015-09-11 16:22 ` Namhyung Kim
2015-09-14 10:15 ` Wangnan (F)
@ 2015-09-14 10:23 ` Wang Nan
2015-09-14 18:07 ` Arnaldo Carvalho de Melo
2015-09-16 7:27 ` [tip:perf/core] perf top: Fix segfault pressing -> with no hist entries tip-bot for Wang Nan
1 sibling, 2 replies; 7+ messages in thread
From: Wang Nan @ 2015-09-14 10:23 UTC (permalink / raw)
To: acme, namhyung; +Cc: linux-kernel, lizefan, pi3orama, Wang Nan
'perf top' segfaults with following operation:
# perf top -e page-faults -p 11400 # 11400 never generates page-fault
Then on the resulting empty interface, press right key:
# ./perf top -e page-faults -p 11400
perf: Segmentation fault
-------- backtrace --------
./perf[0x535428]
/lib64/libc.so.6(+0x3545f)[0x7f0dd360745f]
./perf[0x531d46]
./perf(perf_evlist__tui_browse_hists+0x96)[0x5340d6]
./perf[0x44ba2f]
/lib64/libpthread.so.0(+0x81d0)[0x7f0dd49dc1d0]
/lib64/libc.so.6(clone+0x6c)[0x7f0dd36b90dc]
The bug reside in perf_evsel__hists_browse() that, in the above circumstance
browser->selection can be NULL, but code after skip_annotation doesn't consider
it.
This patch fix it by checking browser->selection before fetching
browser->selection->map.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e4fd40f..182b87a 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2017,7 +2017,8 @@ skip_annotation:
&options[nr_options], dso);
nr_options += add_map_opt(browser, &actions[nr_options],
&options[nr_options],
- browser->selection->map);
+ browser->selection ?
+ browser->selection->map : NULL);
nr_options += add_socket_opt(browser, &actions[nr_options],
&options[nr_options],
socket);
@@ -2027,6 +2028,15 @@ skip_annotation:
&actions[nr_options],
&options[nr_options],
thread, NULL);
+ /*
+ * Note that browser->selection != NULL
+ * when browser->he_selection is not NULL,
+ * so we don't need to check browser->selection
+ * before fetching browser->selection->sym like what
+ * we do before fetching browser->selection->map.
+ *
+ * See hist_browser__show_entry.
+ */
nr_options += add_script_opt(browser,
&actions[nr_options],
&options[nr_options],
--
1.8.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] perf tools: Fix segfault in 'perf top'
2015-09-14 10:23 ` [PATCH v2] " Wang Nan
@ 2015-09-14 18:07 ` Arnaldo Carvalho de Melo
2015-09-14 18:12 ` Arnaldo Carvalho de Melo
2015-09-16 7:27 ` [tip:perf/core] perf top: Fix segfault pressing -> with no hist entries tip-bot for Wang Nan
1 sibling, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-09-14 18:07 UTC (permalink / raw)
To: Wang Nan; +Cc: namhyung, linux-kernel, lizefan, pi3orama
Em Mon, Sep 14, 2015 at 10:23:55AM +0000, Wang Nan escreveu:
> 'perf top' segfaults with following operation:
>
> # perf top -e page-faults -p 11400 # 11400 never generates page-fault
>
> Then on the resulting empty interface, press right key:
So, this happens in perf/urgent, so we need to apply it there first,
which this patch doesn't.
I am fixing it up this time, thanks for the patch!
- Arnaldo
> # ./perf top -e page-faults -p 11400
> perf: Segmentation fault
> -------- backtrace --------
> ./perf[0x535428]
> /lib64/libc.so.6(+0x3545f)[0x7f0dd360745f]
> ./perf[0x531d46]
> ./perf(perf_evlist__tui_browse_hists+0x96)[0x5340d6]
> ./perf[0x44ba2f]
> /lib64/libpthread.so.0(+0x81d0)[0x7f0dd49dc1d0]
> /lib64/libc.so.6(clone+0x6c)[0x7f0dd36b90dc]
>
> The bug reside in perf_evsel__hists_browse() that, in the above circumstance
> browser->selection can be NULL, but code after skip_annotation doesn't consider
> it.
>
> This patch fix it by checking browser->selection before fetching
> browser->selection->map.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/hists.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index e4fd40f..182b87a 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2017,7 +2017,8 @@ skip_annotation:
> &options[nr_options], dso);
> nr_options += add_map_opt(browser, &actions[nr_options],
> &options[nr_options],
> - browser->selection->map);
> + browser->selection ?
> + browser->selection->map : NULL);
> nr_options += add_socket_opt(browser, &actions[nr_options],
> &options[nr_options],
> socket);
> @@ -2027,6 +2028,15 @@ skip_annotation:
> &actions[nr_options],
> &options[nr_options],
> thread, NULL);
> + /*
> + * Note that browser->selection != NULL
> + * when browser->he_selection is not NULL,
> + * so we don't need to check browser->selection
> + * before fetching browser->selection->sym like what
> + * we do before fetching browser->selection->map.
> + *
> + * See hist_browser__show_entry.
> + */
> nr_options += add_script_opt(browser,
> &actions[nr_options],
> &options[nr_options],
> --
> 1.8.3.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] perf tools: Fix segfault in 'perf top'
2015-09-14 18:07 ` Arnaldo Carvalho de Melo
@ 2015-09-14 18:12 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-09-14 18:12 UTC (permalink / raw)
To: Wang Nan; +Cc: namhyung, linux-kernel, lizefan, pi3orama
Em Mon, Sep 14, 2015 at 03:07:14PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Sep 14, 2015 at 10:23:55AM +0000, Wang Nan escreveu:
> > 'perf top' segfaults with following operation:
> >
> > # perf top -e page-faults -p 11400 # 11400 never generates page-fault
> >
> > Then on the resulting empty interface, press right key:
>
> So, this happens in perf/urgent, so we need to apply it there first,
> which this patch doesn't.
>
> I am fixing it up this time, thanks for the patch!
For reference, below is the resulting patch, pushed to acme/perf/urgent,
now checking if there are more patches to go together with this one...
- Arnaldo
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index cf86f2d3a5e7..c04c60d4863c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1968,7 +1968,8 @@ skip_annotation:
&options[nr_options], dso);
nr_options += add_map_opt(browser, &actions[nr_options],
&options[nr_options],
- browser->selection->map);
+ browser->selection ?
+ browser->selection->map : NULL);
/* perf script support */
if (browser->he_selection) {
@@ -1976,6 +1977,15 @@ skip_annotation:
&actions[nr_options],
&options[nr_options],
thread, NULL);
+ /*
+ * Note that browser->selection != NULL
+ * when browser->he_selection is not NULL,
+ * so we don't need to check browser->selection
+ * before fetching browser->selection->sym like what
+ * we do before fetching browser->selection->map.
+ *
+ * See hist_browser__show_entry.
+ */
nr_options += add_script_opt(browser,
&actions[nr_options],
&options[nr_options],
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:perf/core] perf top: Fix segfault pressing -> with no hist entries
2015-09-14 10:23 ` [PATCH v2] " Wang Nan
2015-09-14 18:07 ` Arnaldo Carvalho de Melo
@ 2015-09-16 7:27 ` tip-bot for Wang Nan
1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for Wang Nan @ 2015-09-16 7:27 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, hpa, lizefan, acme, linux-kernel, wangnan0, mingo, namhyung
Commit-ID: bd315aab8a3ab1bc7074774b89a5d8ec7c1ff7ab
Gitweb: http://git.kernel.org/tip/bd315aab8a3ab1bc7074774b89a5d8ec7c1ff7ab
Author: Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 14 Sep 2015 10:23:55 +0000
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Sep 2015 15:10:41 -0300
perf top: Fix segfault pressing -> with no hist entries
'perf top' segfaults with following operation:
# perf top -e page-faults -p 11400 # 11400 never generates page-fault
Then on the resulting empty interface, press right key:
# ./perf top -e page-faults -p 11400
perf: Segmentation fault
-------- backtrace --------
./perf[0x535428]
/lib64/libc.so.6(+0x3545f)[0x7f0dd360745f]
./perf[0x531d46]
./perf(perf_evlist__tui_browse_hists+0x96)[0x5340d6]
./perf[0x44ba2f]
/lib64/libpthread.so.0(+0x81d0)[0x7f0dd49dc1d0]
/lib64/libc.so.6(clone+0x6c)[0x7f0dd36b90dc]
The bug resides in perf_evsel__hists_browse() that, in the above
circumstance browser->selection can be NULL, but code after
skip_annotation doesn't consider it.
This patch fix it by checking browser->selection before fetching
browser->selection->map.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1442226235-117265-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index cf86f2d..c04c60d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1968,7 +1968,8 @@ skip_annotation:
&options[nr_options], dso);
nr_options += add_map_opt(browser, &actions[nr_options],
&options[nr_options],
- browser->selection->map);
+ browser->selection ?
+ browser->selection->map : NULL);
/* perf script support */
if (browser->he_selection) {
@@ -1976,6 +1977,15 @@ skip_annotation:
&actions[nr_options],
&options[nr_options],
thread, NULL);
+ /*
+ * Note that browser->selection != NULL
+ * when browser->he_selection is not NULL,
+ * so we don't need to check browser->selection
+ * before fetching browser->selection->sym like what
+ * we do before fetching browser->selection->map.
+ *
+ * See hist_browser__show_entry.
+ */
nr_options += add_script_opt(browser,
&actions[nr_options],
&options[nr_options],
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-09-16 7:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-11 4:59 [PATCH] perf tools: Fix segfault in 'perf top' Wang Nan
2015-09-11 16:22 ` Namhyung Kim
2015-09-14 10:15 ` Wangnan (F)
2015-09-14 10:23 ` [PATCH v2] " Wang Nan
2015-09-14 18:07 ` Arnaldo Carvalho de Melo
2015-09-14 18:12 ` Arnaldo Carvalho de Melo
2015-09-16 7:27 ` [tip:perf/core] perf top: Fix segfault pressing -> with no hist entries tip-bot for Wang Nan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.