2019-06-12 19:23:38

by Laura Abbott

[permalink] [raw]
Subject: perf build failure with newer glibc headers

Hi,

While doing some build experiments, I found a compile failure with perf and jvmti:

BUILDSTDERR: gcc -Wp,-MD,./.xsk.o.d -Wp,-MT,xsk.o -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-jvmti/jvmti_agent.c:48:21: error: static declaration of 'gettid' follows non-static declaration
BUILDSTDERR: 48 | static inline pid_t gettid(void)
BUILDSTDERR: | ^~~~~~
BUILDSTDERR: In file included from /usr/include/unistd.h:1170,
BUILDSTDERR: from jvmti/jvmti_agent.c:33:
BUILDSTDERR: /usr/include/bits/unistd_ext.h:40:16: note: previous declaration of 'gettid' was here
BUILDSTDERR: 40 | extern __pid_t gettid (void) __THROW;
BUILDSTDERR: | ^~~~~~


This is with the newer glibc headers that came into Fedora earlier this week
(glibc-2.29.9000-27.fc31) It looks like the newer headers now define gettid
so the in file gettid no longer works. Note this was a custom build with
jvmti enabled as regular Fedora doesn't have it enabled which is why this
wasn't reported elsewhere.

I don't know enough about either the glibc headers or perf to make a suggestion
on how to fix this but I'm happy to test.

Thanks,
Laura


2019-06-12 20:58:19

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

Em Wed, Jun 12, 2019 at 03:23:12PM -0400, Laura Abbott escreveu:
> Hi,
>
> While doing some build experiments, I found a compile failure with perf and jvmti:
>
> BUILDSTDERR: gcc -Wp,-MD,./.xsk.o.d -Wp,-MT,xsk.o -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-jvmti/jvmti_agent.c:48:21: error: static declaration of 'gettid' follows non-static declaration
> BUILDSTDERR: 48 | static inline pid_t gettid(void)
> BUILDSTDERR: | ^~~~~~
> BUILDSTDERR: In file included from /usr/include/unistd.h:1170,
> BUILDSTDERR: from jvmti/jvmti_agent.c:33:
> BUILDSTDERR: /usr/include/bits/unistd_ext.h:40:16: note: previous declaration of 'gettid' was here
> BUILDSTDERR: 40 | extern __pid_t gettid (void) __THROW;
> BUILDSTDERR: | ^~~~~~
>
>
> This is with the newer glibc headers that came into Fedora earlier this week
> (glibc-2.29.9000-27.fc31) It looks like the newer headers now define gettid
> so the in file gettid no longer works. Note this was a custom build with
> jvmti enabled as regular Fedora doesn't have it enabled which is why this
> wasn't reported elsewhere.
>
> I don't know enough about either the glibc headers or perf to make a suggestion
> on how to fix this but I'm happy to test.

Bummer, I haven't noticed this because my fedora:rawhide perf build test
container wasn't building the jvmti code:

Makefile.config:925: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel

i.e.:

[perfbuilder@c0326e8b6511 perf]$ cat /tmp/build/perf/feature/test-jvmti.make.output
test-jvmti.c:2:10: fatal error: jvmti.h: No such file or directory
2 | #include <jvmti.h>
| ^~~~~~~~~
compilation terminated.
[perfbuilder@c0326e8b6511 perf]$

Installing it I get:

[root@2d7fe307ad20 perf]# rpm -qa | grep openjdk
java-1.8.0-openjdk-1.8.0.212.b04-4.fc31.x86_64
java-1.8.0-openjdk-headless-1.8.0.212.b04-4.fc31.x86_64
java-1.8.0-openjdk-devel-1.8.0.212.b04-4.fc31.x86_64
[root@2d7fe307ad20 perf]# cat
/tmp/build/perf/feature/test-jvmti.make.output
[root@2d7fe307ad20 perf]# ls -la /tmp/build/perf/feature/test-jvmti.bin
-rwxr-xr-x. 1 root root 21592 Jun 12 20:48
/tmp/build/perf/feature/test-jvmti.bin
[root@2d7fe307ad20 perf]#

And reproduce the problem you reported:

jvmti/jvmti_agent.c:48:21: error: static declaration of ‘gettid’ follows
non-static declaration
48 | static inline pid_t gettid(void)
| ^~~~~~
In file included from /usr/include/unistd.h:1170,
from jvmti/jvmti_agent.c:33:

