Fix (well, work around) perf stat --null segfault

JFYI:

  starship:~> perf stat --null ls
  Segmentation fault (core dumped)

Program received signal SIGSEGV, Segmentation fault.
0x000055555563596e in __perf_cpu_map__nr (cpus=<error reading variable: Cannot access memory at address 0x0>) at cpumap.c:257
257 return RC_CHK_ACCESS(cpus)->nr;
(gdb) bt

  #0  0x000055555563596e in __perf_cpu_map__nr (cpus=<error reading variable: Cannot access memory at address 0x0>) at cpumap.c:257
  #1  perf_cpu_map__max (map=0x0) at cpumap.c:372
  #2  0x00005555555cc249 in perf_stat_init_aggr_mode ()
  #3  0x00005555555d1963 in cmd_stat ()
  #4  0x0000555555631bb2 in run_builtin ()
  #5  0x0000555555631e9e in handle_internal_command ()
  #6  0x00005555555ab183 in main ()

(gdb)

(gdb) print cpus
Cannot access memory at address 0x0
(gdb) bt

  #0  0x000055555563596e in __perf_cpu_map__nr (cpus=<error reading variable: Cannot access memory at address 0x0>) at cpumap.c:257
  #1  perf_cpu_map__max (map=0x0) at cpumap.c:372
  #2  0x00005555555cc249 in perf_stat_init_aggr_mode ()
  #3  0x00005555555d1963 in cmd_stat ()
  #4  0x0000555555631bb2 in run_builtin ()
  #5  0x0000555555631e9e in handle_internal_command ()
  #6  0x00005555555ab183 in main ()

(gdb)

For some build(?) reason I cannot set breakpoints inside builtin-stat.c:

(gdb) b builtin-stat.c:1545
No source file named builtin-stat.c.

... and didn't investigate any further.

System has 128 CPUs.

It's been broken for some time:

starship:~/tip> git bisect bad ced4c249569ab25c32b0d36e2ebdb19c74394bdf is the first bad commit commit ced4c249569ab25c32b0d36e2ebdb19c74394bdf (HEAD)
Author: Ian Rogers <[email protected]>
Date: Fri Jul 18 20:05:05 2025 -0700

perf stat: Don't size aggregation ids from user_requested_cpus

As evsels may have additional CPU terms, the user_requested_cpus may not reflect all the CPUs requested. Use evlist->all_cpus to size the array as that reflects all the CPUs potentially needed by the evlist.

Reviewed-by Thomas Falcon <[email protected]>

Signed-off-by Ian Rogers <[email protected]>

Tested-by James Clark <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Signed-off-by Namhyung Kim <[email protected]>

The bug is the assumption that evsel_list->core.all_cpus exists at this stage.

The attached patch works this around like the old version, but there's more things wrong with this code:

BTW., I couldn't even find where all_cpus pointer gets allocated. It gets zalloc()-ed to NULL, and cleared back to NULL in two places:

  lib/perf/evlist.c:    evlist->all_cpus = NULL;
  lib/perf/evlist.c:    evlist->all_cpus = NULL;

But where does it get allocated? perf_cpu_map__nr() hides it in most cases that ->cpus_all is always NULL:

  int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
  {
        return cpus ? __perf_cpu_map__nr(cpus) : 1;
  }

Which in general is a code robustness red flag, for something this fundamental I doubt there should be any conditionality to this pointer. Allowing cpus pointers to be NULL and hiding it in the helpers hid a real bug for months.

So this code looks like a bit of a trainwreck. ;-)

In addition to fixing the bug, it might also make sense to add a testcase for 'perf stat --null'? (which is a better, more accurate 'time' utility)

As an additional report and side note, I still get this bogus message during a build on standard Ubuntu:

Makefile.config:1151: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-latest-openjdk-devel

The suggested "e.g. openjdk-8-jdk" package *is* installed:

