Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Hi Arnaldo,
On 06/20/2018 07:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
>> perf script in pipped mode is crashing because evsel->priv is not
>> set properly. Fix it.
>>
>> Before:
>> # ./perf record -o - -- ls | ./perf script
>> Segmentation fault (core dumped)
>>
>> After:
>> # ./perf record -o - -- ls | ./perf script
>> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
>> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>>
>> Signed-off-by: Ravi Bangoria
>> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
>
> Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
> object, will check which one does to continue review.
So, it's not about setting evsel->priv to 'struct perf_evsel_script', but
it's about setting proper output stream, either file or stdout. When
'struct perf_evsel_script' was not introduced, we were setting evsel->priv
to output stream. So I think, this commit missed to set evsel->priv properly
in pipped case. Ex, below patch applied _directly_ on top of a14390fde64e
fixes the issue.
--
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index fb5e49b3bc44..66cc4b29bf4d 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1645,6 +1645,9 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
evlist = *pevlist;
evsel = perf_evlist__last(*pevlist);
+ if (!evsel->priv)
+ evsel->priv = stdout;
+
if (evsel->attr.type >= PERF_TYPE_MAX &&
evsel->attr.type != PERF_TYPE_SYNTH)
return 0;
--
To me this commit seems to be the bad. Please let me know if that is not
the case. I'll change the last patch as you suggested an post v2 soon.
Thanks,
Ravi
Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Hi Arnaldo,
On 06/20/2018 07:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
>> perf script in pipped mode is crashing because evsel->priv is not
>> set properly. Fix it.
>>
>> Before:
>> # ./perf record -o - -- ls | ./perf script
>> Segmentation fault (core dumped)
>>
>> After:
>> # ./perf record -o - -- ls | ./perf script
>> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
>> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>>
>> Signed-off-by: Ravi Bangoria
>> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
>
> Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
> object, will check which one does to continue review.
So, it's not about setting evsel->priv to 'struct perf_evsel_script', but
it's about setting proper output stream, either file or stdout. When
'struct perf_evsel_script' was not introduced, we were setting evsel->priv
to output stream. So I think, this commit missed to set evsel->priv properly
in pipped case. Ex, below patch applied _directly_ on top of a14390fde64e
fixes the issue.
--
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index fb5e49b3bc44..66cc4b29bf4d 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1645,6 +1645,9 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
evlist = *pevlist;
evsel = perf_evlist__last(*pevlist);
+ if (!evsel->priv)
+ evsel->priv = stdout;
+
if (evsel->attr.type >= PERF_TYPE_MAX &&
evsel->attr.type != PERF_TYPE_SYNTH)
return 0;
--
To me this commit seems to be the bad. Please let me know if that is not
the case. I'll change the last patch as you suggested an post v2 soon.
Thanks,
Ravi
Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Hi Arnaldo,
On 06/20/2018 07:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
>> perf script in pipped mode is crashing because evsel->priv is not
>> set properly. Fix it.
>>
>> Before:
>> # ./perf record -o - -- ls | ./perf script
>> Segmentation fault (core dumped)
>>
>> After:
>> # ./perf record -o - -- ls | ./perf script
>> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
>> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>>
>> Signed-off-by: Ravi Bangoria
>> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
>
> Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
> object, will check which one does to continue review.
Possible. Actually, git bisect led me to this commit.
Ravi
Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Hi Arnaldo,
On 06/20/2018 07:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
>> perf script in pipped mode is crashing because evsel->priv is not
>> set properly. Fix it.
>>
>> Before:
>> # ./perf record -o - -- ls | ./perf script
>> Segmentation fault (core dumped)
>>
>> After:
>> # ./perf record -o - -- ls | ./perf script
>> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
>> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>>
>> Signed-off-by: Ravi Bangoria
>> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
>
> Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
> object, will check which one does to continue review.
Possible. Actually, git bisect led me to this commit.
Ravi
Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
> perf script in pipped mode is crashing because evsel->priv is not
> set properly. Fix it.
>
> Before:
> # ./perf record -o - -- ls | ./perf script
> Segmentation fault (core dumped)
>
> After:
> # ./perf record -o - -- ls | ./perf script
> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>
> Signed-off-by: Ravi Bangoria
> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
object, will check which one does to continue review.
- Arnaldo
> ---
> tools/perf/builtin-script.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 4d1cee68cfd2..acee05562f5e 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1822,6 +1822,7 @@ static int process_attr(struct perf_tool *tool, union
> perf_event *event,
> struct perf_evlist *evlist;
> struct perf_evsel *evsel, *pos;
> int err;
> + static struct perf_evsel_script *es;
>
> err = perf_event__process_attr(tool, event, pevlist);
> if (err)
> @@ -1830,6 +1831,19 @@ static int process_attr(struct perf_tool *tool, union
> perf_event *event,
> evlist = *pevlist;
> evsel = perf_evlist__last(*pevlist);
>
> + if (!evsel->priv) {
> + if (scr->per_event_dump) {
> + evsel->priv = perf_evsel_script__new(evsel,
> + scr->session->data);
> + } else {
> + es = zalloc(sizeof(*es));
> + if (!es)
> + return -ENOMEM;
> + es->fp = stdout;
> + evsel->priv = es;
> + }
> + }
> +
> if (evsel->attr.type >= PERF_TYPE_MAX &&
> evsel->attr.type != PERF_TYPE_SYNTH)
> return 0;
> --
> 2.14.4
Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
> perf script in pipped mode is crashing because evsel->priv is not
> set properly. Fix it.
>
> Before:
> # ./perf record -o - -- ls | ./perf script
> Segmentation fault (core dumped)
>
> After:
> # ./perf record -o - -- ls | ./perf script
> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>
> Signed-off-by: Ravi Bangoria
> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
object, will check which one does to continue review.
- Arnaldo
> ---
> tools/perf/builtin-script.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 4d1cee68cfd2..acee05562f5e 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1822,6 +1822,7 @@ static int process_attr(struct perf_tool *tool, union
> perf_event *event,
> struct perf_evlist *evlist;
> struct perf_evsel *evsel, *pos;
> int err;
> + static struct perf_evsel_script *es;
>
> err = perf_event__process_attr(tool, event, pevlist);
> if (err)
> @@ -1830,6 +1831,19 @@ static int process_attr(struct perf_tool *tool, union
> perf_event *event,
> evlist = *pevlist;
> evsel = perf_evlist__last(*pevlist);
>
> + if (!evsel->priv) {
> + if (scr->per_event_dump) {
> + evsel->priv = perf_evsel_script__new(evsel,
> + scr->session->data);
> + } else {
> + es = zalloc(sizeof(*es));
> + if (!es)
> + return -ENOMEM;
> + es->fp = stdout;
> + evsel->priv = es;
> + }
> + }
> +
> if (evsel->attr.type >= PERF_TYPE_MAX &&
> evsel->attr.type != PERF_TYPE_SYNTH)
> return 0;
> --
> 2.14.4
[PATCH 2/3] perf script: Fix crash because of missing evsel->priv
perf script in pipped mode is crashing because evsel->priv is not
set properly. Fix it.
Before:
# ./perf record -o - -- ls | ./perf script
Segmentation fault (core dumped)
After:
# ./perf record -o - -- ls | ./perf script
ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
Signed-off-by: Ravi Bangoria
Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
---
tools/perf/builtin-script.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4d1cee68cfd2..acee05562f5e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1822,6 +1822,7 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
struct perf_evlist *evlist;
struct perf_evsel *evsel, *pos;
int err;
+ static struct perf_evsel_script *es;
err = perf_event__process_attr(tool, event, pevlist);
if (err)
@@ -1830,6 +1831,19 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
evlist = *pevlist;
evsel = perf_evlist__last(*pevlist);
+ if (!evsel->priv) {
+ if (scr->per_event_dump) {
+ evsel->priv = perf_evsel_script__new(evsel,
+ scr->session->data);
+ } else {
+ es = zalloc(sizeof(*es));
+ if (!es)
+ return -ENOMEM;
+ es->fp = stdout;
+ evsel->priv = es;
+ }
+ }
+
if (evsel->attr.type >= PERF_TYPE_MAX &&
evsel->attr.type != PERF_TYPE_SYNTH)
return 0;
--
2.14.4
[PATCH 2/3] perf script: Fix crash because of missing evsel->priv
perf script in pipped mode is crashing because evsel->priv is not
set properly. Fix it.
Before:
# ./perf record -o - -- ls | ./perf script
Segmentation fault (core dumped)
After:
# ./perf record -o - -- ls | ./perf script
ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
Signed-off-by: Ravi Bangoria
Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
---
tools/perf/builtin-script.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4d1cee68cfd2..acee05562f5e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1822,6 +1822,7 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
struct perf_evlist *evlist;
struct perf_evsel *evsel, *pos;
int err;
+ static struct perf_evsel_script *es;
err = perf_event__process_attr(tool, event, pevlist);
if (err)
@@ -1830,6 +1831,19 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
evlist = *pevlist;
evsel = perf_evlist__last(*pevlist);
+ if (!evsel->priv) {
+ if (scr->per_event_dump) {
+ evsel->priv = perf_evsel_script__new(evsel,
+ scr->session->data);
+ } else {
+ es = zalloc(sizeof(*es));
+ if (!es)
+ return -ENOMEM;
+ es->fp = stdout;
+ evsel->priv = es;
+ }
+ }
+
if (evsel->attr.type >= PERF_TYPE_MAX &&
evsel->attr.type != PERF_TYPE_SYNTH)
return 0;
--
2.14.4
