Re: [PATCH v7 07/23] perf lock: Only pass sample to handlers

sashiko-bot
2026-04-08T08:38:59Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Returning `NULL` for missing callchain in `get_callstack()` causes the caller to incorrectly interpret it as an out-of-memory error and abort the `perf` session.
--

commit b0949e285b802619269396d6a3f7ff0f978d93cd
Author: Ian Rogers <irogers@google.com>

perf lock: Only pass sample to handlers

The evsel is within the sample and so only the sample needs to be
passed. Remove the parameter and fix call site. Add an additional NULL
check in get_callstack.

> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index e2b585e528aca..8ccb64415d3ba 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
[ ... ]
> @@ -948,6 +938,9 @@ static u64 *get_callstack(struct perf_sample *sample, int max_stack)
> u64 i;
> int c;
>
> + if (!sample->callchain)
> + return NULL;
> +
> callstack = calloc(max_stack, sizeof(*callstack));
> if (callstack == NULL)
> return NULL;

Does returning NULL here introduce a regression where the perf session
unexpectedly aborts with an out-of-memory error?

If a missing callchain is legitimately encountered (e.g. if the user
recorded lock tracepoints without -g), get_callstack() returns NULL.

The caller in report_lock_contention_begin_event() interprets this as an
allocation failure:

if (needs_callstack()) {
u64 *callstack = get_callstack(sample, max_stack_depth);
if (callstack == NULL)
return -ENOMEM;
...
}

Would it be better to differentiate between a missing callchain and a genuine
memory allocation failure to prevent the parsing loop from fatally aborting?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260408072601.210837-1-irogers@google.com?part=7