starship:~/tip> dpkg -l | grep openjdk

 ii  openjdk-11-jre:amd64                                        11.0.29+7-1ubuntu1~25.10                            amd64        OpenJDK Java runtime, using Hotspot JIT
 ii  openjdk-11-jre-headless:amd64                               11.0.29+7-1ubuntu1~25.10                            amd64        OpenJDK Java runtime, using Hotspot JIT (headless)
 ii  openjdk-17-jre:amd64                                        17.0.17+10-1~25.10                                  amd64        OpenJDK Java runtime, using Hotspot JIT
 ii  openjdk-17-jre-headless:amd64                               17.0.17+10-1~25.10                                  amd64        OpenJDK Java runtime, using Hotspot JIT (headless)
 ii  openjdk-21-jre:amd64                                        21.0.9+10-1~25.10                                   amd64        OpenJDK Java runtime, using Hotspot JIT
 ii  openjdk-21-jre-headless:amd64                               21.0.9+10-1~25.10                                   amd64        OpenJDK Java runtime, using Hotspot JIT (headless)
 ii  openjdk-8-jdk:amd64                                         8u472-ga-1~25.10                                    amd64        OpenJDK Development Kit (JDK)
 ii  openjdk-8-jdk-headless:amd64                                8u472-ga-1~25.10                                    amd64        OpenJDK Development Kit (JDK) (headless)
 ii  openjdk-8-jre:amd64                                         8u472-ga-1~25.10                                    amd64        OpenJDK Java runtime, using Hotspot JIT
 ii  openjdk-8-jre-headless:amd64                                8u472-ga-1~25.10                                    amd64        OpenJDK Java runtime, using Hotspot JIT (headless)

Thanks,

Ingo

====================>

Signed-off-by Ingo Molnar <[email protected]>

 tools/perf/builtin-stat.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

tools/perf/builtin-stat.c +0 -0

index 7006f848f87a..973b72218df7 100644

tools/perf/builtin-stat.c +0 -0

        stat_config.cpus_aggr_map = cpu_aggr_map__empty_new(nr);
        return stat_config.cpus_aggr_map ? 0 : -ENOMEM;
 }

