[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aMM6ejbed7s7_2t1@google.com>
Date: Thu, 11 Sep 2025 14:09:14 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: hupu <hupu.gm@...il.com>
Cc: irogers@...gle.com, linux-kernel@...r.kernel.org
Subject: Re: [RFC] perf subcmd: avoid crash in exclude_cmds when excludes is
empty
Hello,
On Wed, Sep 10, 2025 at 04:27:05PM +0800, hupu wrote:
> Dear Maintainers,
>
> When cross-compiling perf tool for ARM64 (though the issue is actually
> platform-independent), I encountered a crash when running `perf help`
> if the perf binary name is not exactly "perf" or if multiple "perf-*"
> binaries exist in the same directory.
>
> This patch is an attempt to fix the issue. I would like to discuss its
> reasonableness with you and see if this is the right direction.
>
>
> 1. Problem Reproduction
> =======================
>
> Case 1: Binary named "perf-master"
> ----------------------------------
> # /mnt/perf-master help
> perf-master: help.c:122: exclude_cmds: Assertion `cmds->names[ci] ==
> NULL' failed.
> Aborted
>
> Case 2: Renaming to "perf"
> --------------------------
> # mv perf-master perf
> # /mnt/perf help
> -> works as expected
> usage: perf [--version] [--help] [OPTIONS] COMMAND [ARGS]
>
> The most commonly used perf commands are:
> annotate Read perf.data (created by perf record) and
> display annotated code
> archive Create archive with object files with build-ids
> found in perf.data file
> bench General framework for benchmark suites
> buildid-cache Manage build-id cache.
> buildid-list List the buildids in a perf.data file
> c2c Shared Data C2C/HITM Analyzer.
> config Get and set variables in a configuration file.
> daemon Run record sessions on background
> data Data file related processing
> diff Read perf.data files and display the differential profile
> evlist List the event names in a perf.data file
> ftrace simple wrapper for kernel's ftrace functionality
> inject Filter to augment the events stream with
> additional information
> iostat Show I/O performance metrics
> kallsyms Searches running kernel for symbols
> kvm Tool to trace/measure kvm guest os
> list List all symbolic event types
> mem Profile memory accesses
> record Run a command and record its profile into perf.data
> report Read perf.data (created by perf record) and
> display the profile
> script Read perf.data (created by perf record) and
> display trace output
> stat Run a command and gather performance counter statistics
> test Runs sanity tests.
> top System profiling tool.
> version display the version of perf binary
> probe Define new dynamic tracepoints
> trace strace inspired tool
> kmem Tool to trace/measure kernel memory properties
> kwork Tool to trace/measure kernel work properties (latencies)
> lock Analyze lock events
> sched Tool to trace/measure scheduler properties (latencies)
> timechart Tool to visualize total system behavior during a workload
>
> See 'perf help COMMAND' for more information on a specific command.
>
> Case 3: Copying to "perf-backup"
> --------------------------------
> # cp perf perf-backup
> # /mnt/perf help
> perf: help.c:122: exclude_cmds: Assertion `cmds->names[ci] == NULL' failed.
> Aborted
>
> # /mnt/perf-backup help
> perf-backup: help.c:122: exclude_cmds: Assertion `cmds->names[ci] ==
> NULL' failed.
> Aborted
>
>
>
> 2. Root Cause
> =============
>
> In `exclude_cmds()`, `cmds` represents commands found in the current
> binary directory and PATH, while `excludes` comes from the
> `~/libexec/perf-core/` directory.
>
> The function attempts to remove commands from `cmds` that already
> exist in `excludes`. However, when `excludes->cnt == 0`, the final
> `assert(cmds->names[ci] == NULL)` may fail, leading to an abort.
>
>
>
> 3. Fix
> ======
>
> Add a simple guard at the beginning of `exclude_cmds()` to return
> early if `excludes->cnt == 0`.
>
> diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> index 9ef569492560..ddaeb4eb3e24 100644
> --- a/tools/lib/subcmd/help.c
> +++ b/tools/lib/subcmd/help.c
> @@ -75,6 +75,9 @@ void exclude_cmds(struct cmdnames *cmds, struct
> cmdnames *excludes)
> size_t ci, cj, ei;
> int cmp;
>
> + if (!excludes->cnt)
> + return;
> +
> ci = cj = ei = 0;
> while (ci < cmds->cnt && ei < excludes->cnt) {
> cmp = strcmp(cmds->names[ci]->name, excludes->names[ei]->name);
>
>
>
> 4.Verification
> ===============
>
> With this patch applied, running perf help works correctly even when
> the binary is named perf, perf-master, or perf-backup:
>
> /mnt/perf help
> /mnt/perf-master help
> /mnt/perf-backup help
>
> All of them now display the expected help information without aborting.
Thanks for the detailed report and the fix. Looks good to me we don't
need to look up the cmdnames when there's nothing to exclude.
Reviewed-by: Namhyung Kim <namhyung@...nel.org>
Thanks,
Namhyung
Powered by blists - more mailing lists