* 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