| Message ID | 1449060693-236232-1-git-send-email-wangnan0@huawei.com |
|---|---|
| State | New |
| Headers | show |
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 <wangnan0@huawei.com> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >> 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 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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);
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 <wangnan0@huawei.com> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> --- tools/perf/ui/browsers/hists.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) -- 1.8.3.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/