So, we'll have to have a feature test, that defines some HAVE_GETTID
that then ifdefs out our inline copy, working on it.

Thanks for the report!

- Arnaldo

2019-06-13 15:22:21

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

Em Wed, Jun 12, 2019 at 05:56:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> So, we'll have to have a feature test, that defines some HAVE_GETTID
> that then ifdefs out our inline copy, working on it.

This should take care of it, please check, perhaps providing a
Tested-by: to add to this,

Thanks,

- Arnaldo

commit a04ef2eb0a66d9479e75e536d919c8c9cd618ee3
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Thu Jun 13 12:04:19 2019 -0300

tools build: Check if gettid() is available before providing helper

Laura reported that the perf build failed in fedora when we got a glibc
that provides gettid(), which I reproduced using fedora rawhide with the
glibc-devel-2.29.9000-26.fc31.x86_64 package.

Add a feature check to avoid providing a gettid() helper in such
systems.

On a fedora rawhide system with this patch applied we now get:

[root@7a5f55352234 perf]# grep gettid /tmp/build/perf/FEATURE-DUMP
feature-gettid=1
[root@7a5f55352234 perf]# cat /tmp/build/perf/feature/test-gettid.make.output
[root@7a5f55352234 perf]# ldd /tmp/build/perf/feature/test-gettid.bin
linux-vdso.so.1 (0x00007ffc6b1f6000)
libc.so.6 => /lib64/libc.so.6 (0x00007f04e0a74000)
/lib64/ld-linux-x86-64.so.2 (0x00007f04e0c47000)
[root@7a5f55352234 perf]# nm /tmp/build/perf/feature/test-gettid.bin | grep -w gettid
U gettid@@GLIBC_2.30
[root@7a5f55352234 perf]#

While on a fedora:29 system:

[acme@quaco perf]$ grep gettid /tmp/build/perf/FEATURE-DUMP
feature-gettid=0
[acme@quaco perf]$ cat /tmp/build/perf/feature/test-gettid.make.output
test-gettid.c: In function ‘main’:
test-gettid.c:8:9: error: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Werror=implicit-function-declaration]
return gettid();
^~~~~~
getgid
cc1: all warnings being treated as errors
[acme@quaco perf]$

Reported-by: Laura Abbott <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Florian Weimer <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: https://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 3b24231c58a2..50377cc2f5f9 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -36,6 +36,7 @@ FEATURE_TESTS_BASIC := \
fortify-source \
sync-compare-and-swap \
get_current_dir_name \
+ gettid \
glibc \
gtk2 \
gtk2-infobar \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 4b8244ee65ce..523ee42db0c8 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -54,6 +54,7 @@ FILES= \
test-get_cpuid.bin \
test-sdt.bin \
test-cxx.bin \
+ test-gettid.bin \
test-jvmti.bin \
test-jvmti-cmlr.bin \
test-sched_getcpu.bin \
@@ -267,6 +268,9 @@ $(OUTPUT)test-sdt.bin:
$(OUTPUT)test-cxx.bin:
$(BUILDXX) -std=gnu++11

+$(OUTPUT)test-gettid.bin:
+ $(BUILD)
+
$(OUTPUT)test-jvmti.bin:
$(BUILD)

diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index a59c53705093..3b3d5d72124a 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -38,6 +38,10 @@
# include "test-get_current_dir_name.c"
#undef main

+#define main main_test_gettid
+# include "test-gettid.c"
+#undef main
+
#define main main_test_glibc
# include "test-glibc.c"
#undef main
@@ -195,6 +199,7 @@ int main(int argc, char *argv[])
main_test_libelf();
main_test_libelf_mmap();
main_test_get_current_dir_name();
+ main_test_gettid();
main_test_glibc();
main_test_dwarf();
main_test_dwarf_getlocations();
diff --git a/tools/build/feature/test-gettid.c b/tools/build/feature/test-gettid.c
new file mode 100644
index 000000000000..ef24e42d3f1b
--- /dev/null
+++ b/tools/build/feature/test-gettid.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019, Red Hat Inc, Arnaldo Carvalho de Melo <[email protected]>
+#define _GNU_SOURCE
+#include <unistd.h>
+
+int main(void)
+{
+ return gettid();
+}
+
+#undef _GNU_SOURCE
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 51dd00f65709..5f16a20cae86 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -332,6 +332,10 @@ ifeq ($(feature-get_current_dir_name), 1)
CFLAGS += -DHAVE_GET_CURRENT_DIR_NAME
endif

