Re: [PATCH] perf c2c report: Fix crash for empty browser

[Arnaldo Carvalho de Melo]

Adding Joe to the CC list.

Em Fri, Jul 27, 2018 at 09:06:23AM +0200, Jiri Olsa escreveu:
> On Thu, Jul 26, 2018 at 11:31:34PM +0000, rodia@xxxxxxxxxxxxx wrote:
> > On 2018-07-26 19:30, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Jul 24, 2018 at 08:20:08AM +0200, Jiri Olsa escreveu:
> > > > Do not try to display entry details if there's
> > > > not any. Currently this ends up in crash:
> > > >   $ perf c2c report
> > > >   perf: Segmentation fault

> > > How to replicate this?

> > > I tried:

> > > $ perf record sleep 1
> > > $ perf c2c report

> > > But it didn't segfault

> > Similarly I have tried :
> > $ perf record sleep 1
> > $ perf c2c report
> > Then Press `d` to show the cache-line contents.

> yep, sry I forgot to mention you need to press the 'd' to show details
 
> > This replies the segfault on my machine (4.17.8-1).
> > The patch mentioned above should solve it, even tough I am not sure as I
> > haven't been able to recompile the kernel.
 
> no need to recompile kernel
 
> > The segfault by itself seems to be due to the report logic, as it did not
> > expect to report on an empty browser.
> > What has stepped me back is that application which I have been testing with
> > rely on multiple threads instantiated through pthread, which should be
> > counted in user-level threads right? But they still seem to return an empty
> > browser.
 
> right, c2c scans read/write accesses and tries to find false sharing
> cases maybe there was nothing to be found
 
> > When instead c2c is runned system-wide, with an application running on
> > multiple threads like firefox or julia, cache hits are measured and also
> > they are traced back in the source code.
 
> I got a cache line (attached) for 'perf bench sched messaging'
> NOT being traced system wide and just for user (you'll get plenty
> of detected cachelines in kernel space):

With that info in mind, we get:

[root@seventh ~]# perf record sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.001 MB perf.data (6 samples) ]
[root@seventh ~]# 
[root@seventh ~]# 
[root@seventh ~]# perf c2c report # and press 'd'
perf: Segmentation fault
-------- backtrace --------
perf[0x5b1d2a]
/lib64/libc.so.6(+0x346df)[0x7fcb566e36df]
perf[0x46fcae]
perf[0x4a9f1e]
perf[0x4aa220]
perf(main+0x301)[0x42c561]
/lib64/libc.so.6(__libc_start_main+0xe9)[0x7fcb566cff29]
perf(_start+0x29)[0x42c999]
[root@seventh ~]#

With your patches the segfault is gone, but I'd do a follow up patch to
show some message telling the user why 'd' showed nothing and
instructing him about what is missing, i.e. is this done on a perf.data
file that has no events of interest? Suggest using 'perf c2c record' or
'perf record -e events,of,interest,to,perf,c2c', was this done on some
workload where no false sharing was detected? Say so, etc.

I applied your patch with a more detailed commit log to state how this
can reproduced, etc, as usual:

https://git.kernel.org/acme/c/983eb6aa7098

- Arnaldo