Re: [PATCH] perf machine: Initialize srcline string member in add_location struct
From: Michael Petlan <hidden>
Date: 2021-09-13 15:13:47
On Thu, 9 Sep 2021, Arnaldo Carvalho de Melo wrote:
Em Mon, Jul 19, 2021 at 04:53:32PM +0200, Michael Petlan escreveu:quoted
It's later supposed to be either a correct address or NULL. Without the initialization, it may contain an undefined value which results in the following segmentation fault: # perf top --sort comm -g --ignore-callees=do_idleLater where? The backtrace below is unresolved, I couldn't reproduce it here, that al variable is local to add_callchain_ip(), and is then passed to several places where I couldn't find al->srcline being used, can you state where it is used and causes the segfault?
Here is a better backtrace of a segfault:
#0 0x00007ffff56b7685 in __strlen_avx2 () from /lib64/libc.so.6
#1 0x00007ffff55e3802 in strdup () from /lib64/libc.so.6
#2 0x00005555558cb139 in hist_entry__init (callchain_size=<optimized out>, sample_self=true, template=0x7fffde7fb110, he=0x7fffd801c250) at util/hist.c:489
#3 hist_entry__new (template=template@entry=0x7fffde7fb110, sample_self=sample_self@entry=true) at util/hist.c:564
#4 0x00005555558cb4ba in hists__findnew_entry (hists=hists@entry=0x5555561d9e38, entry=entry@entry=0x7fffde7fb110, al=al@entry=0x7fffde7fb420,
sample_self=sample_self@entry=true) at util/hist.c:657
#5 0x00005555558cba1b in __hists__add_entry (hists=hists@entry=0x5555561d9e38, al=0x7fffde7fb420, sym_parent=<optimized out>, bi=bi@entry=0x0, mi=mi@entry=0x0,
sample=sample@entry=0x7fffde7fb4b0, sample_self=true, ops=0x0, block_info=0x0) at util/hist.c:288
#6 0x00005555558cbb70 in hists__add_entry (sample_self=true, sample=0x7fffde7fb4b0, mi=0x0, bi=0x0, sym_parent=<optimized out>, al=<optimized out>, hists=0x5555561d9e38)
at util/hist.c:1056
#7 iter_add_single_cumulative_entry (iter=0x7fffde7fb460, al=<optimized out>) at util/hist.c:1056
#8 0x00005555558cc8a4 in hist_entry_iter__add (iter=iter@entry=0x7fffde7fb460, al=al@entry=0x7fffde7fb420, max_stack_depth=<optimized out>, arg=arg@entry=0x7fffffff7db0)
at util/hist.c:1231
#9 0x00005555557cdc9a in perf_event__process_sample (machine=<optimized out>, sample=0x7fffde7fb4b0, evsel=<optimized out>, event=<optimized out>, tool=0x7fffffff7db0)
at builtin-top.c:842
#10 deliver_event (qe=<optimized out>, qevent=<optimized out>) at builtin-top.c:1202
#11 0x00005555558a9318 in do_flush (show_progress=false, oe=0x7fffffff80e0) at util/ordered-events.c:244
#12 __ordered_events__flush (oe=oe@entry=0x7fffffff80e0, how=how@entry=OE_FLUSH__TOP, timestamp=timestamp@entry=0) at util/ordered-events.c:323
#13 0x00005555558a9789 in __ordered_events__flush (timestamp=<optimized out>, how=<optimized out>, oe=<optimized out>) at util/ordered-events.c:339
#14 ordered_events__flush (how=OE_FLUSH__TOP, oe=0x7fffffff80e0) at util/ordered-events.c:341
#15 ordered_events__flush (oe=oe@entry=0x7fffffff80e0, how=how@entry=OE_FLUSH__TOP) at util/ordered-events.c:339
#16 0x00005555557cd631 in process_thread (arg=0x7fffffff7db0) at builtin-top.c:1114
#17 0x00007ffff7bb817a in start_thread () from /lib64/libpthread.so.0
#18 0x00007ffff5656dc3 in clone () from /lib64/libc.so.6
If you look at the frame #2, the code is:
488 if (he->srcline) {
489 he->srcline = strdup(he->srcline);
490 if (he->srcline == NULL)
491 goto err_rawdata;
492 }
If he->srcline is not NULL (it is not NULL if it is uninitialized rubbish),
it gets strdupped and strdupping a rubbish random string causes the problem.
Also, if you look at the commit 1fb7d06a509e, it adds the srcline property
into the struct, but not initializing it everywhere needed.
Michael
- Arnaldoquoted
terminates with perf: Segmentation fault -------- backtrace -------- perf(+0x417b26)[0x557794f1fb26] /lib64/libc.so.6(+0x37400)[0x7f62a0194400] /lib64/libc.so.6(+0x15d685)[0x7f62a02ba685] /lib64/libc.so.6(__strdup+0x12)[0x7f62a01e6802] perf(+0x3769d9)[0x557794e7e9d9] perf(+0x376d3a)[0x557794e7ed3a] perf(+0x377284)[0x557794e7f284] perf(+0x3773e0)[0x557794e7f3e0] perf(hist_entry_iter__add+0xc4)[0x557794e80114] perf(+0x2799aa)[0x557794d819aa] perf(+0x354ec8)[0x557794e5cec8] perf(+0x279341)[0x557794d81341] /lib64/libpthread.so.0(+0x814a)[0x7f62a27b514a] /lib64/libc.so.6(clone+0x43)[0x7f62a0259dc3] Fixes: 1fb7d06a509e ("perf report: Use srcline from callchain for hist entries") Signed-off-by: Michael Petlan <redacted> Reported-by: Juri Lelli <redacted> CC: Milian Wolff <redacted> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Arnaldo Carvalho de Melo <redacted> --- tools/perf/util/machine.c | 1 + 1 file changed, 1 insertion(+)diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index da19be7da284..44e40bad0e33 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c@@ -2149,6 +2149,7 @@ static int add_callchain_ip(struct thread *thread, al.filtered = 0; al.sym = NULL; + al.srcline = NULL; if (!cpumode) { thread__find_cpumode_addr_location(thread, ip, &al); } else {-- 2.18.4-- - Arnaldo