+ifeq ($(feature-gettid), 1)
+ CFLAGS += -DHAVE_GETTID
+endif
+
ifdef NO_LIBELF
NO_DWARF := 1
NO_DEMANGLE := 1
diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
index f7eb63cbbc65..88108598d6e9 100644
--- a/tools/perf/jvmti/jvmti_agent.c
+++ b/tools/perf/jvmti/jvmti_agent.c
@@ -45,10 +45,12 @@
static char jit_path[PATH_MAX];
static void *marker_addr;

+#ifndef HAVE_GETTID
static inline pid_t gettid(void)
{
return (pid_t)syscall(__NR_gettid);
}
+#endif

static int get_e_machine(struct jitheader *hdr)
{

2019-06-13 17:17:02

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

On Wed, Jun 12, 2019 at 05:56:11PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 12, 2019 at 03:23:12PM -0400, Laura Abbott escreveu:
> > Hi,
> >
> > While doing some build experiments, I found a compile failure with perf and jvmti:
> >
> > BUILDSTDERR: gcc -Wp,-MD,./.xsk.o.d -Wp,-MT,xsk.o -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-jvmti/jvmti_agent.c:48:21: error: static declaration of 'gettid' follows non-static declaration
> > BUILDSTDERR: 48 | static inline pid_t gettid(void)
> > BUILDSTDERR: | ^~~~~~
> > BUILDSTDERR: In file included from /usr/include/unistd.h:1170,
> > BUILDSTDERR: from jvmti/jvmti_agent.c:33:
> > BUILDSTDERR: /usr/include/bits/unistd_ext.h:40:16: note: previous declaration of 'gettid' was here
> > BUILDSTDERR: 40 | extern __pid_t gettid (void) __THROW;
> > BUILDSTDERR: | ^~~~~~
> >
> >
> > This is with the newer glibc headers that came into Fedora earlier this week
> > (glibc-2.29.9000-27.fc31) It looks like the newer headers now define gettid
> > so the in file gettid no longer works. Note this was a custom build with
> > jvmti enabled as regular Fedora doesn't have it enabled which is why this
> > wasn't reported elsewhere.
> >
> > I don't know enough about either the glibc headers or perf to make a suggestion
> > on how to fix this but I'm happy to test.
>
> Bummer, I haven't noticed this because my fedora:rawhide perf build test
> container wasn't building the jvmti code:
>
> Makefile.config:925: No openjdk development package found, please install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel
>
> i.e.:
>
> [perfbuilder@c0326e8b6511 perf]$ cat /tmp/build/perf/feature/test-jvmti.make.output
> test-jvmti.c:2:10: fatal error: jvmti.h: No such file or directory
> 2 | #include <jvmti.h>
> | ^~~~~~~~~
> compilation terminated.
> [perfbuilder@c0326e8b6511 perf]$
>
> Installing it I get:
>
> [root@2d7fe307ad20 perf]# rpm -qa | grep openjdk
> java-1.8.0-openjdk-1.8.0.212.b04-4.fc31.x86_64
> java-1.8.0-openjdk-headless-1.8.0.212.b04-4.fc31.x86_64
> java-1.8.0-openjdk-devel-1.8.0.212.b04-4.fc31.x86_64
> [root@2d7fe307ad20 perf]# cat
> /tmp/build/perf/feature/test-jvmti.make.output
> [root@2d7fe307ad20 perf]# ls -la /tmp/build/perf/feature/test-jvmti.bin
> -rwxr-xr-x. 1 root root 21592 Jun 12 20:48
> /tmp/build/perf/feature/test-jvmti.bin
> [root@2d7fe307ad20 perf]#
>
> And reproduce the problem you reported:
>
> jvmti/jvmti_agent.c:48:21: error: static declaration of ‘gettid’ follows
> non-static declaration
> 48 | static inline pid_t gettid(void)
> | ^~~~~~
> In file included from /usr/include/unistd.h:1170,
> from jvmti/jvmti_agent.c:33:
>
> So, we'll have to have a feature test, that defines some HAVE_GETTID
> that then ifdefs out our inline copy, working on it.

ok, I did not see your email before I posted my reply,
feature test is better, of course ;-)

