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] [day] [month] [year] [list]
Message-ID: <b57142d5ada4a852836f7c0bcbd9cffc4fb8681e.camel@gmail.com>
Date:   Mon, 26 Jul 2021 12:43:21 +0200
From:   Riccardo Mancini <rickyman7@...il.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Namhyung Kim <namhyung@...nel.org>,
        Ian Rogers <irogers@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 1/3] tools libc_compat: add gettid

Hi Arnaldo,

as reported by the kernel test robot, I forgot to include sys/syscall.h and
unistd.h, which were included by some other headers in perf but is causing
troubles in the bpf selftest.
However, this is not all, since bpf tests are still failing since they do not
use HAVE_* macros, therefore gettid gets defined twice (in libc and
libc_compat.h).
Since, afaict, bpf is no longer using libc_compat.h [1] (perf is be the only
user atm), I would remove its dependency on libc_compat.h.
Furthermore, this made me also think whether the HAVE_* or NEED_* macros are
more suited for this case (reallocarray is using COMPAT_NEED_REALLOCARRAY,
gettid is using HAVE_GETTID). I believe HAVE_* are better since compilation
fails if they are missing or misused (as in this case), while NEED_* failures
may be more subtle (ie. only happening with older versions of libc). Therefore,
I think we should also change COMPAT_NEED_REALLOCARRAY to HAVE_REALLOCARRAY.
What do you think?

[1] https://lore.kernel.org/bpf/20200819013607.3607269-2-andriin@fb.com/

Thanks,
Riccardo

On Mon, 2021-07-26 at 06:04 +0800, kernel test robot wrote:
> Hi Riccardo,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on tip/perf/core]
> [also build test ERROR on linux/master linus/master v5.14-rc2 next-20210723]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:   
> https://github.com/0day-ci/linux/commits/Riccardo-Mancini/tools-add-gettid-to-libc_compat-h/20210722-233601
> base:  
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git c76826a65f50038f0504
> 24365dbf3f97203f8710
> config: x86_64-rhel-8.3-kselftests (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         #
> https://github.com/0day-ci/linux/commit/42df183984cce4c25932242bbf9133684e9425db
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Riccardo-Mancini/tools-add-gettid-to-
> libc_compat-h/20210722-233601
>         git checkout 42df183984cce4c25932242bbf9133684e9425db
>         # save the attached .config to linux build tree
>         mkdir build_dir
>         make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash -C
> tools/testing/selftests/bpf install
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@...el.com>
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from main.h:15,
>                     from xlated_dumper.c:14:
>    tools/include/tools/libc_compat.h: In function 'gettid':
>    tools/include/tools/libc_compat.h:24:16: warning: implicit declaration of
> function 'syscall' [-Wimplicit-function-declaration]
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                ^~~~~~~
> > > tools/include/tools/libc_compat.h:24:24: error: '__NR_gettid' undeclared
> > > (first use in this function)
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                        ^~~~~~~~~~~
>    tools/include/tools/libc_compat.h:24:24: note: each undeclared identifier is
> reported only once for each function it appears in
> --
>    In file included from main.h:15,
>                     from common.c:27:
> > > tools/include/tools/libc_compat.h:22:21: error: static declaration of
> > > 'gettid' follows non-static declaration
>       22 | static inline pid_t gettid(void)
>          |                     ^~~~~~
>    In file included from /usr/include/unistd.h:1170,
>                     from common.c:15:
>    /usr/include/x86_64-linux-gnu/bits/unistd_ext.h:34:16: note: previous
> declaration of 'gettid' was here
>       34 | extern __pid_t gettid (void) __THROW;
>          |                ^~~~~~
>    In file included from main.h:15,
>                     from common.c:27:
>    tools/include/tools/libc_compat.h: In function 'gettid':
> > > tools/include/tools/libc_compat.h:24:24: error: '__NR_gettid' undeclared
> > > (first use in this function)
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                        ^~~~~~~~~~~
>    tools/include/tools/libc_compat.h:24:24: note: each undeclared identifier is
> reported only once for each function it appears in
> --
>    In file included from main.h:15,
>                     from btf.c:20:
>    tools/include/tools/libc_compat.h: In function 'gettid':
> > > tools/include/tools/libc_compat.h:24:24: error: '__NR_gettid' undeclared
> > > (first use in this function)
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                        ^~~~~~~~~~~
>    tools/include/tools/libc_compat.h:24:24: note: each undeclared identifier is
> reported only once for each function it appears in
> 
> 
> vim +/__NR_gettid +24 tools/include/tools/libc_compat.h
> 
>     20  
>     21  #ifndef HAVE_GETTID
>   > 22  static inline pid_t gettid(void)
>     23  {
>   > 24          return (pid_t)syscall(__NR_gettid);
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