Agreed on the trainwreck :-) So NULL is a valid CPU map value. It means no CPUs, but magically no CPUs can also be converted in the code to become the "any" CPU value of -1. So iterating the perf_cpu_map of NULL will yield a single CPU of -1 :-( This behavior existed before my time working on the code, I've tried to make the expectation in the using code clearer, does the user want the magic "any" CPU behavior? "any" CPU is distinct from every/all CPUs which would be something like CPUs 0-127, and then does "all" include online and offline CPUs? This is something that's been discussed and I think things can be better, I'd rather the code was explicit whenever terms like "all", "online" and "any" were being used - it is very easy to confuse "all" and "any". I've been trying to adjust the code and make it more sensible with changes like:
https://lore.kernel.org/all/[email protected]/
but I agree with you that things are a mess here. At least we shouldn't be leaking memory (due to the reference count checking) and we shouldn't confuse indices and CPU numbers (due to the introduction of a wrapper type), problems that were pervasive and broke things like uncore counters frequently before.

Fwiw, part of me thinks we should change the perf_cpu_map to be a cpu_set_t, the "any" case could be an additional boolean in the struct. Perhaps we can just auto change "all" to "any" as is somewhat done here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/perf/evlist.c?h=perf-tools-next#n92
Then perf_cpu_map can be a cpu_set_t and I think life will be clearer.

Fwiw, even on the kernel side this is borked as I tried to explain in this RFC with 0 follow ups:
https://lore.kernel.org/lkml/[email protected]/

Imagine the case of an event opened with 1 CPU and a TID. The filter will mean the counter only counts on the 1 CPU but the enabled and running time will mean the counter will be scaled (on your system) 128 times!
All of this matters when you consider different events opened in the same evlist and there being combined perf_cpu_maps like the evlist's all_cpus value.

tools/perf/builtin-stat.c static int perf_stat_init_aggr_mode(void) · line 1548

Quoted 5 lines

tools/perf/builtin-stat.c +4 -1

-       nr = perf_cpu_map__max(evsel_list->core.all_cpus).cpu + 1;
+       if (evsel_list->core.all_cpus)
+               nr = perf_cpu_map__max(evsel_list->core.all_cpus).cpu + 1;
+       else
+               nr = 0;

So I think the better fix is to make max handle a NULL perf_cpu_map. That will yield "any" CPU of -1 ( :-( ) and then the -1 + 1 will give an empty aggregation map.

I'll write this up, add the test and see if I can spot similar issues wrt NULL in the vicinity in the perf_cpu_map code.

Quoted 20 lines · Ian Rogers

Agreed on the trainwreck :-) So NULL is a valid CPU map value. It means no CPUs, but magically no CPUs can also be converted in the code to become the "any" CPU value of -1. So iterating the perf_cpu_map of NULL will yield a single CPU of -1 :-( This behavior existed before my time working on the code, I've tried to make the expectation in the using code clearer, does the user want the magic "any" CPU behavior? "any" CPU is distinct from every/all CPUs which would be something like CPUs 0-127, and then does "all" include online and offline CPUs? This is something that's been discussed and I think things can be better, I'd rather the code was explicit whenever terms like "all", "online" and "any" were being used - it is very easy to confuse "all" and "any". I've been trying to adjust the code and make it more sensible with changes like:
https://lore.kernel.org/all/[email protected]/
but I agree with you that things are a mess here. At least we shouldn't be leaking memory (due to the reference count checking) and we shouldn't confuse indices and CPU numbers (due to the introduction of a wrapper type), problems that were pervasive and broke things like uncore counters frequently before.

Quoted 6 lines

Fwiw, part of me thinks we should change the perf_cpu_map to be a cpu_set_t, the "any" case could be an additional boolean in the struct. Perhaps we can just auto change "all" to "any" as is somewhat done here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/perf/evlist.c?h=perf-tools-next#n92
Then perf_cpu_map can be a cpu_set_t and I think life will be clearer.

Yeah, so when I looked at the data structure a few minutes ago:

        /**
         * A sized, reference counted, sorted array of integers representing CPU
         * numbers. This is commonly used to capture which CPUs a PMU is associated
         * with. The indices into the cpumap are frequently used as they avoid having
         * gaps if CPU numbers were used. For events associated with a pid, rather than
         * a CPU, a single dummy map with an entry of -1 is used.
         */
        DECLARE_RC_STRUCT(perf_cpu_map) {
                refcount_t      refcnt;
                /** Length of the map array. */
                int             nr;
                /** The CPU values. */
                struct perf_cpu map[];
        };

My first thought was "Why isn't this a bitmask?". :-)

The thing is, even with 8192 CPUs, the mask size is 1K. That's still not catastrophic compared to the nightmare of managing a [8192] array...

And I would keep such core data structures as simple as
possible: a single continuous bitmask. No sparse support, no NULL tricks. Just an always-present bitmask, and maybe an any/all modifier flag if the ABIs strongly suggest so.

The closer a data structure is to the core concepts of a tool, the more important its robust KISS properties are - at almost any runtime cost. </handwaving> :)

Quoted 3 lines

Fwiw, even on the kernel side this is borked as I tried to explain in this RFC with 0 follow ups:
https://lore.kernel.org/lkml/[email protected]/

So that's a neat patch and kudos on the ASCII graphics. :-)

I'd suggest a re-send.

Quoted 18 lines

Imagine the case of an event opened with 1 CPU and a TID. The filter will mean the counter only counts on the 1 CPU but the enabled and running time will mean the counter will be scaled (on your system) 128 times!
All of this matters when you consider different events opened in the same evlist and there being combined perf_cpu_maps like the evlist's all_cpus value.

tools/perf/builtin-stat.c static int perf_stat_init_aggr_mode(void) · line 1548

Quoted 5 lines

tools/perf/builtin-stat.c +4 -1

-       nr = perf_cpu_map__max(evsel_list->core.all_cpus).cpu + 1;
+       if (evsel_list->core.all_cpus)
+               nr = perf_cpu_map__max(evsel_list->core.all_cpus).cpu + 1;
+       else
+               nr = 0;

So I think the better fix is to make max handle a NULL perf_cpu_map. That will yield "any" CPU of -1 ( :-( ) and then the -1 + 1 will give an empty aggregation map.

Quoted 2 lines

I'll write this up, add the test and see if I can spot similar issues wrt NULL in the vicinity in the perf_cpu_map code.

Thank you! In the short run I'd prefer whichever fix can restore perf stat --null functionality the fastest. ;-)

Thanks,

Ingo