jirka

2019-06-13 17:17:15

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

On Wed, Jun 12, 2019 at 03:23:12PM -0400, Laura Abbott wrote:
> Hi,
>
> While doing some build experiments, I found a compile failure with perf and jvmti:
>
> BUILDSTDERR: gcc -Wp,-MD,./.xsk.o.d -Wp,-MT,xsk.o -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-jvmti/jvmti_agent.c:48:21: error: static declaration of 'gettid' follows non-static declaration
> BUILDSTDERR: 48 | static inline pid_t gettid(void)
> BUILDSTDERR: | ^~~~~~
> BUILDSTDERR: In file included from /usr/include/unistd.h:1170,
> BUILDSTDERR: from jvmti/jvmti_agent.c:33:
> BUILDSTDERR: /usr/include/bits/unistd_ext.h:40:16: note: previous declaration of 'gettid' was here
> BUILDSTDERR: 40 | extern __pid_t gettid (void) __THROW;
> BUILDSTDERR: | ^~~~~~
>
>
> This is with the newer glibc headers that came into Fedora earlier this week
> (glibc-2.29.9000-27.fc31) It looks like the newer headers now define gettid
> so the in file gettid no longer works. Note this was a custom build with
> jvmti enabled as regular Fedora doesn't have it enabled which is why this
> wasn't reported elsewhere.

hum, I guess we need some version macro conditions
if that's the case

so this glibc version is available on rawhide now?
I'll try to get some server with it

thanks,
jirka

>
> I don't know enough about either the glibc headers or perf to make a suggestion
> on how to fix this but I'm happy to test.
>
> Thanks,
> Laura

2019-06-13 19:26:41

by Laura Abbott

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

On 6/13/19 11:19 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 12, 2019 at 05:56:11PM -0300, Arnaldo Carvalho de Melo escreveu:
>> So, we'll have to have a feature test, that defines some HAVE_GETTID
>> that then ifdefs out our inline copy, working on it.
>
> This should take care of it, please check, perhaps providing a
> Tested-by: to add to this,
>

built okay on the tree that previously failed

Tested-by: Laura Abbott <[email protected]>

