Re: [PATCH 2/2] perf stat: Fix a segfault with --per-cluster --metric-only

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Jun 27, 2024 at 01:03:53PM -0700, Namhyung Kim wrote:
> The new --per-cluster option was added recently but it forgot to update
> the aggr_header fields which are used for --metric-only option.  And it
> resulted in a segfault due to NULL string in fputs().

Before:

acme@number:~$ sudo ~acme/bin/perf stat -a -x : --per-cluster -M tma_core_bound --metric-only true
Segmentation fault
acme@number:~$ 
acme@number:~$ sudo su -
root@number:~# gdb perf
(gdb) run stat -a -x : --per-cluster -M tma_core_bound --metric-only true
Starting program: /root/bin/perf stat -a -x : --per-cluster -M tma_core_bound --metric-only true

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6f7f8dd in __strlen_avx2 () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff6f7f8dd in __strlen_avx2 () from /lib64/libc.so.6
#1  0x00007ffff6e97a3a in fputs () from /lib64/libc.so.6
#2  0x000000000056b805 in print_metric_headers ()
#3  0x000000000056e084 in evlist.print_counters ()
#4  0x0000000000432513 in cmd_stat ()
#5  0x00000000004c5fb9 in run_builtin ()
#6  0x00000000004c62c9 in handle_internal_command ()
#7  0x0000000000410e57 in main ()
(gdb)

After:

acme@number:~$ sudo ~acme/bin/perf stat -a -x : --per-cluster -M tma_core_bound --metric-only true
cluster:cpus:%  tma_core_bound:%  tma_core_bound:
S0-D0-CLS0:2:18.2::::
S0-D0-CLS8:2:26.7::::
S0-D0-CLS16:2:14.2::::
S0-D0-CLS24:2:10.6::::
S0-D0-CLS32:2:0.6::::
S0-D0-CLS40:2:42.5::::
S0-D0-CLS48:2:21.1::::
S0-D0-CLS56:2:36.8::::
S0-D0-CLS64:0:::::::1.0:
S0-D0-CLS72:0:::::::0.8:
S0-D0-CLS80:0:::::::1.0:
acme@number:~$

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Fixes: cbc917a1b03b ("perf stat: Support per-cluster aggregation")
> Cc: Yicong Yang <yangyicong@hisilicon.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/stat-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index e8673c9f6b49..462227f663cb 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -38,6 +38,7 @@
>  static int aggr_header_lens[] = {
>  	[AGGR_CORE] 	= 18,
>  	[AGGR_CACHE]	= 22,
> +	[AGGR_CLUSTER]	= 20,
>  	[AGGR_DIE] 	= 12,
>  	[AGGR_SOCKET] 	= 6,
>  	[AGGR_NODE] 	= 6,
> @@ -49,6 +50,7 @@ static int aggr_header_lens[] = {
>  static const char *aggr_header_csv[] = {
>  	[AGGR_CORE] 	= 	"core%scpus%s",
>  	[AGGR_CACHE]	= 	"cache%scpus%s",
> +	[AGGR_CLUSTER]	= 	"cluster%scpus%s",
>  	[AGGR_DIE] 	= 	"die%scpus%s",
>  	[AGGR_SOCKET] 	= 	"socket%scpus%s",
>  	[AGGR_NONE] 	= 	"cpu%s",
> @@ -60,6 +62,7 @@ static const char *aggr_header_csv[] = {
>  static int aggr_header_num[] = {
>  	[AGGR_CORE] 	= 	2,
>  	[AGGR_CACHE]	= 	2,
> +	[AGGR_CLUSTER]	= 	2,
>  	[AGGR_DIE] 	= 	2,
>  	[AGGR_SOCKET] 	= 	2,
>  	[AGGR_NONE] 	= 	1,
> -- 
> 2.45.2.803.g4e1b14247a-goog