Perf test case 6 "Parse event definition strings"
dumps core when executed on s390.
Root case is a NULL pointer supplied in function
test_event()
+---> parse_events()
The third parameter is set to NULL:
(gdb) where
#0 parse_events (evlist=0x149dc90, str=0x133242a "intel_pt//u", err=0x0)
at util/parse-events.c:1835
#1 0x00000000010d3d1c in test_event (e=0x14330e0 <test.events+1272>)
at tests/parse-events.c:1696
#2 0x00000000010d3e88 in test_events (events=0x1432be8 <test.events>, cnt=54)
at tests/parse-events.c:1718
#3 0x00000000010d44c0 in test__parse_events (test=0x142b500
<generic_tests+280>, subtest=-1) at tests/parse-events.c:1838
Function parse_events(xx, xx, struct parse_events_error *err) dives
into a bison generated scanner and creates
parser state information for it first:
struct parse_events_state parse_state = {
.list = LIST_HEAD_INIT(parse_state.list),
.idx = evlist->nr_entries,
.error = err, <--- NULL POINTER !!!
.evlist = evlist,
};
Now various functions inside the bison scanner are called to end up in
__parse_events_add_pmu(struct parse_events_state *parse_state, ..) with
first parameter being a pointer to above structure definition.
Now the event name is not found (because being executed on s390) and
this function tries to create an error message with
asprintf(&parse_state->error.str, ....)
which references above NULL pointer and dumps core.
Fix this by providing a pointer to the necessary error information
instead of NULL.
Please note that the test still fails on non x86 platforms but for
different and valid reason.
Output with this fix:
[root@s35lp76 perf]# ./perf test -vvvvv -F 6
6: Parse event definition strings :
--- start ---
running test 0 'syscalls:sys_enter_openat'
Using CPUID IBM,3906,703,M03,3.5,002f
running test 1 'syscalls:*'
running test 2 'r1a'
running test 3 '1:1'
running test 4 'instructions'
...
running test 51 'L1-dcache-misses/name=cachepmu/'
running test 52 'intel_pt//u'
failed to parse event 'intel_pt//u', err 1
omitting PMU cpu tests
omitting PMU cpu tests
running test 0 'config=10,config1,config2=3,umask=1'
---- end ----
Parse event definition strings: FAILED!
Fixes: b3f58c8da64b ("perf tests parse-events: Add intel_pt parse test")
Signed-off-by: Thomas Richter <[email protected]>
Reviewed-by: Hendrik Brueckner <[email protected]>
---
tools/perf/tests/parse-events.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index b9ebe15afb13..f1012d7aea7a 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1686,6 +1686,7 @@ static struct terms_test test__terms[] = {
static int test_event(struct evlist_test *e)
{
+ struct parse_events_error errinfo;
struct perf_evlist *evlist;
int ret;
@@ -1693,7 +1694,7 @@ static int test_event(struct evlist_test *e)
if (evlist == NULL)
return -ENOMEM;
- ret = parse_events(evlist, e->name, NULL);
+ ret = parse_events(evlist, e->name, &errinfo);
if (ret) {
pr_debug("failed to parse event '%s', err %d\n",
e->name, ret);
--
2.14.3
On Fri, 8 Jun 2018 15:17:28 +0200
Thomas Richter <[email protected]> wrote:
> Perf test case 6 "Parse event definition strings"
> dumps core when executed on s390.
I reported it actually fails on any $ARCH system without
Intel Processor Trace (PT) h/w:
https://www.spinics.net/lists/linux-perf-users/msg06020.html
There was a follow-up patch sent here:
https://www.spinics.net/lists/linux-perf-users/msg06029.html
which worked for me, but I don't know what has/has-not transpired
since, other than it is still broken, as you found.
Kim
On 06/08/2018 04:53 PM, Kim Phillips wrote:
> On Fri, 8 Jun 2018 15:17:28 +0200
> Thomas Richter <[email protected]> wrote:
>
>> Perf test case 6 "Parse event definition strings"
>> dumps core when executed on s390.
>
> I reported it actually fails on any $ARCH system without
> Intel Processor Trace (PT) h/w:
>
> https://www.spinics.net/lists/linux-perf-users/msg06020.html
>
> There was a follow-up patch sent here:
>
> https://www.spinics.net/lists/linux-perf-users/msg06029.html
>
> which worked for me, but I don't know what has/has-not transpired
> since, other than it is still broken, as you found.
>
> Kim
Looks like this is still broken. I also can not find this patch on
git clone -b perf/core git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
We actually need 2 patches
- one to fix the core dump
- one to test for x86 platform
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, Jun 11, 2018 at 09:00:53AM +0200, Thomas-Mich Richter wrote:
> On 06/08/2018 04:53 PM, Kim Phillips wrote:
> > On Fri, 8 Jun 2018 15:17:28 +0200
> > Thomas Richter <[email protected]> wrote:
> >
> >> Perf test case 6 "Parse event definition strings"
> >> dumps core when executed on s390.
> >
> > I reported it actually fails on any $ARCH system without
> > Intel Processor Trace (PT) h/w:
> >
> > https://www.spinics.net/lists/linux-perf-users/msg06020.html
> >
> > There was a follow-up patch sent here:
> >
> > https://www.spinics.net/lists/linux-perf-users/msg06029.html
> >
> > which worked for me, but I don't know what has/has-not transpired
> > since, other than it is still broken, as you found.
> >
> > Kim
>
>
> Looks like this is still broken. I also can not find this patch on
> git clone -b perf/core git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
>
> We actually need 2 patches
> - one to fix the core dump
> - one to test for x86 platform
right.. I forgot about those, will rebase/repost it
jirka
Adding optional 'valid' callback for events tests in
parse-events object, so we don't try to parse PMUs,
which are not supported.
Following line is displayed for skipped test:
running test 52 'intel_pt//u'... SKIP
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/parse-events.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 3b0bfdf5a594..347378ddaa8b 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1309,6 +1309,11 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
return 0;
}
+static bool test__intel_pt_valid(void)
+{
+ return !!perf_pmu__find("intel_pt");
+}
+
static int test__intel_pt(struct perf_evlist *evlist)
{
struct perf_evsel *evsel = perf_evlist__first(evlist);
@@ -1375,6 +1380,7 @@ struct evlist_test {
const char *name;
__u32 type;
const int id;
+ bool (*valid)(void);
int (*check)(struct perf_evlist *evlist);
};
@@ -1648,6 +1654,7 @@ static struct evlist_test test__events[] = {
},
{
.name = "intel_pt//u",
+ .valid = test__intel_pt_valid,
.check = test__intel_pt,
.id = 52,
},
@@ -1690,6 +1697,11 @@ static int test_event(struct evlist_test *e)
struct perf_evlist *evlist;
int ret;
+ if (e->valid && !e->valid()) {
+ pr_debug("... SKIP");
+ return 0;
+ }
+
evlist = perf_evlist__new();
if (evlist == NULL)
return -ENOMEM;
@@ -1716,10 +1728,11 @@ static int test_events(struct evlist_test *events, unsigned cnt)
for (i = 0; i < cnt; i++) {
struct evlist_test *e = &events[i];
- pr_debug("running test %d '%s'\n", e->id, e->name);
+ pr_debug("running test %d '%s'", e->id, e->name);
ret1 = test_event(e);
if (ret1)
ret2 = ret1;
+ pr_debug("\n");
}
return ret2;
@@ -1801,7 +1814,7 @@ static int test_pmu_events(void)
}
while (!ret && (ent = readdir(dir))) {
- struct evlist_test e;
+ struct evlist_test e = { 0 };
char name[2 * NAME_MAX + 1 + 12 + 3];
/* Names containing . are special and cannot be used directly */
--
2.13.6
Add missing error handling for parse_events calls in test_event
function that led to following segfault on s390:
running test 52 'intel_pt//u'
perf: Segmentation fault
...
/lib64/libc.so.6(vasprintf+0xe6) [0x3fffca3f106]
/lib64/libc.so.6(asprintf+0x46) [0x3fffca1aa96]
./perf(parse_events_add_pmu+0xb8) [0x80132088]
./perf(parse_events_parse+0xc62) [0x8019529a]
./perf(parse_events+0x98) [0x801341c0]
./perf(test__parse_events+0x48) [0x800cd140]
./perf(cmd_test+0x26a) [0x800bd44a]
test child interrupted
Adding the struct parse_events_error argument to parse_events
call. Also adding parse_events_print_error to get more details
on the parsing failures, like:
# perf test 6 -v
running test 52 'intel_pt//u'failed to parse event 'intel_pt//u', err 1, str 'Cannot find PMU `intel_pt'. Missing kernel support?'
event syntax error: 'intel_pt//u'
\___ Cannot find PMU `intel_pt'. Missing kernel support?
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/parse-events.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 7d4077068454..3b0bfdf5a594 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1686,6 +1686,7 @@ static struct terms_test test__terms[] = {
static int test_event(struct evlist_test *e)
{
+ struct parse_events_error err = { 0 };
struct perf_evlist *evlist;
int ret;
@@ -1693,10 +1694,11 @@ static int test_event(struct evlist_test *e)
if (evlist == NULL)
return -ENOMEM;
- ret = parse_events(evlist, e->name, NULL);
+ ret = parse_events(evlist, e->name, &err);
if (ret) {
- pr_debug("failed to parse event '%s', err %d\n",
- e->name, ret);
+ pr_debug("failed to parse event '%s', err %d, str '%s'\n",
+ e->name, ret, err.str);
+ parse_events_print_error(&err, e->name);
} else {
ret = e->check(evlist);
}
--
2.13.6
On Mon, 11 Jun 2018 11:34:21 +0200
Jiri Olsa <[email protected]> wrote:
> Add missing error handling for parse_events calls in test_event
> function that led to following segfault on s390:
like I said, this happens on any $ARCH machine without intel_pt
hardware.
Other than that, for both 1 & 2 in this series:
[Reported-and-]Tested-by: Kim Phillips <[email protected]>
Thanks,
Kim
Em Mon, Jun 11, 2018 at 11:34:22AM +0200, Jiri Olsa escreveu:
> Adding optional 'valid' callback for events tests in
> parse-events object, so we don't try to parse PMUs,
> which are not supported.
>
> Following line is displayed for skipped test:
>
> running test 52 'intel_pt//u'... SKIP
Would be nice to test the _parsing_ of this even on machines where this
PMU is not present, as developers using these machines as workstations
may end up breaking this.
Anyway, intel_pt now is in tons of machines, so should be caught easily,
if not by anyone, by me ;-\
- Arnaldo
> Link: http://lkml.kernel.org/n/[email protected]
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> tools/perf/tests/parse-events.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 3b0bfdf5a594..347378ddaa8b 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1309,6 +1309,11 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
> return 0;
> }
>
> +static bool test__intel_pt_valid(void)
> +{
> + return !!perf_pmu__find("intel_pt");
> +}
> +
> static int test__intel_pt(struct perf_evlist *evlist)
> {
> struct perf_evsel *evsel = perf_evlist__first(evlist);
> @@ -1375,6 +1380,7 @@ struct evlist_test {
> const char *name;
> __u32 type;
> const int id;
> + bool (*valid)(void);
> int (*check)(struct perf_evlist *evlist);
> };
>
> @@ -1648,6 +1654,7 @@ static struct evlist_test test__events[] = {
> },
> {
> .name = "intel_pt//u",
> + .valid = test__intel_pt_valid,
> .check = test__intel_pt,
> .id = 52,
> },
> @@ -1690,6 +1697,11 @@ static int test_event(struct evlist_test *e)
> struct perf_evlist *evlist;
> int ret;
>
> + if (e->valid && !e->valid()) {
> + pr_debug("... SKIP");
> + return 0;
> + }
> +
> evlist = perf_evlist__new();
> if (evlist == NULL)
> return -ENOMEM;
> @@ -1716,10 +1728,11 @@ static int test_events(struct evlist_test *events, unsigned cnt)
> for (i = 0; i < cnt; i++) {
> struct evlist_test *e = &events[i];
>
> - pr_debug("running test %d '%s'\n", e->id, e->name);
> + pr_debug("running test %d '%s'", e->id, e->name);
> ret1 = test_event(e);
> if (ret1)
> ret2 = ret1;
> + pr_debug("\n");
> }
>
> return ret2;
> @@ -1801,7 +1814,7 @@ static int test_pmu_events(void)
> }
>
> while (!ret && (ent = readdir(dir))) {
> - struct evlist_test e;
> + struct evlist_test e = { 0 };
> char name[2 * NAME_MAX + 1 + 12 + 3];
>
> /* Names containing . are special and cannot be used directly */
> --
> 2.13.6
Commit-ID: 933ccf2002aaef1037cb676622a694f5390c3d59
Gitweb: https://git.kernel.org/tip/933ccf2002aaef1037cb676622a694f5390c3d59
Author: Jiri Olsa <[email protected]>
AuthorDate: Mon, 11 Jun 2018 11:34:21 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Jun 2018 11:59:36 -0300
perf tests: Add event parsing error handling to parse events test
Add missing error handling for parse_events calls in test_event function
that led to following segfault on s390:
running test 52 'intel_pt//u'
perf: Segmentation fault
...
/lib64/libc.so.6(vasprintf+0xe6) [0x3fffca3f106]
/lib64/libc.so.6(asprintf+0x46) [0x3fffca1aa96]
./perf(parse_events_add_pmu+0xb8) [0x80132088]
./perf(parse_events_parse+0xc62) [0x8019529a]
./perf(parse_events+0x98) [0x801341c0]
./perf(test__parse_events+0x48) [0x800cd140]
./perf(cmd_test+0x26a) [0x800bd44a]
test child interrupted
Adding the struct parse_events_error argument to parse_events call. Also
adding parse_events_print_error to get more details on the parsing
failures, like:
# perf test 6 -v
running test 52 'intel_pt//u'failed to parse event 'intel_pt//u', err 1, str 'Cannot find PMU `intel_pt'. Missing kernel support?'
event syntax error: 'intel_pt//u'
\___ Cannot find PMU `intel_pt'. Missing kernel support?
Committer note:
Use named initializers in the struct parse_events_error variable to
avoid breaking the build on centos5, 6 and others with a similar gcc:
cc1: warnings being treated as errors
tests/parse-events.c: In function 'test_event':
tests/parse-events.c:1696: error: missing initializer
tests/parse-events.c:1696: error: (near initialization for 'err.str')
Reported-by: Kim Phillips <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
Tested-by: Kim Phillips <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Hendrik Brueckner <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Richter <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/parse-events.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 7d4077068454..9751e7563a45 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1686,6 +1686,7 @@ static struct terms_test test__terms[] = {
static int test_event(struct evlist_test *e)
{
+ struct parse_events_error err = { .idx = 0, };
struct perf_evlist *evlist;
int ret;
@@ -1693,10 +1694,11 @@ static int test_event(struct evlist_test *e)
if (evlist == NULL)
return -ENOMEM;
- ret = parse_events(evlist, e->name, NULL);
+ ret = parse_events(evlist, e->name, &err);
if (ret) {
- pr_debug("failed to parse event '%s', err %d\n",
- e->name, ret);
+ pr_debug("failed to parse event '%s', err %d, str '%s'\n",
+ e->name, ret, err.str);
+ parse_events_print_error(&err, e->name);
} else {
ret = e->check(evlist);
}
Commit-ID: 16ddcfbf7f3d07aa781e26b39f2c28636a4ed2fd
Gitweb: https://git.kernel.org/tip/16ddcfbf7f3d07aa781e26b39f2c28636a4ed2fd
Author: Jiri Olsa <[email protected]>
AuthorDate: Mon, 11 Jun 2018 11:34:22 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Jun 2018 11:59:36 -0300
perf tests: Add valid callback for parse-events test
Adding optional 'valid' callback for events tests in parse-events
object, so we don't try to parse PMUs, which are not supported.
Following line is displayed for skipped test:
running test 52 'intel_pt//u'... SKIP
Committer note:
Use named initializers in the struct evlist_test variable to avoid
breaking the build on centos:5, 6 and others with a similar gcc:
cc1: warnings being treated as errors
tests/parse-events.c: In function 'test_pmu_events':
tests/parse-events.c:1817: error: missing initializer
tests/parse-events.c:1817: error: (near initialization for 'e.type')
Signed-off-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Hendrik Brueckner <[email protected]>
Cc: Kim Phillips <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Richter <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/parse-events.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 9751e7563a45..61211918bfba 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1309,6 +1309,11 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
return 0;
}
+static bool test__intel_pt_valid(void)
+{
+ return !!perf_pmu__find("intel_pt");
+}
+
static int test__intel_pt(struct perf_evlist *evlist)
{
struct perf_evsel *evsel = perf_evlist__first(evlist);
@@ -1375,6 +1380,7 @@ struct evlist_test {
const char *name;
__u32 type;
const int id;
+ bool (*valid)(void);
int (*check)(struct perf_evlist *evlist);
};
@@ -1648,6 +1654,7 @@ static struct evlist_test test__events[] = {
},
{
.name = "intel_pt//u",
+ .valid = test__intel_pt_valid,
.check = test__intel_pt,
.id = 52,
},
@@ -1690,6 +1697,11 @@ static int test_event(struct evlist_test *e)
struct perf_evlist *evlist;
int ret;
+ if (e->valid && !e->valid()) {
+ pr_debug("... SKIP");
+ return 0;
+ }
+
evlist = perf_evlist__new();
if (evlist == NULL)
return -ENOMEM;
@@ -1716,10 +1728,11 @@ static int test_events(struct evlist_test *events, unsigned cnt)
for (i = 0; i < cnt; i++) {
struct evlist_test *e = &events[i];
- pr_debug("running test %d '%s'\n", e->id, e->name);
+ pr_debug("running test %d '%s'", e->id, e->name);
ret1 = test_event(e);
if (ret1)
ret2 = ret1;
+ pr_debug("\n");
}
return ret2;
@@ -1801,7 +1814,7 @@ static int test_pmu_events(void)
}
while (!ret && (ent = readdir(dir))) {
- struct evlist_test e;
+ struct evlist_test e = { .id = 0, };
char name[2 * NAME_MAX + 1 + 12 + 3];
/* Names containing . are special and cannot be used directly */