> Thanks,
>
> - Arnaldo
>
> commit a04ef2eb0a66d9479e75e536d919c8c9cd618ee3
> Author: Arnaldo Carvalho de Melo <[email protected]>
> Date: Thu Jun 13 12:04:19 2019 -0300
>
> tools build: Check if gettid() is available before providing helper
>
> Laura reported that the perf build failed in fedora when we got a glibc
> that provides gettid(), which I reproduced using fedora rawhide with the
> glibc-devel-2.29.9000-26.fc31.x86_64 package.
>
> Add a feature check to avoid providing a gettid() helper in such
> systems.
>
> On a fedora rawhide system with this patch applied we now get:
>
> [root@7a5f55352234 perf]# grep gettid /tmp/build/perf/FEATURE-DUMP
> feature-gettid=1
> [root@7a5f55352234 perf]# cat /tmp/build/perf/feature/test-gettid.make.output
> [root@7a5f55352234 perf]# ldd /tmp/build/perf/feature/test-gettid.bin
> linux-vdso.so.1 (0x00007ffc6b1f6000)
> libc.so.6 => /lib64/libc.so.6 (0x00007f04e0a74000)
> /lib64/ld-linux-x86-64.so.2 (0x00007f04e0c47000)
> [root@7a5f55352234 perf]# nm /tmp/build/perf/feature/test-gettid.bin | grep -w gettid
> U gettid@@GLIBC_2.30
> [root@7a5f55352234 perf]#
>
> While on a fedora:29 system:
>
> [acme@quaco perf]$ grep gettid /tmp/build/perf/FEATURE-DUMP
> feature-gettid=0
> [acme@quaco perf]$ cat /tmp/build/perf/feature/test-gettid.make.output
> test-gettid.c: In function ‘main’:
> test-gettid.c:8:9: error: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Werror=implicit-function-declaration]
> return gettid();
> ^~~~~~
> getgid
> cc1: all warnings being treated as errors
> [acme@quaco perf]$
>
> Reported-by: Laura Abbott <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Florian Weimer <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Stephane Eranian <[email protected]>
> Link: https://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 3b24231c58a2..50377cc2f5f9 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -36,6 +36,7 @@ FEATURE_TESTS_BASIC := \
> fortify-source \
> sync-compare-and-swap \
> get_current_dir_name \
> + gettid \
> glibc \
> gtk2 \
> gtk2-infobar \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 4b8244ee65ce..523ee42db0c8 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -54,6 +54,7 @@ FILES= \
> test-get_cpuid.bin \
> test-sdt.bin \
> test-cxx.bin \
> + test-gettid.bin \
> test-jvmti.bin \
> test-jvmti-cmlr.bin \
> test-sched_getcpu.bin \
> @@ -267,6 +268,9 @@ $(OUTPUT)test-sdt.bin:
> $(OUTPUT)test-cxx.bin:
> $(BUILDXX) -std=gnu++11
>
> +$(OUTPUT)test-gettid.bin:
> + $(BUILD)
> +
> $(OUTPUT)test-jvmti.bin:
> $(BUILD)
>
> diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> index a59c53705093..3b3d5d72124a 100644
> --- a/tools/build/feature/test-all.c
> +++ b/tools/build/feature/test-all.c
> @@ -38,6 +38,10 @@
> # include "test-get_current_dir_name.c"
> #undef main
>
> +#define main main_test_gettid
> +# include "test-gettid.c"
> +#undef main
> +
> #define main main_test_glibc
> # include "test-glibc.c"
> #undef main
> @@ -195,6 +199,7 @@ int main(int argc, char *argv[])
> main_test_libelf();
> main_test_libelf_mmap();
> main_test_get_current_dir_name();
> + main_test_gettid();
> main_test_glibc();
> main_test_dwarf();
> main_test_dwarf_getlocations();
> diff --git a/tools/build/feature/test-gettid.c b/tools/build/feature/test-gettid.c
> new file mode 100644
> index 000000000000..ef24e42d3f1b
> --- /dev/null
> +++ b/tools/build/feature/test-gettid.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019, Red Hat Inc, Arnaldo Carvalho de Melo <[email protected]>
> +#define _GNU_SOURCE
> +#include <unistd.h>
> +
> +int main(void)
> +{
> + return gettid();
> +}
> +
> +#undef _GNU_SOURCE
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 51dd00f65709..5f16a20cae86 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -332,6 +332,10 @@ ifeq ($(feature-get_current_dir_name), 1)
> CFLAGS += -DHAVE_GET_CURRENT_DIR_NAME
> endif
>
> +ifeq ($(feature-gettid), 1)
> + CFLAGS += -DHAVE_GETTID
> +endif
> +
> ifdef NO_LIBELF
> NO_DWARF := 1
> NO_DEMANGLE := 1
> diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
> index f7eb63cbbc65..88108598d6e9 100644
> --- a/tools/perf/jvmti/jvmti_agent.c
> +++ b/tools/perf/jvmti/jvmti_agent.c
> @@ -45,10 +45,12 @@
> static char jit_path[PATH_MAX];
> static void *marker_addr;
>
> +#ifndef HAVE_GETTID
> static inline pid_t gettid(void)
> {
> return (pid_t)syscall(__NR_gettid);
> }
> +#endif
>
> static int get_e_machine(struct jitheader *hdr)
> {
>


2019-06-13 20:36:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

Em Thu, Jun 13, 2019 at 03:24:41PM -0400, Laura Abbott escreveu:
> On 6/13/19 11:19 AM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, Jun 12, 2019 at 05:56:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>So, we'll have to have a feature test, that defines some HAVE_GETTID
> >>that then ifdefs out our inline copy, working on it.
> >
> >This should take care of it, please check, perhaps providing a
> >Tested-by: to add to this,
> >
>
> built okay on the tree that previously failed
>
> Tested-by: Laura Abbott <[email protected]>

Thanks for checking!

- Arnaldo

> >Thanks,
> >
> >- Arnaldo
> >
> >commit a04ef2eb0a66d9479e75e536d919c8c9cd618ee3
> >Author: Arnaldo Carvalho de Melo <[email protected]>
> >Date: Thu Jun 13 12:04:19 2019 -0300
> >
> > tools build: Check if gettid() is available before providing helper
> > Laura reported that the perf build failed in fedora when we got a glibc
> > that provides gettid(), which I reproduced using fedora rawhide with the
> > glibc-devel-2.29.9000-26.fc31.x86_64 package.
> > Add a feature check to avoid providing a gettid() helper in such
> > systems.
> > On a fedora rawhide system with this patch applied we now get:
> > [root@7a5f55352234 perf]# grep gettid /tmp/build/perf/FEATURE-DUMP
> > feature-gettid=1
> > [root@7a5f55352234 perf]# cat /tmp/build/perf/feature/test-gettid.make.output
> > [root@7a5f55352234 perf]# ldd /tmp/build/perf/feature/test-gettid.bin
> > linux-vdso.so.1 (0x00007ffc6b1f6000)
> > libc.so.6 => /lib64/libc.so.6 (0x00007f04e0a74000)
> > /lib64/ld-linux-x86-64.so.2 (0x00007f04e0c47000)
> > [root@7a5f55352234 perf]# nm /tmp/build/perf/feature/test-gettid.bin | grep -w gettid
> > U gettid@@GLIBC_2.30
> > [root@7a5f55352234 perf]#
> > While on a fedora:29 system:
> > [acme@quaco perf]$ grep gettid /tmp/build/perf/FEATURE-DUMP
> > feature-gettid=0
> > [acme@quaco perf]$ cat /tmp/build/perf/feature/test-gettid.make.output
> > test-gettid.c: In function ‘main’:
> > test-gettid.c:8:9: error: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Werror=implicit-function-declaration]
> > return gettid();
> > ^~~~~~
> > getgid
> > cc1: all warnings being treated as errors
> > [acme@quaco perf]$
> > Reported-by: Laura Abbott <[email protected]>
> > Cc: Adrian Hunter <[email protected]>
> > Cc: Florian Weimer <[email protected]>
> > Cc: Jiri Olsa <[email protected]>
> > Cc: Namhyung Kim <[email protected]>
> > Cc: Stephane Eranian <[email protected]>
> > Link: https://lkml.kernel.org/n/[email protected]
> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> >
> >diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> >index 3b24231c58a2..50377cc2f5f9 100644
> >--- a/tools/build/Makefile.feature
> >+++ b/tools/build/Makefile.feature
> >@@ -36,6 +36,7 @@ FEATURE_TESTS_BASIC := \
> > fortify-source \
> > sync-compare-and-swap \
> > get_current_dir_name \
> >+ gettid \
> > glibc \
> > gtk2 \
> > gtk2-infobar \
> >diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> >index 4b8244ee65ce..523ee42db0c8 100644
> >--- a/tools/build/feature/Makefile
> >+++ b/tools/build/feature/Makefile
> >@@ -54,6 +54,7 @@ FILES= \
> > test-get_cpuid.bin \
> > test-sdt.bin \
> > test-cxx.bin \
> >+ test-gettid.bin \
> > test-jvmti.bin \
> > test-jvmti-cmlr.bin \
> > test-sched_getcpu.bin \
> >@@ -267,6 +268,9 @@ $(OUTPUT)test-sdt.bin:
> > $(OUTPUT)test-cxx.bin:
> > $(BUILDXX) -std=gnu++11
> >+$(OUTPUT)test-gettid.bin:
> >+ $(BUILD)
> >+
> > $(OUTPUT)test-jvmti.bin:
> > $(BUILD)
> >diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> >index a59c53705093..3b3d5d72124a 100644
> >--- a/tools/build/feature/test-all.c
> >+++ b/tools/build/feature/test-all.c
> >@@ -38,6 +38,10 @@
> > # include "test-get_current_dir_name.c"
> > #undef main
> >+#define main main_test_gettid
> >+# include "test-gettid.c"
> >+#undef main
> >+
> > #define main main_test_glibc
> > # include "test-glibc.c"
> > #undef main
> >@@ -195,6 +199,7 @@ int main(int argc, char *argv[])
> > main_test_libelf();
> > main_test_libelf_mmap();
> > main_test_get_current_dir_name();
> >+ main_test_gettid();
> > main_test_glibc();
> > main_test_dwarf();
> > main_test_dwarf_getlocations();
> >diff --git a/tools/build/feature/test-gettid.c b/tools/build/feature/test-gettid.c
> >new file mode 100644
> >index 000000000000..ef24e42d3f1b
> >--- /dev/null
> >+++ b/tools/build/feature/test-gettid.c
> >@@ -0,0 +1,11 @@
> >+// SPDX-License-Identifier: GPL-2.0
> >+// Copyright (C) 2019, Red Hat Inc, Arnaldo Carvalho de Melo <[email protected]>
> >+#define _GNU_SOURCE
> >+#include <unistd.h>
> >+
> >+int main(void)
> >+{
> >+ return gettid();
> >+}
> >+
> >+#undef _GNU_SOURCE
> >diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> >index 51dd00f65709..5f16a20cae86 100644
> >--- a/tools/perf/Makefile.config
> >+++ b/tools/perf/Makefile.config
> >@@ -332,6 +332,10 @@ ifeq ($(feature-get_current_dir_name), 1)
> > CFLAGS += -DHAVE_GET_CURRENT_DIR_NAME
> > endif
> >+ifeq ($(feature-gettid), 1)
> >+ CFLAGS += -DHAVE_GETTID
> >+endif
> >+
> > ifdef NO_LIBELF
> > NO_DWARF := 1
> > NO_DEMANGLE := 1
> >diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
> >index f7eb63cbbc65..88108598d6e9 100644
> >--- a/tools/perf/jvmti/jvmti_agent.c
> >+++ b/tools/perf/jvmti/jvmti_agent.c
> >@@ -45,10 +45,12 @@
> > static char jit_path[PATH_MAX];
> > static void *marker_addr;
> >+#ifndef HAVE_GETTID
> > static inline pid_t gettid(void)
> > {
> > return (pid_t)syscall(__NR_gettid);
> > }
> >+#endif
> > static int get_e_machine(struct jitheader *hdr)
> > {
> >
>

2019-06-13 20:45:48

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

On Thu, Jun 13, 2019 at 12:19:25PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 12, 2019 at 05:56:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> > So, we'll have to have a feature test, that defines some HAVE_GETTID
> > that then ifdefs out our inline copy, working on it.
>
> This should take care of it, please check, perhaps providing a
> Tested-by: to add to this,
>
> Thanks,
>
> - Arnaldo
>
> commit a04ef2eb0a66d9479e75e536d919c8c9cd618ee3
> Author: Arnaldo Carvalho de Melo <[email protected]>
> Date: Thu Jun 13 12:04:19 2019 -0300
>
> tools build: Check if gettid() is available before providing helper
>
> Laura reported that the perf build failed in fedora when we got a glibc
> that provides gettid(), which I reproduced using fedora rawhide with the
> glibc-devel-2.29.9000-26.fc31.x86_64 package.
>
> Add a feature check to avoid providing a gettid() helper in such
> systems.
>
> On a fedora rawhide system with this patch applied we now get:
>
> [root@7a5f55352234 perf]# grep gettid /tmp/build/perf/FEATURE-DUMP
> feature-gettid=1
> [root@7a5f55352234 perf]# cat /tmp/build/perf/feature/test-gettid.make.output
> [root@7a5f55352234 perf]# ldd /tmp/build/perf/feature/test-gettid.bin
> linux-vdso.so.1 (0x00007ffc6b1f6000)
> libc.so.6 => /lib64/libc.so.6 (0x00007f04e0a74000)
> /lib64/ld-linux-x86-64.so.2 (0x00007f04e0c47000)
> [root@7a5f55352234 perf]# nm /tmp/build/perf/feature/test-gettid.bin | grep -w gettid
> U gettid@@GLIBC_2.30
> [root@7a5f55352234 perf]#
>
> While on a fedora:29 system:
>
> [acme@quaco perf]$ grep gettid /tmp/build/perf/FEATURE-DUMP
> feature-gettid=0
> [acme@quaco perf]$ cat /tmp/build/perf/feature/test-gettid.make.output
> test-gettid.c: In function ‘main’:
> test-gettid.c:8:9: error: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Werror=implicit-function-declaration]
> return gettid();
> ^~~~~~
> getgid
> cc1: all warnings being treated as errors
> [acme@quaco perf]$
>
> Reported-by: Laura Abbott <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Florian Weimer <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Stephane Eranian <[email protected]>
> Link: https://lkml.kernel.org/n/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

