2013-10-08 10:50:21

by Stephane Eranian

Subject: [PATCH] perf tools: fix missing default initialization for tool->mmap2

This patch fixes a SEGFAULT bug with perf sched
related to the new MMAP2 record type. There was
a missing default initialization for the tool->mmap2
pointer in perf_tool__fill_defaults().

Reproducer:
# perf sched record -- sleep 1
# perf sched lat

Reported-by: Mike Galbraith <[email protected]>
Signed-off-by: Stephane Eranian <[email protected]>
--

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 211b325..8415c54 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -256,6 +256,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
tool->sample = process_event_sample_stub;
if (tool->mmap == NULL)
tool->mmap = process_event_stub;
+ if (tool->mmap2 == NULL)
+ tool->mmap2 = process_event_stub;
if (tool->comm == NULL)
tool->comm = process_event_stub;
if (tool->fork == NULL)

2013-10-08 11:59:35

by Ingo Molnar

Subject: Re: [PATCH] perf tools: fix missing default initialization for tool->mmap2

* Stephane Eranian <[email protected]> wrote:

>
> This patch fixes a SEGFAULT bug with perf sched
> related to the new MMAP2 record type. There was
> a missing default initialization for the tool->mmap2
> pointer in perf_tool__fill_defaults().
>
> Reproducer:
> # perf sched record -- sleep 1
> # perf sched lat
>
> Reported-by: Mike Galbraith <[email protected]>
> Signed-off-by: Stephane Eranian <[email protected]>
> --
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 211b325..8415c54 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -256,6 +256,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
> tool->sample = process_event_sample_stub;
> if (tool->mmap == NULL)
> tool->mmap = process_event_stub;
> + if (tool->mmap2 == NULL)
> + tool->mmap2 = process_event_stub;
> if (tool->comm == NULL)
> tool->comm = process_event_stub;
> if (tool->fork == NULL)

I think it's already fixed in tip:perf/urgent:

6adb0b0ae26f perf tools: Add default handler for mmap2 events

Thanks,

Ingo

2013-10-08 12:02:11

by Stephane Eranian

Subject: Re: [PATCH] perf tools: fix missing default initialization for tool->mmap2

On Tue, Oct 8, 2013 at 1:59 PM, Ingo Molnar <[email protected]> wrote:
>
> * Stephane Eranian <[email protected]> wrote:
>
>>
>> This patch fixes a SEGFAULT bug with perf sched
>> related to the new MMAP2 record type. There was
>> a missing default initialization for the tool->mmap2
>> pointer in perf_tool__fill_defaults().
>>
>> Reproducer:
>> # perf sched record -- sleep 1
>> # perf sched lat
>>
>> Reported-by: Mike Galbraith <[email protected]>
>> Signed-off-by: Stephane Eranian <[email protected]>
>> --
>>
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index 211b325..8415c54 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -256,6 +256,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
>> tool->sample = process_event_sample_stub;
>> if (tool->mmap == NULL)
>> tool->mmap = process_event_stub;
>> + if (tool->mmap2 == NULL)
>> + tool->mmap2 = process_event_stub;
>> if (tool->comm == NULL)
>> tool->comm = process_event_stub;
>> if (tool->fork == NULL)
>
> I think it's already fixed in tip:perf/urgent:
>
> 6adb0b0ae26f perf tools: Add default handler for mmap2 events
>
Ok, it's good then.
Thanks for checking.

2013-10-08 12:28:31

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH] perf tools: fix missing default initialization for tool->mmap2

Em Tue, Oct 08, 2013 at 02:02:06PM +0200, Stephane Eranian escreveu:
> On Tue, Oct 8, 2013 at 1:59 PM, Ingo Molnar <[email protected]> wrote:
> >> --- a/tools/perf/util/session.c
> >> +++ b/tools/perf/util/session.c
> >> @@ -256,6 +256,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
> >> tool->sample = process_event_sample_stub;
> >> if (tool->mmap == NULL)
> >> tool->mmap = process_event_stub;
> >> + if (tool->mmap2 == NULL)
> >> + tool->mmap2 = process_event_stub;
> >> if (tool->comm == NULL)
> >> tool->comm = process_event_stub;
> >> if (tool->fork == NULL)

> > I think it's already fixed in tip:perf/urgent:

> > 6adb0b0ae26f perf tools: Add default handler for mmap2 events

> Ok, it's good then.
> Thanks for checking.

Yeah, since 'perf sched' doesn't use mmap records, just relying on the
stub default handler is enough.

- Arnaldo

2013-10-08 12:46:27

by Stephane Eranian

Subject: Re: [PATCH] perf tools: fix missing default initialization for tool->mmap2

On Tue, Oct 8, 2013 at 2:28 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Tue, Oct 08, 2013 at 02:02:06PM +0200, Stephane Eranian escreveu:
>> On Tue, Oct 8, 2013 at 1:59 PM, Ingo Molnar <[email protected]> wrote:
>> >> --- a/tools/perf/util/session.c
>> >> +++ b/tools/perf/util/session.c
>> >> @@ -256,6 +256,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
>> >> tool->sample = process_event_sample_stub;
>> >> if (tool->mmap == NULL)
>> >> tool->mmap = process_event_stub;
>> >> + if (tool->mmap2 == NULL)
>> >> + tool->mmap2 = process_event_stub;
>> >> if (tool->comm == NULL)
>> >> tool->comm = process_event_stub;
>> >> if (tool->fork == NULL)
>
>> > I think it's already fixed in tip:perf/urgent:
>
>> > 6adb0b0ae26f perf tools: Add default handler for mmap2 events
>
>> Ok, it's good then.
>> Thanks for checking.
>
> Yeah, since 'perf sched' doesn't use mmap records, just relying on the
> stub default handler is enough.
>
Yeah, exactly.