linux-kernel - Re: [PATCH 00/12] libperf: Add events to perf/event.h

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190826161849.GF24801@kernel.org>
Date:   Mon, 26 Aug 2019 13:18:49 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Michael Petlan <mpetlan@...hat.com>
Subject: Re: [PATCH 00/12] libperf: Add events to perf/event.h

Em Mon, Aug 26, 2019 at 01:06:28PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sun, Aug 25, 2019 at 08:17:40PM +0200, Jiri Olsa escreveu:
> > hi,
> > as a preparation for sampling libperf interface, moving event
> > definitions into the library header. Moving just the kernel 
> > non-AUX events now.
> > 
> > In order to keep libperf simple, we switch 'u64/u32/u16/u8'
> > types used events to their generic '__u*' versions.
> > 
> > Perf added 'u*' types mainly to ease up printing __u64 values
> > as stated in the linux/types.h comment:
> > 
> >   /*
> >    * We define u64 as uint64_t for every architecture
> >    * so that we can print it with "%"PRIx64 without getting warnings.
> >    *
> >    * typedef __u64 u64;
> >    * typedef __s64 s64;
> >    */
> > 
> > Adding and using new PRI_lu64 and PRI_lx64 macros to be used for
> > that.  Using extra '_' to ease up the reading and differentiate
> > them from standard PRI*64 macros.
> 
> So, this is not building on android env:
> 
> builtin-sched.c: In function 'process_lost':
> builtin-sched.c:2646:2: error: unknown conversion type character 'l' in format [-Werror=format=]
>   printf("lost %" PRI_lu64 " events on cpu %d\n", event->lost.lost, sample->cpu);
>   ^
> builtin-sched.c:2646:2: error: format '%d' expects argument of type 'int', but argument 2 has type '__u64' [-Werror=format=]
> builtin-sched.c:2646:2: error: too many arguments for format [-Werror=format-extra-args]
>   MKDIR    /tmp/build/perf/util/
> 
> 
> [perfbuilder@...c2c7bdaab ~]$ /opt/android-ndk-r12b//toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-gcc -v
> Using built-in specs.
> COLLECT_GCC=/opt/android-ndk-r12b//toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-gcc
> COLLECT_LTO_WRAPPER=/opt/android-ndk-r12b/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/../libexec/gcc/arm-linux-androideabi/4.9.x/lto-wrapper
> Target: arm-linux-androideabi
> Configured with: /usr/local/google/buildbot/src/android/gcc/toolchain/build/../gcc/gcc-4.9/configure --prefix=/tmp/59719db9ae19ff43aef46bbcb79596b6 --target=arm-linux-androideabi --host=x86_64-linux-gnu --build=x86_64-linux-gnu --with-gnu-as --with-gnu-ld --enable-languages=c,c++ --with-gmp=/buildbot/tmp/build/toolchain/temp-install --with-mpfr=/buildbot/tmp/build/toolchain/temp-install --with-mpc=/buildbot/tmp/build/toolchain/temp-install --with-cloog=/buildbot/tmp/build/toolchain/temp-install --with-isl=/buildbot/tmp/build/toolchain/temp-install --with-ppl=/buildbot/tmp/build/toolchain/temp-install --disable-ppl-version-check --disable-cloog-version-check --disable-isl-version-check --enable-cloog-backend=isl --with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm' --disable-libssp --enable-threads --disable-nls --disable-libmudflap --disable-libgomp --disable-libstdc__-v3 --disable-sjlj-exceptions --disable-shared --disable-tls --disable-libitm --with-float=soft --with-fpu=vfp --with-arch=armv5te --enable-target-optspace --enable-bionic-libs --enable-libatomic-ifuncs=no --enable-initfini-array --disable-nls --prefix=/tmp/59719db9ae19ff43aef46bbcb79596b6 --with-sysroot=/tmp/59719db9ae19ff43aef46bbcb79596b6/sysroot --with-binutils-version=2.25 --with-mpfr-version=3.1.1 --with-mpc-version=1.0.1 --with-gmp-version=5.0.5 --with-gcc-version=4.9 --with-gdb-version=none --with-gxx-include-dir=/tmp/59719db9ae19ff43aef46bbcb79596b6/include/c++/4.9.x --with-bugurl=http://source.android.com/source/report-bugs.html --enable-languages=c,c++ --disable-bootstrap --enable-plugins --enable-libgomp --enable-gnu-indirect-function --disable-libsanitizer --enable-gold --enable-threads --enable-eh-frame-hdr-for-static --enable-graphite=yes --with-isl-version=0.11.1 --with-cloog-version=0.18.0 --with-arch=armv5te --program-transform-name='s&^&arm-linux-androideabi-&' --enable-gold=default
> Thread model: posix
> gcc version 4.9.x 20150123 (prerelease) (GCC)
> [perfbuilder@...c2c7bdaab ~]$
> 
> It doesn't build on the r15b as well.
> 
> I'll investigate after lunch.

