From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2" Date: Wed, 25 Jul 2018 18:48:35 -0700 Message-ID: <20180725184835.5d37bef4@cakuba.netronome.com> References: <20180725072126.2232-1-tmricht@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: daniel@iogearbox.net, ast@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, heiko.carstens@de.ibm.com, brueckner@linux.vnet.ibm.com, schwidefsky@de.ibm.com, wangnan0@huawei.com To: Thomas Richter Return-path: Received: from mail-qt0-f196.google.com ([209.85.216.196]:46237 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728528AbeGZDDG (ORCPT ); Wed, 25 Jul 2018 23:03:06 -0400 Received: by mail-qt0-f196.google.com with SMTP id d4-v6so88874qtn.13 for ; Wed, 25 Jul 2018 18:48:41 -0700 (PDT) In-Reply-To: <20180725072126.2232-1-tmricht@linux.ibm.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 25 Jul 2018 09:21:26 +0200, Thomas Richter wrote: > commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own se= ctions") Hmm.. are you sure it's not 531b014e7a2f ("tools: bpf: make use of reallocarray") that caused the issue? That commit made us switch from XSI-compliant to GNU-specific strerror_r() implementation.. /me checks Yes it looks like 531b014e7a2f~ builds just fine. Daniel, did you try to apply v1 to the bpf tree? Perhaps there is a confusion about the trees here, if this is caused by my recent change it's a bpf-next material. strerror() works, but strerror_r() seems nicer, so perhaps we could keep it if the patch worked in bpf-next? > causes a compiler error when building the perf tool in the linux-next tre= e. > I compile it using a FEDORA 28 installation, my gcc compiler version: > gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20) >=20 > The file that causes the error is tools/lib/bpf/libbpf.c >=20 > Here is the error message: >=20 > [root@p23lp27] # make V=3D1 EXTRA_CFLAGS=3D"-Wp,-D_FORTIFY_SOURCE=3D2 -O2" > [...] > make -f /home6/tmricht/linux-next/tools/build/Makefile.build > dir=3D./util/scripting-engines obj=3Dlibperf > libbpf.c: In function =E2=80=98bpf_object__elf_collect=E2=80=99: > libbpf.c:811:15: error: ignoring return value of =E2=80=98strerror_r=E2= =80=99, > declared with attribute warn_unused_result [-Werror=3Dunused-result] > strerror_r(-err, errmsg, sizeof(errmsg)); > ^ > cc1: all warnings being treated as errors > mv: cannot stat './.libbpf.o.tmp': No such file or directory > /home6/tmricht/linux-next/tools/build/Makefile.build:96: recipe for targe= t 'libbpf.o' failed >=20 > Since this is the only occurance of strerror_r() replace it > by strerror(). The additional functionality of strerror_r() to > copy the error message into the supplied buffer is not needed. > This is also consistant with all the other pr_warning() statements > in this file which all use strerror(). >=20 > Also fixes a possible initialization issue. >=20 > Cc: Wang Nan > Cc: Alexei Starovoitov > Cc: Daniel Borkmann > Signed-off-by: Thomas Richter > > tools/lib/bpf/libbpf.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) >=20 > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 955f8eafbf41..f9eb68ff2f4f 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -806,11 +806,8 @@ static int bpf_object__elf_collect(struct bpf_object= *obj) > err =3D bpf_object__add_program(obj, data->d_buf, > data->d_size, name, idx); > if (err) { > - char errmsg[STRERR_BUFSIZE]; > - > - strerror_r(-err, errmsg, sizeof(errmsg)); > pr_warning("failed to alloc program %s (%s): %s", > - name, obj->path, errmsg); > + name, obj->path, strerror(-err)); > } > } else if (sh.sh_type =3D=3D SHT_REL) { > void *reloc =3D obj->efile.reloc; > @@ -2334,7 +2331,7 @@ bpf_perf_event_read_simple(void *mem, unsigned long= size, > __u64 data_tail =3D header->data_tail; > __u64 data_head =3D header->data_head; > void *base, *begin, *end; > - int ret; > + int ret =3D 0; > =20 > asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */ > if (data_head =3D=3D data_tail) This looks like a separate issue. The ret variable should really be enum bpf_perf_event_ret, so could you please initialize it to one of the values of this enum? The uninitilized condition can only happen if (data_head !=3D data_tail) but at the same time (data_head % size =3D=3D data_tail % size) which should never really happen... Perhaps initializing to LIBBPF_PERF_EVENT_ERROR would make sense? Or better still adding: diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index f732237610e5..fa5a25945f19 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -2289,6 +2289,8 @@ bpf_perf_event_read_simple(void *mem, unsigned long s= ize, =20 begin =3D base + data_tail % size; end =3D base + data_head % size; + if (being =3D=3D end) + return LIBBPF_PERF_EVENT_ERROR; =20 while (begin !=3D end) { struct perf_event_header *ehdr;