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 <wangn...@huawei.com> Cc: Arnaldo Carvalho de Melo <a...@redhat.com> Cc: Namhyung Kim <namhy...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
