* [PATCH] perf tests: Fix Tool PMU test segfault
@ 2025-02-12 16:38 James Clark
2025-02-12 17:41 ` Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: James Clark @ 2025-02-12 16:38 UTC (permalink / raw)
To: linux-perf-users
Cc: James Clark, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan, Athira Rajeev, linux-kernel
tool_pmu__event_to_str() now handles skipped events by returning NULL,
so it's wrong to re-check for a skip on the resulting string. Calling
tool_pmu__skip_event() with a NULL string results in a segfault so
remove the unnecessary skip to fix it:
$ perf test -vv "parsing with PMU name"
12.2: Parsing with PMU name:
...
---- unexpected signal (11) ----
12.2: Parsing with PMU name : FAILED!
Fixes: ee8aef2d2321 ("perf tools: Add skip check in tool_pmu__event_to_str()")
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/tests/tool_pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c
index 187942b749b7..1e900ef92e37 100644
--- a/tools/perf/tests/tool_pmu.c
+++ b/tools/perf/tests/tool_pmu.c
@@ -27,7 +27,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)
parse_events_error__init(&err);
ret = parse_events(evlist, str, &err);
if (ret) {
- if (tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
+ if (!tool_pmu__event_to_str(ev)) {
ret = TEST_OK;
goto out;
}
@@ -59,7 +59,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)
}
}
- if (!found && !tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
+ if (!found && tool_pmu__event_to_str(ev)) {
pr_debug("FAILED %s:%d Didn't find tool event '%s' in parsed evsels\n",
__FILE__, __LINE__, str);
ret = TEST_FAIL;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf tests: Fix Tool PMU test segfault
2025-02-12 16:38 [PATCH] perf tests: Fix Tool PMU test segfault James Clark
@ 2025-02-12 17:41 ` Ian Rogers
2025-02-12 19:17 ` Liang, Kan
2025-02-13 17:21 ` Namhyung Kim
2 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2025-02-12 17:41 UTC (permalink / raw)
To: James Clark
Cc: linux-perf-users, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan,
Athira Rajeev, linux-kernel
On Wed, Feb 12, 2025 at 8:39 AM James Clark <james.clark@linaro.org> wrote:
>
> tool_pmu__event_to_str() now handles skipped events by returning NULL,
> so it's wrong to re-check for a skip on the resulting string. Calling
> tool_pmu__skip_event() with a NULL string results in a segfault so
> remove the unnecessary skip to fix it:
>
> $ perf test -vv "parsing with PMU name"
>
> 12.2: Parsing with PMU name:
> ...
> ---- unexpected signal (11) ----
> 12.2: Parsing with PMU name : FAILED!
>
> Fixes: ee8aef2d2321 ("perf tools: Add skip check in tool_pmu__event_to_str()")
> Signed-off-by: James Clark <james.clark@linaro.org>
Thanks James, I'd assumed this had been covered by Kan and Namhyung.
Tested-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/tests/tool_pmu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c
> index 187942b749b7..1e900ef92e37 100644
> --- a/tools/perf/tests/tool_pmu.c
> +++ b/tools/perf/tests/tool_pmu.c
> @@ -27,7 +27,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)
> parse_events_error__init(&err);
> ret = parse_events(evlist, str, &err);
> if (ret) {
> - if (tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
> + if (!tool_pmu__event_to_str(ev)) {
> ret = TEST_OK;
> goto out;
> }
> @@ -59,7 +59,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)
> }
> }
>
> - if (!found && !tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
> + if (!found && tool_pmu__event_to_str(ev)) {
> pr_debug("FAILED %s:%d Didn't find tool event '%s' in parsed evsels\n",
> __FILE__, __LINE__, str);
> ret = TEST_FAIL;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf tests: Fix Tool PMU test segfault
2025-02-12 16:38 [PATCH] perf tests: Fix Tool PMU test segfault James Clark
2025-02-12 17:41 ` Ian Rogers
@ 2025-02-12 19:17 ` Liang, Kan
2025-02-13 17:21 ` Namhyung Kim
2 siblings, 0 replies; 4+ messages in thread
From: Liang, Kan @ 2025-02-12 19:17 UTC (permalink / raw)
To: James Clark, linux-perf-users
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Athira Rajeev, linux-kernel
On 2025-02-12 11:38 a.m., James Clark wrote:
> tool_pmu__event_to_str() now handles skipped events by returning NULL,
> so it's wrong to re-check for a skip on the resulting string. Calling
> tool_pmu__skip_event() with a NULL string results in a segfault so
> remove the unnecessary skip to fix it:
>
> $ perf test -vv "parsing with PMU name"
>
> 12.2: Parsing with PMU name:
> ...
> ---- unexpected signal (11) ----
> 12.2: Parsing with PMU name : FAILED!
>
Oops, right. Thanks for the fix.
Acked-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> Fixes: ee8aef2d2321 ("perf tools: Add skip check in tool_pmu__event_to_str()")
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---> tools/perf/tests/tool_pmu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c
> index 187942b749b7..1e900ef92e37 100644
> --- a/tools/perf/tests/tool_pmu.c
> +++ b/tools/perf/tests/tool_pmu.c
> @@ -27,7 +27,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)
> parse_events_error__init(&err);
> ret = parse_events(evlist, str, &err);
> if (ret) {
> - if (tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
> + if (!tool_pmu__event_to_str(ev)) {
> ret = TEST_OK;
> goto out;
> }
> @@ -59,7 +59,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu)
> }
> }
>
> - if (!found && !tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
> + if (!found && tool_pmu__event_to_str(ev)) {
> pr_debug("FAILED %s:%d Didn't find tool event '%s' in parsed evsels\n",
> __FILE__, __LINE__, str);
> ret = TEST_FAIL;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf tests: Fix Tool PMU test segfault
2025-02-12 16:38 [PATCH] perf tests: Fix Tool PMU test segfault James Clark
2025-02-12 17:41 ` Ian Rogers
2025-02-12 19:17 ` Liang, Kan
@ 2025-02-13 17:21 ` Namhyung Kim
2 siblings, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2025-02-13 17:21 UTC (permalink / raw)
To: linux-perf-users, James Clark
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Athira Rajeev, linux-kernel
On Wed, 12 Feb 2025 16:38:56 +0000, James Clark wrote:
> tool_pmu__event_to_str() now handles skipped events by returning NULL,
> so it's wrong to re-check for a skip on the resulting string. Calling
> tool_pmu__skip_event() with a NULL string results in a segfault so
> remove the unnecessary skip to fix it:
>
> $ perf test -vv "parsing with PMU name"
>
> [...]
Applied to perf-tools-next, thanks!
Best regards,
Namhyung
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-02-13 17:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-12 16:38 [PATCH] perf tests: Fix Tool PMU test segfault James Clark
2025-02-12 17:41 ` Ian Rogers
2025-02-12 19:17 ` Liang, Kan
2025-02-13 17:21 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).