linux-kernel - Re: perf tools build broken after v5.1-rc1

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]
Date:   Thu, 2 May 2019 12:41:04 -0400
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Vineet Gupta <Vineet.Gupta1@...opsys.com>
Cc:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
        Arnd Bergmann <arnd@...db.de>, Rich Felker <dalias@...c.org>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        lkml <linux-kernel@...r.kernel.org>,
        "linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        arcml <linux-snps-arc@...ts.infradead.org>
Subject: Re: perf tools build broken after v5.1-rc1

Em Thu, May 02, 2019 at 04:09:43PM +0000, Vineet Gupta escreveu:
> On 5/2/19 7:36 AM, Arnaldo Carvalho de Melo wrote:
> > Em Wed, May 01, 2019 at 09:17:52PM +0000, Vineet Gupta escreveu:
> >> On 5/1/19 1:41 PM, Arnaldo Carvalho de Melo wrote:
> >>>> The 1a787fc5ba18ac7 commit copied over the changes for arm64, but
> >>>> missed all the other architectures changed in c8ce48f06503 and the
> >>>> related commits.
> >>> Right, I have a patch copying the missing headers, and that fixed the
> >>> build with the glibc-based toolchain, but then broke the uCLibc one :-\
> >  
> >> tools/perf/util/cloexec.c  #includes <sys/syscall.h> which for glibc includes
> >> asm/unistd.h
> >  
> >> uClibc <sys/syscall.h> OTOH #include <bits/sysnum.h> containign#define __NR_*
> >> (generated by parsing kernel's unistd). This header does the right thing by
> >> chekcing for redefs, but in the end we still collide with newly added
> >> tools/arc/arc/*/**/unistd.h which doesn't have conditional definitions. I'm sure
> >> this is not an ARC problem, any uClibc build would be affected. Do you have a arm
> >> uclibc toolchain to test ?
> > This solves it for fedora:29,
> > arc_gnu_2017.09-rc2_prebuilt_uclibc_le_arc700_linux_install,
> > arc_gnu_2019.03-rc1_prebuilt_uclibc_le_archs_linux_install and
> > arc_gnu_2019.03-rc1_prebuilt_glibc_le_archs_linux_install.
> >
> > Also ok with:
> >
> >   make -C tools/perf build-test
> >
> > Now build testing with the full set of containers.
> >
> > - Arnaldo
> >
> > commit 1931594a680dba28e98b526192dd065430c850c0
> > Author: Arnaldo Carvalho de Melo <acme@...hat.com>
> > Date:   Thu May 2 09:26:23 2019 -0400
> >
> >     perf tools: Remove needless asm/unistd.h include fixing build in some places
> >     
> >     We were including sys/syscall.h and asm/unistd.h, since sys/syscall.h
> >     includes asm/unistd.h, sometimes this leads to the redefinition of
> >     defines, breaking the build.
> >     
> >     Noticed on ARC with uCLibc.
> 
> Thx for this Arnaldo.
> 
> While this takes care of immediate issues, for the long term, are you open to idea
> of removing the header duplicity.

In the beginning we used the kernel headers directly, then, acting on
advice/complaints from Linus about tooling breaking when changes were
made in the kernel sources we were using directly, we moved to have
copies and notice when things change so that we could think about what
changed and act accordingly, without putting the burden to the kernel
developers to keep tools/ building, I want to keep it that way.

Now you say, validly, that there are bits that are designed to be used
by userspace, so for those, we should go back to not copying and using
it direcly, elliminating the duplicity you don't like.

I don't know, I'm used to the duplicity and the checks, not breaking
tools even when kernel developers make mistakes in the UAPI headers,
tools/perf is self container wrt the latest and greatest stuff not
present in older environments, and the onus is on perf developers to do
the sync.

This specific issue here happened because I made a mistake, which I
fixed when reported, now I have three containers for cross building for
ARC, two versions for the uCLibc based toolchain, one for the glibc one,
libnuma, elfutils and zlib are cross build there, so should make it less
likely problems like this will happen again.

> We could use a "less evil" idiom of copying only the minimal bits (since the sync
> onus remains one way or the other)
> e.g. I spotted below in bpf code and also seen in other ah-hoc multi arch projects
 
> #ifdef __NR_xx
> # if defined (__arch_y__)
> 
> # elif defined (__arch_z__)
> 
> # endif
> #endif

- Arnaldo

BTW: since the last report:

  25 fedora:24-x-ARC-uClibc        : Ok   arc-linux-gcc (ARCompact ISA Linux uClibc toolchain 2017.09-rc2) 7.1.1 20170710
  26 fedora:25                     : Ok   gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
  27 fedora:26                     : Ok   gcc (GCC) 7.3.1 20180130 (Red Hat 7.3.1-2)
  28 fedora:28                     : Ok   gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)
  29 fedora:29                     : Ok   gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)
  30 fedora:30                     : Ok   gcc (GCC) 9.0.1 20190312 (Red Hat 9.0.1-0.10)
  31 fedora:30-x-ARC-glibc         : Ok   arc-linux-gcc (ARC HS GNU/Linux glibc toolchain 2019.03-rc1) 8.3.1 20190225
  32 fedora:30-x-ARC-uClibc        : Ok   arc-linux-gcc (ARCv2 ISA Linux uClibc toolchain 2019.03-rc1) 8.3.1 20190225