Fix s390x compile error on F32 utils/stat-display.c
PATCH 1/1 Fix s390x compile error on F32 utils/stat-display.c
Fix a compile error on F32 and gcc version 10.1 on s390 in file utils/stat-display.c. The error does not show up with make DEBUG=y. In fact the issue shows up when using both compiler options -O6 and -D_FORTIFY_SOURCE=2 (which are omitted with DEBUG=Y).
This is the offending call chain:print_counter_aggr()
printout(config, -1, 0, ...) with 2nd parm id set to -1
aggr_printout(config, x, id --> -1, ...) which leads to this code:
case AGGR_NONE:
if (evsel->percore && !config->percore_show_thread) {
....
} else {
fprintf(config->output, "CPU%*d%s",
config->csv_output ? 0 : -7,
evsel__cpus(evsel)->map[id],
^^ id is -1 !!!!
config->csv_sep);
}This is a compiler inlining issue which is detected on s390 but not on other plattforms.
Output before:
# make util/stat-display.o
..... util/stat-display.c: In function ‘perf_evlist__print_counters’:
util/stat-display.c:121:4: error: array subscript -1 is below array
bounds of ‘int[]’ [-Werror=array-bounds]
121 | fprintf(config->output, "CPU%*d%s",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
122 | config->csv_output ? 0 : -7,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
123 | evsel__cpus(evsel)->map[id],
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
124 | config->csv_sep);
| ~~~~~~~~~~~~~~~~
In file included from util/evsel.h:13,
from util/evlist.h:13,
from util/stat-display.c:9:
/root/linux/tools/lib/perf/include/internal/cpumap.h:10:7:
note: while referencing ‘map’
10 | int map[];
| ^~~ cc1: all warnings being treated as errors
mv: cannot stat 'util/.stat-display.o.tmp': No such file or directory make[3]: *** [/root/linux/tools/build/Makefile.build:97: util/stat-display.o] Error 1
make[2]: *** [Makefile.perf:716: util/stat-display.o] Error 2 make[1]: *** [Makefile.perf:231: sub-make] Error 2
make: *** [Makefile:110: util/stat-display.o] Error 2 [root@t35lp46 perf]#
Output after:
# make util/stat-display.o
.....
CC util/stat-display.o
[root@t35lp46 perf]#--- tools/perf/util/stat-display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
tools/perf/util/stat-display.c +5 -4
@@ -118,10 +118,11 @@ static void aggr_printout(struct perf_stat_config *config, config->csv_output ?0 :-3 , cpu_map__id_to_cpu(id), config->csv_sep); }else { -fprintf (config->output,"CPU%*d%s" , - config->csv_output ?0 :-7 , -evsel__cpus (evsel)->map[id], - config->csv_sep); +if (id >-1 ) +fprintf (config->output,"CPU%*d%s" , + config->csv_output ?0 :-7 , +evsel__cpus (evsel)->map[id], + config->csv_sep); }break ;case AGGR_THREAD :
Signature · 1 line
2.26.2
Arnaldo Carvalho de Melo
maintainerEm Wed, Jul 22, 2020 at 11:20:53AM +0200, Thomas Richter escreveu:
Quoted 20 lines
Fix a compile error on F32 and gcc version 10.1 on s390 in file
utils/stat-display.c. The error does not show up with make DEBUG=y. In fact the issue shows up when using both compiler options -O6 and -D_FORTIFY_SOURCE=2 (which are omitted with DEBUG=Y).This is the offending call chain:
print_counter_aggr()
printout(config, -1, 0, ...)with 2nd parm id set to -1aggr_printout(config, x, id --> -1, ...) which leads to this code: case AGGR_NONE: if (evsel->percore && !config->percore_show_thread) { .... } else { fprintf(config->output, "CPU%*d%s", config->csv_output ? 0 : -7, evsel__cpus(evsel)->map[id], ^^ id is -1 !!!! config->csv_sep); }
Quoted 2 lines
This is a compiler inlining issue which is detected on s390 but not on other plattforms.
What is the sequence of events that gets to this? I.e. is it valid to get a config->aggr_mode == AGGR_NONE, then have evsel not be percore and config->percore_show_thread to be false?
I wonder if this won't be papering over some bug :-\
Jin?
This is where this came from:
commit 4fc4d8dfa056dfd48afe73b9ea3b7570ceb80b9c (tag: perf-core-for-mingo-5.2-20190517)
Author: Jin Yao <[email protected]> Date: Fri Apr 12 21:59:49 2019 +0800
perf stat: Support 'percore' event qualifier
With this patch, we can use the 'percore' event qualifier in perf-stat.
---
Also please add at least Jiri and Namhyung on the CC list, having the person that added that array usage also helps.
[acme@quaco perf]$ scripts/get_maintainer.pl tools/perf | grep reviewer
Mark Rutland <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Alexander Shishkin <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Jiri Olsa <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM)
Namhyung Kim <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) [acme@quaco perf]$
Thanks,
- Arnaldo
Quoted 65 lines
Output before:
# make util/stat-display.o .....
util/stat-display.c: In function ‘perf_evlist__print_counters’:
util/stat-display.c:121:4: error: array subscript -1 is below arraybounds of ‘int[]’ [-Werror=array-bounds] 121 | fprintf(config->output, "CPU%*d%s", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 122 | config->csv_output ? 0 : -7, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 123 | evsel__cpus(evsel)->map[id], | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 124 | config->csv_sep); | ~~~~~~~~~~~~~~~~ In file included from util/evsel.h:13, from util/evlist.h:13, from util/stat-display.c:9: /root/linux/tools/lib/perf/include/internal/cpumap.h:10:7: note: while referencing ‘map’ 10 | int map[]; | ^~~cc1: all warnings being treated as errors
mv: cannot stat 'util/.stat-display.o.tmp': No such file or directory make[3]: *** [/root/linux/tools/build/Makefile.build:97:util/stat-display.o] Error 1
make[2]: *** [Makefile.perf:716:util/stat-display.o] Error 2 make[1]: *** [Makefile.perf:231: sub-make] Error 2
make: *** [Makefile:110:util/stat-display.o] Error 2 [root@t35lp46 perf]#Output after:
# make util/stat-display.o ..... CC util/stat-display.o [root@t35lp46 perf]#Signed-off-by Thomas Richter <[email protected]>--- tools/perf/util/stat-display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)tools/perf/util/stat-display.c +5
-4@@ -118,10 +118,11 @@ static void aggr_printout(struct perf_stat_config *config, config->csv_output ?0 :-3 , cpu_map__id_to_cpu(id), config->csv_sep); }else { -fprintf (config->output,"CPU%*d%s" , - config->csv_output ?0 :-7 , -evsel__cpus (evsel)->map[id], - config->csv_sep); +if (id >-1 ) +fprintf (config->output,"CPU%*d%s" , + config->csv_output ?0 :-7 , +evsel__cpus (evsel)->map[id], + config->csv_sep); }break ;case AGGR_THREAD :Signature · 1 line
2.26.2
Signature · 1 line
- Arnaldo
Hi Arnaldo, Jin, Jiri
any update on this issue?
Thanks.
Quoted 123 lines · Arnaldo Carvalho de Melo
Em Wed, Jul 22, 2020 at 11:20:53AM +0200, Thomas Richter escreveu:
Quoted 20 lines
Fix a compile error on F32 and gcc version 10.1 on s390 in file
utils/stat-display.c. The error does not show up with make DEBUG=y. In fact the issue shows up when using both compiler options -O6 and -D_FORTIFY_SOURCE=2 (which are omitted with DEBUG=Y).This is the offending call chain:
print_counter_aggr()
printout(config, -1, 0, ...)with 2nd parm id set to -1aggr_printout(config, x, id --> -1, ...) which leads to this code: case AGGR_NONE: if (evsel->percore && !config->percore_show_thread) { .... } else { fprintf(config->output, "CPU%*d%s", config->csv_output ? 0 : -7, evsel__cpus(evsel)->map[id], ^^ id is -1 !!!! config->csv_sep); }Quoted 2 lines
This is a compiler inlining issue which is detected on s390 but not on other plattforms.
What is the sequence of events that gets to this? I.e. is it valid to get a
config->aggr_mode==AGGR_NONE, then have evsel not be percore andconfig->percore_show_threadto be false?I wonder if this won't be papering over some bug :-\
Jin?
This is where this came from:
commit4fc4d8dfa056dfd48afe73b9ea3b7570ceb80b9c(tag: perf-core-for-mingo-5.2-20190517)Author: Jin Yao <[email protected]> Date: Fri Apr 12 21:59:49 2019 +0800perf stat: Support 'percore' event qualifier
With this patch, we can use the 'percore' event qualifier in perf-stat.
---
Also please add at least Jiri and Namhyung on the CC list, having the person that added that array usage also helps.
[acme@quaco perf]$
scripts/get_maintainer.pltools/perf| grep reviewer
Mark Rutland <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Alexander Shishkin <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Jiri Olsa <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM)
Namhyung Kim <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) [acme@quaco perf]$Thanks,
- Arnaldo
Quoted 65 lines
Output before:
# make util/stat-display.o .....
util/stat-display.c: In function ‘perf_evlist__print_counters’:
util/stat-display.c:121:4: error: array subscript -1 is below arraybounds of ‘int[]’ [-Werror=array-bounds] 121 | fprintf(config->output, "CPU%*d%s", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 122 | config->csv_output ? 0 : -7, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 123 | evsel__cpus(evsel)->map[id], | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 124 | config->csv_sep); | ~~~~~~~~~~~~~~~~ In file included from util/evsel.h:13, from util/evlist.h:13, from util/stat-display.c:9: /root/linux/tools/lib/perf/include/internal/cpumap.h:10:7: note: while referencing ‘map’ 10 | int map[]; | ^~~cc1: all warnings being treated as errors
mv: cannot stat 'util/.stat-display.o.tmp': No such file or directory make[3]: *** [/root/linux/tools/build/Makefile.build:97:util/stat-display.o] Error 1
make[2]: *** [Makefile.perf:716:util/stat-display.o] Error 2 make[1]: *** [Makefile.perf:231: sub-make] Error 2
make: *** [Makefile:110:util/stat-display.o] Error 2 [root@t35lp46 perf]#Output after:
# make util/stat-display.o ..... CC util/stat-display.o [root@t35lp46 perf]#Signed-off-by Thomas Richter <[email protected]>--- tools/perf/util/stat-display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)tools/perf/util/stat-display.c +5
-4@@ -118,10 +118,11 @@ static void aggr_printout(struct perf_stat_config *config, config->csv_output ?0 :-3 , cpu_map__id_to_cpu(id), config->csv_sep); }else { -fprintf (config->output,"CPU%*d%s" , - config->csv_output ?0 :-7 , -evsel__cpus (evsel)->map[id], - config->csv_sep); +if (id >-1 ) +fprintf (config->output,"CPU%*d%s" , + config->csv_output ?0 :-7 , +evsel__cpus (evsel)->map[id], + config->csv_sep); }break ;case AGGR_THREAD :Signature · 1 line
2.26.2
Signature · 5 lines
Thomas Richter, Dept 3252, IBM s390 Linux Development, Boeblingen, Germany -- Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
Quoted 53 lines · Arnaldo Carvalho de Melo
Em Wed, Jul 22, 2020 at 11:20:53AM +0200, Thomas Richter escreveu:
Quoted 20 lines
Fix a compile error on F32 and gcc version 10.1 on s390 in file
utils/stat-display.c. The error does not show up with make DEBUG=y. In fact the issue shows up when using both compiler options -O6 and -D_FORTIFY_SOURCE=2 (which are omitted with DEBUG=Y).This is the offending call chain:
print_counter_aggr()
printout(config, -1, 0, ...)with 2nd parm id set to -1aggr_printout(config, x, id --> -1, ...) which leads to this code: case AGGR_NONE: if (evsel->percore && !config->percore_show_thread) { .... } else { fprintf(config->output, "CPU%*d%s", config->csv_output ? 0 : -7, evsel__cpus(evsel)->map[id], ^^ id is -1 !!!! config->csv_sep); }Quoted 2 lines
This is a compiler inlining issue which is detected on s390 but not on other plattforms.
What is the sequence of events that gets to this? I.e. is it valid to get a
config->aggr_mode==AGGR_NONE, then have evsel not be percore andconfig->percore_show_threadto be false?I wonder if this won't be papering over some bug :-\
Jin?
This is where this came from:
commit4fc4d8dfa056dfd48afe73b9ea3b7570ceb80b9c(tag: perf-core-for-mingo-5.2-20190517)Author: Jin Yao <[email protected]> Date: Fri Apr 12 21:59:49 2019 +0800perf stat: Support 'percore' event qualifier
With this patch, we can use the 'percore' event qualifier in perf-stat.
---
Also please add at least Jiri and Namhyung on the CC list, having the person that added that array usage also helps.
[acme@quaco perf]$
scripts/get_maintainer.pltools/perf| grep reviewer
Mark Rutland <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Alexander Shishkin <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Jiri Olsa <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM)
Namhyung Kim <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) [acme@quaco perf]$
Quoted 34 lines
Thanks,
- Arnaldo
Quoted 30 lines
Output before:
# make util/stat-display.o .....
util/stat-display.c: In function ‘perf_evlist__print_counters’:
util/stat-display.c:121:4: error: array subscript -1 is below arraybounds of ‘int[]’ [-Werror=array-bounds] 121 | fprintf(config->output, "CPU%*d%s", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 122 | config->csv_output ? 0 : -7, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 123 | evsel__cpus(evsel)->map[id], | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 124 | config->csv_sep); | ~~~~~~~~~~~~~~~~ In file included from util/evsel.h:13, from util/evlist.h:13, from util/stat-display.c:9: /root/linux/tools/lib/perf/include/internal/cpumap.h:10:7: note: while referencing ‘map’ 10 | int map[]; | ^~~cc1: all warnings being treated as errors
mv: cannot stat 'util/.stat-display.o.tmp': No such file or directory make[3]: *** [/root/linux/tools/build/Makefile.build:97:util/stat-display.o] Error 1
make[2]: *** [Makefile.perf:716:util/stat-display.o] Error 2 make[1]: *** [Makefile.perf:231: sub-make] Error 2
make: *** [Makefile:110:util/stat-display.o] Error 2 [root@t35lp46 perf]#
I tested on s390 with rhel8, but did not get this error, f32's gcc must be smarter
Quoted 21 lines
Quoted 21 lines
Output after:
# make util/stat-display.o ..... CC util/stat-display.o [root@t35lp46 perf]#Signed-off-by Thomas Richter <[email protected]>--- tools/perf/util/stat-display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)tools/perf/util/stat-display.c +0
-1@@ -118,10 +118,11 @@ static void aggr_printout(struct perf_stat_config *config, config->csv_output ?0 :-3 , cpu_map__id_to_cpu(id), config->csv_sep); }else { - fprintf(config->output, "CPU%*d%s",
Quoted 8 lines
tools/perf/util/stat-display.c static void aggr_printout(struct perf_stat_config *config, · line 125
Quoted 8 lines
tools/perf/util/stat-display.c +5
-3- config->csv_output ?0 :-7 , -evsel__cpus (evsel)->map[id], - config->csv_sep); +if (id >-1 ) +fprintf (config->output,"CPU%*d%s" , + config->csv_output ?0 :-7 , +evsel__cpus (evsel)->map[id], + config->csv_sep);
-1 is only through print_counter_aggr which is called for AGGR_GLOBAL case as you described, but I guess we can have this check as precaution
you could put it together with above else
} else if (id > -1)
thanks,
jirka
Quoted 89 lines · Jiri Olsa
Quoted 53 lines · Arnaldo Carvalho de Melo
Em Wed, Jul 22, 2020 at 11:20:53AM +0200, Thomas Richter escreveu:
Quoted 20 lines
Fix a compile error on F32 and gcc version 10.1 on s390 in file
utils/stat-display.c. The error does not show up with make DEBUG=y. In fact the issue shows up when using both compiler options -O6 and -D_FORTIFY_SOURCE=2 (which are omitted with DEBUG=Y).This is the offending call chain:
print_counter_aggr()
printout(config, -1, 0, ...)with 2nd parm id set to -1aggr_printout(config, x, id --> -1, ...) which leads to this code: case AGGR_NONE: if (evsel->percore && !config->percore_show_thread) { .... } else { fprintf(config->output, "CPU%*d%s", config->csv_output ? 0 : -7, evsel__cpus(evsel)->map[id], ^^ id is -1 !!!! config->csv_sep); }Quoted 2 lines
This is a compiler inlining issue which is detected on s390 but not on other plattforms.
What is the sequence of events that gets to this? I.e. is it valid to get a
config->aggr_mode==AGGR_NONE, then have evsel not be percore andconfig->percore_show_threadto be false?I wonder if this won't be papering over some bug :-\
Jin?
This is where this came from:
commit4fc4d8dfa056dfd48afe73b9ea3b7570ceb80b9c(tag: perf-core-for-mingo-5.2-20190517)Author: Jin Yao <[email protected]> Date: Fri Apr 12 21:59:49 2019 +0800perf stat: Support 'percore' event qualifier
With this patch, we can use the 'percore' event qualifier in perf-stat.
---
Also please add at least Jiri and Namhyung on the CC list, having the person that added that array usage also helps.
[acme@quaco perf]$
scripts/get_maintainer.pltools/perf| grep reviewer
Mark Rutland <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Alexander Shishkin <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) Jiri Olsa <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM)
Namhyung Kim <[email protected]> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) [acme@quaco perf]$Quoted 34 lines
Thanks,
- Arnaldo
Quoted 30 lines
Output before:
# make util/stat-display.o .....
util/stat-display.c: In function ‘perf_evlist__print_counters’:
util/stat-display.c:121:4: error: array subscript -1 is below arraybounds of ‘int[]’ [-Werror=array-bounds] 121 | fprintf(config->output, "CPU%*d%s", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 122 | config->csv_output ? 0 : -7, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 123 | evsel__cpus(evsel)->map[id], | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 124 | config->csv_sep); | ~~~~~~~~~~~~~~~~ In file included from util/evsel.h:13, from util/evlist.h:13, from util/stat-display.c:9: /root/linux/tools/lib/perf/include/internal/cpumap.h:10:7: note: while referencing ‘map’ 10 | int map[]; | ^~~cc1: all warnings being treated as errors
mv: cannot stat 'util/.stat-display.o.tmp': No such file or directory make[3]: *** [/root/linux/tools/build/Makefile.build:97:util/stat-display.o] Error 1
make[2]: *** [Makefile.perf:716:util/stat-display.o] Error 2 make[1]: *** [Makefile.perf:231: sub-make] Error 2
make: *** [Makefile:110:util/stat-display.o] Error 2 [root@t35lp46 perf]#
Quoted 3 lines
I tested on s390 with rhel8, but did not get this error, f32's gcc must be smarter
i Jiri,
that is correct. I have installed Fedora 32 on my LPAR and the gcc version is:
[root@t35lp46 ~]# gcc --version
gcc (GCC) 10.1.1 20200507 (Red Hat 10.1.1-1)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
[root@t35lp46 ~]# cat /etc/redhat-release
Fedora release 32 (Thirty Two)
[root@t35lp46 ~]#
Same commands on a RHEL 8.2 installation reveal
[root@m35lp76 ~]# gcc --version
gcc (GCC) 8.3.1 20191121 (Red Hat 8.3.1-5)
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
[root@m35lp76 ~]# cat /etc/redhat-release
Red Hat Enterprise Linux release 8.2 Beta (Ootpa)
[root@m35lp76 ~]#
So there are 2 major revisions difference between a F32 and a RHEL8 gcc compiler installation.
Quoted 33 lines
Quoted 21 lines
Quoted 21 lines
Output after:
# make util/stat-display.o ..... CC util/stat-display.o [root@t35lp46 perf]#Signed-off-by Thomas Richter <[email protected]>--- tools/perf/util/stat-display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)tools/perf/util/stat-display.c +0
-1@@ -118,10 +118,11 @@ static void aggr_printout(struct perf_stat_config *config, config->csv_output ?0 :-3 , cpu_map__id_to_cpu(id), config->csv_sep); }else { - fprintf(config->output, "CPU%*d%s",Quoted 8 lines
tools/perf/util/stat-display.c static void aggr_printout(struct perf_stat_config *config, · line 125
Quoted 8 lines
tools/perf/util/stat-display.c +5
-3- config->csv_output ?0 :-7 , -evsel__cpus (evsel)->map[id], - config->csv_sep); +if (id >-1 ) +fprintf (config->output,"CPU%*d%s" , + config->csv_output ?0 :-7 , +evsel__cpus (evsel)->map[id], + config->csv_sep);-1 is only through
print_counter_aggrwhich is called forAGGR_GLOBALcase as you described, but I guess we can have this check as precaution
Quoted 7 lines
you could put it together with above else
} else if (id > -1)thanks,
jirka
As suggested I will send a new patch.
Signature · 5 lines
Thomas Richter, Dept 3252, IBM s390 Linux Development, Boeblingen, Germany -- Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294