$ make $EXTRA_MAKE_ARGS ARCH=$ARCH CROSS_COMPILE=$CROSS_COMPILE EXTRA_CFLAGS="$EXTRA_CFLAGS" -C /git/perf/tools/perf O=/tmp/build/perf /tmp/build/perf/builtin-sched.i

We end up with one 'l' too many?

[perfbuilder@...c2c7bdaab ~]$ grep 'printf("lost' /tmp/build/perf/builtin-sched.i
 printf("lost %" "l" "ll""u" " events on cpu %d\n", event->lost.lost, sample->cpu);
[perfbuilder@...c2c7bdaab ~]$

And if we do this on a fedora:30 x86_64:

$ make -C tools/perf O=/tmp/build/perf /tmp/build/perf/builtin-sched.i
[acme@...co perf]$ grep -A4 'printf("lost' /tmp/build/perf/builtin-sched.i
 printf("lost %" "l" 
# 2646 "builtin-sched.c" 3 4
                "l" "u" 
# 2646 "builtin-sched.c"
                         " events on cpu %d\n", event->lost.lost, sample->cpu);
[acme@...co perf]$

I.e. on 32-bit arches we shouldn't add that extra "l", right?

I bet the build for the mips/mipsel will fail too, lemme see... Yeah,
both failed:


[root@...co ~]# grep -m1 -A6 -- -Werror=format=  dm.log/debian\:experimental-x-mips
builtin-sched.c:2646:9: error: unknown conversion type character 'l' in format [-Werror=format=]
  printf("lost %" PRI_lu64 " events on cpu %d\n", event->lost.lost, sample->cpu);
         ^~~~~~~~
In file included from builtin-sched.c:31:
/usr/mips-linux-gnu/include/inttypes.h:47:28: note: format string is defined here
 #  define __PRI64_PREFIX "ll"
                            ^
[root@...co ~]#

[root@...co ~]# grep -m1 -A6 -- -Werror=format=  dm.log/debian\:experimental-x-mipsel
builtin-sched.c:2646:9: error: unknown conversion type character 'l' in format [-Werror=format=]
  printf("lost %" PRI_lu64 " events on cpu %d\n", event->lost.lost, sample->cpu);
         ^~~~~~~~
In file included from builtin-sched.c:31:
/usr/mipsel-linux-gnu/include/inttypes.h:47:28: note: format string is defined here
 #  define __PRI64_PREFIX "ll"
                            ^
[root@...co ~]#

And also on a uclibc ARC arch container:

[root@...co ~]# grep -m1 -A6 -- -Werror=format=  dm.log/fedora\:24-x-ARC-uClibc
builtin-sched.c:2646:9: error: unknown conversion type character 'l' in format [-Werror=format=]
  printf("lost %" PRI_lu64 " events on cpu %d\n", event->lost.lost, sample->cpu);
         ^~~~~~~~
In file included from builtin-sched.c:31:0:
/arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install/arc-snps-linux-uclibc/sysroot/usr/include/inttypes.h:47:28: note: format string is defined here
 #  define __PRI64_PREFIX "ll"
                            ^
[root@...co ~]#

The _fix_ will come after lunch :)

- Arnaldo