lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 25 Aug 2020 08:15:34 +0200 From: Thomas Richter <tmricht@...ux.ibm.com> To: Jiri Olsa <jolsa@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org> Cc: Jin Yao <yao.jin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>, linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, svens@...ux.ibm.com, gor@...ux.ibm.com, sumanthk@...ux.ibm.com, heiko.carstens@...ibm.com Subject: Re: [PATCH] Fix s390x compile error on F32 utils/stat-display.c On 8/24/20 10:22 PM, Jiri Olsa wrote: > On Wed, Aug 12, 2020 at 08:27:08AM -0300, Arnaldo Carvalho de Melo wrote: >> Em Wed, Jul 22, 2020 at 11:20:53AM +0200, Thomas Richter escreveu: >>> 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. >> >> 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 <yao.jin@...ux.intel.com> >> 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@...co perf]$ scripts/get_maintainer.pl tools/perf | grep reviewer >> Mark Rutland <mark.rutland@....com> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) >> Alexander Shishkin <alexander.shishkin@...ux.intel.com> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) >> Jiri Olsa <jolsa@...hat.com> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) >> Namhyung Kim <namhyung@...nel.org> (reviewer:PERFORMANCE EVENTS SUBSYSTEM) >> [acme@...co perf]$ >> >> Thanks, >> >> - Arnaldo >> >>> 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@...lp46 perf]# > > 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@...lp46 ~]# 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@...lp46 ~]# cat /etc/redhat-release Fedora release 32 (Thirty Two) [root@...lp46 ~]# Same commands on a RHEL 8.2 installation reveal [root@...lp76 ~]# 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@...lp76 ~]# cat /etc/redhat-release Red Hat Enterprise Linux release 8.2 Beta (Ootpa) [root@...lp76 ~]# So there are 2 major revisions difference between a F32 and a RHEL8 gcc compiler installation. >>> >>> Output after: >>> # make util/stat-display.o >>> ..... >>> CC util/stat-display.o >>> [root@...lp46 perf]# >>> >>> Signed-off-by: Thomas Richter <tmricht@...ux.ibm.com> >>> --- >>> tools/perf/util/stat-display.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c >>> index 57d0706e1330..e49e544188e4 100644 >>> --- a/tools/perf/util/stat-display.c >>> +++ b/tools/perf/util/stat-display.c >>> @@ -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); > > -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 > As suggested I will send a new patch. -- 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
Powered by blists - more mailing lists