Before this patch we can trigger a segfault by following steps:
Step 1: perf report
Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
Step 3: Use '/' to filter symbols, use a filter which returns
empty result
Step 4: Press 'ENTER' (notice here that the old selection is still
there. This is another problem)
Step 5: Press 'ENTER' to annotate that symbol
Step 6: Press 'LEFT' to go out.
Result: segfault:
perf: Segmentation fault
-------- backtrace --------
/home/wangnan/perf[0x53e568]
/lib64/libc.so.6(+0x3545f)[0x7fba75d3245f]
/home/wangnan/perf[0x537516]
/home/wangnan/perf[0x533fef]
/home/wangnan/perf[0x53b347]
/home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d206]
/home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
/home/wangnan/perf[0x47efa2]
/home/wangnan/perf(main+0x5f5)[0x432fa5]
/lib64/libc.so.6(__libc_start_main+0xf4)[0x7fba75d1ebd4]
/home/wangnan/perf[0x4330d4]
This is because in this case 'nd' could be NULL in
ui_browser__hists_seek(), but that function never check it.
This patch adds checker for potential NULL pointer in that function.
After this patch the above steps won't segfault again.
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Namhyung Kim <[email protected]>
---
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 33da341..9458da8 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1280,8 +1280,10 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
* Moves not relative to the first visible entry invalidates its
* row_offset:
*/
- h = rb_entry(browser->top, struct hist_entry, rb_node);
- h->row_offset = 0;
+ if (browser->top) {
+ h = rb_entry(browser->top, struct hist_entry, rb_node);
+ h->row_offset = 0;
+ }
/*
* Here we have to check if nd is expanded (+), if it is we can't go
@@ -1297,6 +1299,8 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
* and stop when we printed enough lines to fill the screen.
*/
do_offset:
+ if (!nd)
+ return;
if (offset > 0) {
do {
h = rb_entry(nd, struct hist_entry, rb_node);
--
1.8.3.4
With following steps:
Step 1: perf report
Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
Step 3: Use '/' to filter symbols, use a filter which returns
empty result
Step 4: Press 'ENTER'
We see that, even if we have filter all symbols (and the main interface
is empty), pressing 'ENTER' still select one symbol. This behavior
surprise user. This patch resets browser->selection in
hist_browser__refresh() and let it choose default selection. In this
case browser->selection keeps NULL so user won't see annotation item
in menu.
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Namhyung Kim <[email protected]>
---
Note that if this patch is applied before 1/2 then the steps listed in
commit message in 1/2 won't trigger segfault. However I believe patch 1/1
is still required.
---
tools/perf/ui/browsers/hists.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 9458da8..523a9ef 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1192,6 +1192,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
}
ui_browser__hists_init_top(browser);
+ hb->selection = NULL;
for (nd = browser->top; nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
@@ -2102,7 +2103,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (browser->he_selection != NULL) {
thread = hist_browser__selected_thread(browser);
- map = browser->selection->map;
+ map = browser->selection ? browser->selection->map : NULL;
socked_id = browser->he_selection->socket;
}
switch (key) {
@@ -2321,7 +2322,8 @@ 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
On Wed, Dec 02, 2015 at 12:51:32PM +0000, Wang Nan wrote:
> Before this patch we can trigger a segfault by following steps:
>
> Step 1: perf report
>
> Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
>
> Step 3: Use '/' to filter symbols, use a filter which returns
> empty result
>
> Step 4: Press 'ENTER' (notice here that the old selection is still
> there. This is another problem)
I guess your data file doesn't contain callchains. Otherwise 'ENTER'
key will toggle it and not show context menu. We now support 'm' key
to show context menu in any case.
>
> Step 5: Press 'ENTER' to annotate that symbol
>
> Step 6: Press 'LEFT' to go out.
>
> Result: segfault:
>
> perf: Segmentation fault
> -------- backtrace --------
> /home/wangnan/perf[0x53e568]
> /lib64/libc.so.6(+0x3545f)[0x7fba75d3245f]
> /home/wangnan/perf[0x537516]
> /home/wangnan/perf[0x533fef]
> /home/wangnan/perf[0x53b347]
> /home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d206]
> /home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
> /home/wangnan/perf[0x47efa2]
> /home/wangnan/perf(main+0x5f5)[0x432fa5]
> /lib64/libc.so.6(__libc_start_main+0xf4)[0x7fba75d1ebd4]
> /home/wangnan/perf[0x4330d4]
>
> This is because in this case 'nd' could be NULL in
> ui_browser__hists_seek(), but that function never check it.
>
> This patch adds checker for potential NULL pointer in that function.
> After this patch the above steps won't segfault again.
>
> Signed-off-by: Wang Nan <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> ---
> 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 33da341..9458da8 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -1280,8 +1280,10 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
> * Moves not relative to the first visible entry invalidates its
> * row_offset:
> */
> - h = rb_entry(browser->top, struct hist_entry, rb_node);
> - h->row_offset = 0;
> + if (browser->top) {
> + h = rb_entry(browser->top, struct hist_entry, rb_node);
> + h->row_offset = 0;
> + }
Did you have a problem with this code? It seems that
ui_browser__hists_init_top() ensures browser->top is initialized.
>
> /*
> * Here we have to check if nd is expanded (+), if it is we can't go
> @@ -1297,6 +1299,8 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
> * and stop when we printed enough lines to fill the screen.
> */
> do_offset:
> + if (!nd)
> + return;
So I guess checking 'nd' being NULL here is enough. And please add a
blank line for readability.
Thanks,
Namhyung
> if (offset > 0) {
> do {
> h = rb_entry(nd, struct hist_entry, rb_node);
> --
> 1.8.3.4
>
On Wed, Dec 02, 2015 at 12:51:33PM +0000, Wang Nan wrote:
> With following steps:
>
> Step 1: perf report
>
> Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
>
> Step 3: Use '/' to filter symbols, use a filter which returns
> empty result
>
> Step 4: Press 'ENTER'
>
> We see that, even if we have filter all symbols (and the main interface
> is empty), pressing 'ENTER' still select one symbol. This behavior
> surprise user. This patch resets browser->selection in
> hist_browser__refresh() and let it choose default selection. In this
> case browser->selection keeps NULL so user won't see annotation item
> in menu.
>
> Signed-off-by: Wang Nan <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> ---
>
> Note that if this patch is applied before 1/2 then the steps listed in
> commit message in 1/2 won't trigger segfault. However I believe patch 1/1
> is still required.
>
> ---
> tools/perf/ui/browsers/hists.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 9458da8..523a9ef 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -1192,6 +1192,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
> }
>
> ui_browser__hists_init_top(browser);
> + hb->selection = NULL;
This code assumes that hb->selection is not NULL if hb->he_selection
is not NULL. So I think that the right (and simple) fix is to reset
hb->he_selection rather than hb->selection (or both). It'll make the
change below unnecessary IMHO.
Thanks,
Namhyung
>
> for (nd = browser->top; nd; nd = rb_next(nd)) {
> struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
> @@ -2102,7 +2103,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>
> if (browser->he_selection != NULL) {
> thread = hist_browser__selected_thread(browser);
> - map = browser->selection->map;
> + map = browser->selection ? browser->selection->map : NULL;
> socked_id = browser->he_selection->socket;
> }
> switch (key) {
> @@ -2321,7 +2322,8 @@ 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
>
On 2015/12/2 23:59, Namhyung Kim wrote:
> On Wed, Dec 02, 2015 at 12:51:32PM +0000, Wang Nan wrote:
>> Before this patch we can trigger a segfault by following steps:
>>
>> Step 1: perf report
>>
>> Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
>>
>> Step 3: Use '/' to filter symbols, use a filter which returns
>> empty result
>>
>> Step 4: Press 'ENTER' (notice here that the old selection is still
>> there. This is another problem)
> I guess your data file doesn't contain callchains. Otherwise 'ENTER'
> key will toggle it and not show context menu.
Yes. With callchain information, this step should be replaced
by:
Toggle an entry and select a leaf entry which can be annotated
with UP and DOWN, don't press 'ENTER'.
I get similar result.
> We now support 'm' key
> to show context menu in any case.
>
>
>> Step 5: Press 'ENTER' to annotate that symbol
>>
>> Step 6: Press 'LEFT' to go out.
>>
>> Result: segfault:
>>
>> perf: Segmentation fault
>> -------- backtrace --------
>> /home/wangnan/perf[0x53e568]
>> /lib64/libc.so.6(+0x3545f)[0x7fba75d3245f]
>> /home/wangnan/perf[0x537516]
>> /home/wangnan/perf[0x533fef]
>> /home/wangnan/perf[0x53b347]
>> /home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d206]
>> /home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
>> /home/wangnan/perf[0x47efa2]
>> /home/wangnan/perf(main+0x5f5)[0x432fa5]
>> /lib64/libc.so.6(__libc_start_main+0xf4)[0x7fba75d1ebd4]
>> /home/wangnan/perf[0x4330d4]
>>
>> This is because in this case 'nd' could be NULL in
>> ui_browser__hists_seek(), but that function never check it.
>>
>> This patch adds checker for potential NULL pointer in that function.
>> After this patch the above steps won't segfault again.
>>
>> Signed-off-by: Wang Nan <[email protected]>
>> Cc: Arnaldo Carvalho de Melo <[email protected]>
>> Cc: Namhyung Kim <[email protected]>
>> ---
>> 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 33da341..9458da8 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -1280,8 +1280,10 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
>> * Moves not relative to the first visible entry invalidates its
>> * row_offset:
>> */
>> - h = rb_entry(browser->top, struct hist_entry, rb_node);
>> - h->row_offset = 0;
>> + if (browser->top) {
>> + h = rb_entry(browser->top, struct hist_entry, rb_node);
>> + h->row_offset = 0;
>> + }
> Did you have a problem with this code? It seems that
> ui_browser__hists_init_top() ensures browser->top is initialized.
>
No. Add it for safty because I see in following code that browser->top
can be NULL.
Will be removed in next version.
Thank you.
On 2015/12/3 0:17, Namhyung Kim wrote:
> On Wed, Dec 02, 2015 at 12:51:33PM +0000, Wang Nan wrote:
>> With following steps:
>>
>> Step 1: perf report
>>
>> Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
>>
>> Step 3: Use '/' to filter symbols, use a filter which returns
>> empty result
>>
>> Step 4: Press 'ENTER'
>>
>> We see that, even if we have filter all symbols (and the main interface
>> is empty), pressing 'ENTER' still select one symbol. This behavior
>> surprise user. This patch resets browser->selection in
>> hist_browser__refresh() and let it choose default selection. In this
>> case browser->selection keeps NULL so user won't see annotation item
>> in menu.
>>
>> Signed-off-by: Wang Nan <[email protected]>
>> Cc: Arnaldo Carvalho de Melo <[email protected]>
>> Cc: Namhyung Kim <[email protected]>
>> ---
>>
>> Note that if this patch is applied before 1/2 then the steps listed in
>> commit message in 1/2 won't trigger segfault. However I believe patch 1/1
>> is still required.
>>
>> ---
>> tools/perf/ui/browsers/hists.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index 9458da8..523a9ef 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -1192,6 +1192,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
>> }
>>
>> ui_browser__hists_init_top(browser);
>> + hb->selection = NULL;
> This code assumes that hb->selection is not NULL if hb->he_selection
> is not NULL. So I think that the right (and simple) fix is to reset
> hb->he_selection rather than hb->selection (or both). It'll make the
> change below unnecessary IMHO.
No. Only set hb->he_selection causes crash:
Step 0: user 'perf record ls' create a perf.data without callchain;
Step 1: perf report
Step 2: choose a annotable entry, don't press 'ENTER'
Step 3: use '/' and enter a filter like 'asdfasdf' which ensure no entry
would be left
Step 3: Press 'ENTER' twice
Crash:
# ./perf report
perf: Segmentation fault
-------- backtrace --------
./perf[0x53e588]
/tmp/oxygen_root/lib64/libc.so.6(+0x3545f)[0x7f2b4d6da45f]
./perf[0x539e03]
./perf(perf_evlist__tui_browse_hists+0x96)[0x53d226]
./perf(cmd_report+0x1b9f)[0x442c7f]
./perf[0x47efc2]
./perf(main+0x5f5)[0x432fa5]
/tmp/oxygen_root/lib64/libc.so.6(__libc_start_main+0xf4)[0x7f2b4d6c6bd4]
./perf[0x4330d4]
GDB result:
Program received signal SIGSEGV, Segmentation fault.
hist_browser__toggle_fold (browser=0x1f71160) at ui/browsers/hists.c:352
(gdb) bt
#0 hist_browser__toggle_fold (browser=0x1f71160) at ui/browsers/hists.c:352
#1 hist_browser__run (help=0x649038 "For a higher level overview, try:
perf report --sort comm,dso", browser=0x1f71160) at ui/
browsers/hists.c:539
#2 perf_evsel__hists_browse (evsel=0x19ef140,
nr_events=nr_events@entry=1, helpline=helpline@entry=0x649038 "For a
higher leve
l overview, try: perf report --sort comm,dso",
left_exits=left_exits@entry=false, hbt=hbt@entry=0x0,
min_pcnt=<optimized out>,
env=0x19ed850) at ui/browsers/hists.c:2101
#3 0x000000000053d227 in perf_evlist__tui_browse_hists
(evlist=0x19ee730, help=help@entry=0x649038 "For a higher level overvie
w, try: perf report --sort comm,dso", hbt=hbt@entry=0x0,
min_pcnt=<optimized out>, env=<optimized out>) at ui/browsers/hists.c:
2555
#4 0x0000000000442c80 in report__browse_hists (rep=0x7fffffffca20) at
builtin-report.c:440
#5 __cmd_report (rep=0x7fffffffca20) at builtin-report.c:576
#6 cmd_report (argc=0, argv=0x7fffffffe590, prefix=<optimized out>) at
builtin-report.c:962
#7 0x000000000047efc3 in run_builtin (p=p@entry=0x8ff9e0
<commands+192>, argc=argc@entry=1, argv=argv@entry=0x7fffffffe590) at
perf.c:387
#8 0x0000000000432fa6 in handle_internal_command (argv=0x7fffffffe590,
argc=1) at perf.c:448
#9 run_argv (argv=0x7fffffffe310, argcp=0x7fffffffe31c) at perf.c:492
#10 main (argc=1, argv=0x7fffffffe590) at perf.c:609
But setting both of them to NULL in hist_browser__refresh() can avoid
this crash.
So we have two choice:
1. In hist_browser__refresh() let's set both selection and he_selection
to NULL;
By this way after step 3 we won't see annotation options. The
context menu
(by pressing ENTER or 'm') is:
Run scripts for all samples
Switch to another data file in PWD
Exit
2. In hist_browser__toggle_fold() let's check both he amd ms.
By this way we still get annotation and map options in context menu:
Annotate __strcoll_l
Browse map details
Run scripts for all samples
Switch to another data file in PWD
Exit
I'm not sure which one is better because I don't really understand this
part of
code. But for me the second result is surprising because I have filtered all
entries out in my interface, and I believe I should select nothing, so
pressing
'ENTER' or 'm' I shall not get annotation option because I don't know
which entry
would be annotated.
Thank you.
Hi,
On Thu, Dec 03, 2015 at 11:05:13AM +0800, Wangnan (F) wrote:
> On 2015/12/3 0:17, Namhyung Kim wrote:
> >On Wed, Dec 02, 2015 at 12:51:33PM +0000, Wang Nan wrote:
> >>With following steps:
> >>
> >> Step 1: perf report
> >>
> >> Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
> >>
> >> Step 3: Use '/' to filter symbols, use a filter which returns
> >> empty result
> >>
> >> Step 4: Press 'ENTER'
> >>
> >>We see that, even if we have filter all symbols (and the main interface
> >>is empty), pressing 'ENTER' still select one symbol. This behavior
> >>surprise user. This patch resets browser->selection in
> >>hist_browser__refresh() and let it choose default selection. In this
> >>case browser->selection keeps NULL so user won't see annotation item
> >>in menu.
> >>
> >>Signed-off-by: Wang Nan <[email protected]>
> >>Cc: Arnaldo Carvalho de Melo <[email protected]>
> >>Cc: Namhyung Kim <[email protected]>
> >>---
> >>
> >>Note that if this patch is applied before 1/2 then the steps listed in
> >>commit message in 1/2 won't trigger segfault. However I believe patch 1/1
> >>is still required.
> >>
> >>---
> >> tools/perf/ui/browsers/hists.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> >>index 9458da8..523a9ef 100644
> >>--- a/tools/perf/ui/browsers/hists.c
> >>+++ b/tools/perf/ui/browsers/hists.c
> >>@@ -1192,6 +1192,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
> >> }
> >> ui_browser__hists_init_top(browser);
> >>+ hb->selection = NULL;
> >This code assumes that hb->selection is not NULL if hb->he_selection
> >is not NULL. So I think that the right (and simple) fix is to reset
> >hb->he_selection rather than hb->selection (or both). It'll make the
> >change below unnecessary IMHO.
>
> No. Only set hb->he_selection causes crash:
>
> Step 0: user 'perf record ls' create a perf.data without callchain;
> Step 1: perf report
> Step 2: choose a annotable entry, don't press 'ENTER'
> Step 3: use '/' and enter a filter like 'asdfasdf' which ensure no entry
> would be left
> Step 3: Press 'ENTER' twice
>
> Crash:
> # ./perf report
> perf: Segmentation fault
> -------- backtrace --------
> ./perf[0x53e588]
> /tmp/oxygen_root/lib64/libc.so.6(+0x3545f)[0x7f2b4d6da45f]
> ./perf[0x539e03]
> ./perf(perf_evlist__tui_browse_hists+0x96)[0x53d226]
> ./perf(cmd_report+0x1b9f)[0x442c7f]
> ./perf[0x47efc2]
> ./perf(main+0x5f5)[0x432fa5]
> /tmp/oxygen_root/lib64/libc.so.6(__libc_start_main+0xf4)[0x7f2b4d6c6bd4]
> ./perf[0x4330d4]
>
>
> GDB result:
>
> Program received signal SIGSEGV, Segmentation fault.
> hist_browser__toggle_fold (browser=0x1f71160) at ui/browsers/hists.c:352
So this is same crash you already fixed. Now I think that it'd be
better to check hb->he_selection instead of hb->selection in that
patch for the sake of consistency.
> (gdb) bt
> #0 hist_browser__toggle_fold (browser=0x1f71160) at ui/browsers/hists.c:352
> #1 hist_browser__run (help=0x649038 "For a higher level overview, try: perf
> report --sort comm,dso", browser=0x1f71160) at ui/
> browsers/hists.c:539
> #2 perf_evsel__hists_browse (evsel=0x19ef140, nr_events=nr_events@entry=1,
> helpline=helpline@entry=0x649038 "For a higher leve
> l overview, try: perf report --sort comm,dso",
> left_exits=left_exits@entry=false, hbt=hbt@entry=0x0, min_pcnt=<optimized
> out>,
> env=0x19ed850) at ui/browsers/hists.c:2101
> #3 0x000000000053d227 in perf_evlist__tui_browse_hists (evlist=0x19ee730,
> help=help@entry=0x649038 "For a higher level overvie
> w, try: perf report --sort comm,dso", hbt=hbt@entry=0x0, min_pcnt=<optimized
> out>, env=<optimized out>) at ui/browsers/hists.c:
> 2555
> #4 0x0000000000442c80 in report__browse_hists (rep=0x7fffffffca20) at
> builtin-report.c:440
> #5 __cmd_report (rep=0x7fffffffca20) at builtin-report.c:576
> #6 cmd_report (argc=0, argv=0x7fffffffe590, prefix=<optimized out>) at
> builtin-report.c:962
> #7 0x000000000047efc3 in run_builtin (p=p@entry=0x8ff9e0 <commands+192>,
> argc=argc@entry=1, argv=argv@entry=0x7fffffffe590) at
> perf.c:387
> #8 0x0000000000432fa6 in handle_internal_command (argv=0x7fffffffe590,
> argc=1) at perf.c:448
> #9 run_argv (argv=0x7fffffffe310, argcp=0x7fffffffe31c) at perf.c:492
> #10 main (argc=1, argv=0x7fffffffe590) at perf.c:609
>
> But setting both of them to NULL in hist_browser__refresh() can avoid this
> crash.
>
> So we have two choice:
>
> 1. In hist_browser__refresh() let's set both selection and he_selection to
> NULL;
>
> By this way after step 3 we won't see annotation options. The context
> menu
> (by pressing ENTER or 'm') is:
>
> Run scripts for all samples
> Switch to another data file in PWD
> Exit
>
>
> 2. In hist_browser__toggle_fold() let's check both he amd ms.
>
> By this way we still get annotation and map options in context menu:
>
> Annotate __strcoll_l
> Browse map details
> Run scripts for all samples
> Switch to another data file in PWD
> Exit
>
> I'm not sure which one is better because I don't really understand this part
> of
> code. But for me the second result is surprising because I have filtered all
> entries out in my interface, and I believe I should select nothing, so
> pressing
> 'ENTER' or 'm' I shall not get annotation option because I don't know which
> entry
> would be annotated.
Agreed. I also think that the choice 1 is the right thing.
Thanks,
Namhyung