Re: [PATCH v3 1/3] perf sched: Suppress latency table output when trace samples are missing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL pointer check after calling `thread__get_runtime()` in `map_switch_event()` could lead to a segmentation fault.
- [Medium] The default `sched.tool` initialization for `perf sched` subcommands omits the `.attr` and `.feature` callbacks, preventing `evlist` population in pipe mode.
--

--- Patch [1]: [PATCH v3 1/3] perf sched: Suppress latency table output when trace samples are missing ---
commit 74145a7e4615a4aa12c0b60a699cc17c4a941dd7
Author: Aaron Tomlin <atomlin@atomlin.com>

perf sched: Suppress latency table output when trace samples are missing

This commit updates perf_sched__read_events() to abort early and return an
error if perf_session__has_traces() evaluates to false. This suppresses
empty latency table outputs when no tracepoint samples are present in the
perf.data file.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 7fd63a9db4574..b3cf678573e0a 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
[ ... ]
> @@ -2018,18 +2018,19 @@ static int perf_sched__read_events(struct perf_sched *sched)
>  	if (perf_session__set_tracepoints_handlers(session, handlers))
>  		goto out_delete;
>  
> -	if (perf_session__has_traces(session, "record -R")) {
> -		int err = perf_session__process_events(session);
> -		if (err) {
> -			pr_err("Failed to process events, error %d", err);
> -			goto out_delete;
> -		}
> +	if (!perf_session__has_traces(session, "record -R"))
> +		goto out_delete;

[Severity: Medium]
This is a pre-existing issue, but does this patch now cause pipe mode to
always fail and return an error code?

Because sched.tool is initialized in cmd_sched() without the mandatory
.attr and .feature callbacks:

	perf_tool__init(&sched.tool, /*ordered_events=*/true);
	sched.tool.sample	 = perf_sched__process_tracepoint_sample;
	sched.tool.comm		 = perf_sched__process_comm;
	sched.tool.namespaces	 = perf_event__process_namespaces;
	sched.tool.lost		 = perf_event__process_lost;
	sched.tool.fork		 = perf_sched__process_fork_event;

In pipe mode, PERF_RECORD_HEADER_ATTR events are silently dropped by the
fallback stub handler, which prevents evlist population. As a result,
perf_session__has_traces() always evaluates to false when reading from a
pipe.

With this patch adding an early abort, does this mean commands like
'perf sched latency -i -' will now unconditionally fail instead of processing
what they can?

[ ... ]

While reviewing the rest of the file, I noticed another pre-existing issue
in map_switch_event() that might surface under memory pressure:

	sched_out:
		if (sched->map.task_name) {
			tr = thread__get_runtime(sched->curr_out_thread[this_cpu.cpu]);
			if (strcmp(tr->shortname, "") == 0)
				goto out;

[Severity: High]
This is a pre-existing issue, but could this lead to a segmentation fault?

If memory allocation fails inside thread__init_runtime(), thread__get_runtime()
returns NULL. Unlike other places in map_switch_event() that verify the
returned pointer, the tr pointer is immediately dereferenced by strcmp() here
without a NULL check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726032533.712462-1-atomlin@atomlin.com?part=1