looks good to me, if needed:

Acked-by: Jiri Olsa <[email protected]>

jirka

>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 3b24231c58a2..50377cc2f5f9 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -36,6 +36,7 @@ FEATURE_TESTS_BASIC := \
> fortify-source \
> sync-compare-and-swap \
> get_current_dir_name \
> + gettid \
> glibc \
> gtk2 \
> gtk2-infobar \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 4b8244ee65ce..523ee42db0c8 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -54,6 +54,7 @@ FILES= \
> test-get_cpuid.bin \
> test-sdt.bin \
> test-cxx.bin \
> + test-gettid.bin \
> test-jvmti.bin \
> test-jvmti-cmlr.bin \
> test-sched_getcpu.bin \
> @@ -267,6 +268,9 @@ $(OUTPUT)test-sdt.bin:
> $(OUTPUT)test-cxx.bin:
> $(BUILDXX) -std=gnu++11
>
> +$(OUTPUT)test-gettid.bin:
> + $(BUILD)
> +
> $(OUTPUT)test-jvmti.bin:
> $(BUILD)
>
> diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> index a59c53705093..3b3d5d72124a 100644
> --- a/tools/build/feature/test-all.c
> +++ b/tools/build/feature/test-all.c
> @@ -38,6 +38,10 @@
> # include "test-get_current_dir_name.c"
> #undef main
>
> +#define main main_test_gettid
> +# include "test-gettid.c"
> +#undef main
> +
> #define main main_test_glibc
> # include "test-glibc.c"
> #undef main
> @@ -195,6 +199,7 @@ int main(int argc, char *argv[])
> main_test_libelf();
> main_test_libelf_mmap();
> main_test_get_current_dir_name();
> + main_test_gettid();
> main_test_glibc();
> main_test_dwarf();
> main_test_dwarf_getlocations();
> diff --git a/tools/build/feature/test-gettid.c b/tools/build/feature/test-gettid.c
> new file mode 100644
> index 000000000000..ef24e42d3f1b
> --- /dev/null
> +++ b/tools/build/feature/test-gettid.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2019, Red Hat Inc, Arnaldo Carvalho de Melo <[email protected]>
> +#define _GNU_SOURCE
> +#include <unistd.h>
> +
> +int main(void)
> +{
> + return gettid();
> +}
> +
> +#undef _GNU_SOURCE
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 51dd00f65709..5f16a20cae86 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -332,6 +332,10 @@ ifeq ($(feature-get_current_dir_name), 1)
> CFLAGS += -DHAVE_GET_CURRENT_DIR_NAME
> endif
>
> +ifeq ($(feature-gettid), 1)
> + CFLAGS += -DHAVE_GETTID
> +endif
> +
> ifdef NO_LIBELF
> NO_DWARF := 1
> NO_DEMANGLE := 1
> diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c
> index f7eb63cbbc65..88108598d6e9 100644
> --- a/tools/perf/jvmti/jvmti_agent.c
> +++ b/tools/perf/jvmti/jvmti_agent.c
> @@ -45,10 +45,12 @@
> static char jit_path[PATH_MAX];
> static void *marker_addr;
>
> +#ifndef HAVE_GETTID
> static inline pid_t gettid(void)
> {
> return (pid_t)syscall(__NR_gettid);
> }
> +#endif
>
> static int get_e_machine(struct jitheader *hdr)
> {

2019-06-13 21:56:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: perf build failure with newer glibc headers

Em Thu, Jun 13, 2019 at 10:44:54PM +0200, Jiri Olsa escreveu:
> On Thu, Jun 13, 2019 at 12:19:25PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Jun 12, 2019 at 05:56:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> > commit a04ef2eb0a66d9479e75e536d919c8c9cd618ee3
> > Author: Arnaldo Carvalho de Melo <[email protected]>
> > Date: Thu Jun 13 12:04:19 2019 -0300
> >
> > tools build: Check if gettid() is available before providing helper

> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> looks good to me, if needed:
>
> Acked-by: Jiri Olsa <[email protected]>

Thanks, added.

- Arnaldo