| 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: Mon, 24 Aug 2020 22:22:15 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Jin Yao <yao.jin@...ux.intel.com>,
Thomas Richter <tmricht@...ux.ibm.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 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
> >
> > 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
Powered by blists - more mailing lists