[PATCH 2/3] perf: Allow strong and weak functions in LIB_OBJS

Previous thread: Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework by David Brownell on Wednesday, November 24, 2010 - 8:59 pm. (6 messages)

Next thread: [GIT PULL] Samsung fixes for 2.6.37-rc4 by Kukjin Kim on Wednesday, November 24, 2010 - 9:21 pm. (5 messages)
From: Ian Munsie
Date: Wednesday, November 24, 2010 - 9:12 pm

From: Ian Munsie <imunsie@au1.ibm.com>

If a 32bit userspace perf is running on a 64bit kernel, the end of the
final map in the kernel would incorrectly be set to 2^32-1 rather than
2^64-1.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 tools/perf/util/event.c  |    2 +-
 tools/perf/util/symbol.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index bc47446..4283417 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -392,7 +392,7 @@ static void event_set_kernel_mmap_len(struct map **maps, event_t *self)
 	 * a zero sized synthesized MMAP event for the kernel.
 	 */
 	if (maps[MAP__FUNCTION]->end == 0)
-		maps[MAP__FUNCTION]->end = ~0UL;
+		maps[MAP__FUNCTION]->end = ~0ULL;
 }
 
 static int event__process_kernel_mmap(event_t *self,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0500895..a348906 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -121,7 +121,7 @@ static void __map_groups__fixup_end(struct map_groups *self, enum map_type type)
 	 * We still haven't the actual symbols, so guess the
 	 * last map final address.
 	 */
-	curr->end = ~0UL;
+	curr->end = ~0ULL;
 }
 
 static void map_groups__fixup_end(struct map_groups *self)
-- 
1.7.2.3

--

From: Ian Munsie
Date: Wednesday, November 24, 2010 - 9:12 pm

From: Ian Munsie <imunsie@au1.ibm.com>

When we build perf we place all of the .o files from the library files
(util, arch/x/util, etc) into libperf.a which is then linked into perf.
The problem is that the linker will by default only consider .o files
within the .a archive if they are necessary to satisfy an unresolved
symbol. As weak functions are not unresolved, it will not consider a .o
file from the archive containing the strong versions of weak functions
unless it requires it for another reason.

This patch adds the --whole-archive flags to the linker when passing in
the libperf.a file to ensure that it will consider every .o file in the
archive, not just what it believes that it needs. The end result is that
weak functions can now be overridden by strong variants of them in the
libperf.a file.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 tools/perf/Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 2d414b3..c5f7b7f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -940,7 +940,8 @@ $(OUTPUT)perf.o: perf.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFLAGS
 
 $(OUTPUT)perf$X: $(OUTPUT)perf.o $(BUILTIN_OBJS) $(PERFLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) $(ALL_LDFLAGS) $(OUTPUT)perf.o \
-               $(BUILTIN_OBJS) $(LIBS) -o $@
+               $(BUILTIN_OBJS) -Wl,--whole-archive $(LIBS) \
+	       -Wl,--no-whole-archive -o $@
 	@rm -f trace
 	@ln perf$X trace 2>/dev/null
 
-- 
1.7.2.3

--

From: Arnaldo Carvalho de Melo
Date: Friday, November 26, 2010 - 2:18 pm

After I applied this patch I got this on a x86_64 Fedora 14 box:

    LINK /home/acme/git/build/perf/perf
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `__pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `__pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib/gcc/x86_64-redhat-linux/4.5.1/../../../../lib64/libiberty.a(choose-temp.o): In function `choose_temp_base':
(.text+0x57): warning: the use of `mktemp' is dangerous, better use `mkstemp'
/usr/lib/gcc/x86_64-redhat-linux/4.5.1/../../../../lib64/libiberty.a(strerror.o): In function `errno_max':
(.text+0x151): warning: `sys_nerr' is deprecated; use `strerror' or `strerror_r' instead
/usr/lib/gcc/x86_64-redhat-linux/4.5.1/../../../../lib64/libiberty.a(xstrdup.o): In function `xstrdup':
(.text+0x0): multiple definition of `xstrdup'
/home/acme/git/build/perf/libperf.a(wrapper.o):/media/tbs/acme/git/linux/tools/perf/util/wrapper.c:15: first defined here
collect2: ld returned 1 exit status
make: *** [/home/acme/git/build/perf/perf] Error 1
make: Leaving directory `/media/tbs/acme/git/linux/tools/perf'
[acme@felicio linux]$

- Arnaldo
--

From: Ian Munsie
Date: Sunday, November 28, 2010 - 5:53 pm

Hi Arnaldo,


I'm not seeing any of those warnings or errors on either of the systems
I have been testing on (x86_64 + PPC64), but it looks like the linker is
trying to pull in too much from the system libraries.

Can you test the below patch to see if the messages go away - it only
applies the whole-archive flag to libperf.a, whereas before I was also
applying it to pthread, libelf, etc.

Cheers,
-Ian


From da1ff2cafce30343fba34b81935d0c7772f8c6ae Mon Sep 17 00:00:00 2001
From: Ian Munsie <imunsie@au1.ibm.com>
Date: Thu, 25 Nov 2010 13:44:52 +1100
Subject: [PATCH] perf: Allow strong and weak functions in LIB_OBJS

When we build perf we place all of the .o files from the library files
(util, arch/x/util, etc) into libperf.a which is then linked into perf.
The problem is that the linker will by default only consider .o files
within the .a archive if they are necessary to satisfy an unresolved
symbol. As weak functions are not unresolved, it will not consider a .o
file from the archive containing the strong versions of weak functions
unless it requires it for another reason.

This patch adds the --whole-archive flags to the linker when passing in
the libperf.a file to ensure that it will consider every .o file in the
archive, not just what it believes that it needs. The end result is that
weak functions can now be overridden by strong variants of them in the
libperf.a file.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 tools/perf/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 2d414b3..f760a2d 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -902,7 +902,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 
-LIBS = $(PERFLIBS) $(EXTLIBS)
+LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive $(EXTLIBS)
 
 BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
 ...
From: Ian Munsie
Date: Monday, December 6, 2010 - 11:42 pm

Hey Arnaldo,

Just wondering if you got around to testing this patch?

Thanks,
-Ian
--

From: Arnaldo Carvalho de Melo
Date: Tuesday, December 7, 2010 - 7:55 am

Yeah, fell thru the cracks, but I tested it now on several systems and
it now seems ok, applying to perf/core, thanks,

- Arnaldo
--

From: tip-bot for Ian Munsie
Date: Wednesday, December 8, 2010 - 12:39 am

Commit-ID:  b38aa89600be39b3e10c5b6529aed2e66518598e
Gitweb:     http://git.kernel.org/tip/b38aa89600be39b3e10c5b6529aed2e66518598e
Author:     Ian Munsie <imunsie@au1.ibm.com>
AuthorDate: Mon, 29 Nov 2010 11:53:07 +1100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 7 Dec 2010 11:58:50 -0200

perf makefile: Allow strong and weak functions in LIB_OBJS

When we build perf we place all of the .o files from the library files
(util, arch/x/util, etc) into libperf.a which is then linked into perf.

The problem is that the linker will by default only consider .o files
within the .a archive if they are necessary to satisfy an unresolved
symbol. As weak functions are not unresolved, it will not consider a .o
file from the archive containing the strong versions of weak functions
unless it requires it for another reason.

This patch adds the --whole-archive flags to the linker when passing in
the libperf.a file to ensure that it will consider every .o file in the
archive, not just what it believes that it needs. The end result is that
weak functions can now be overridden by strong variants of them in the
libperf.a file.

Cc: "tom.leiming" <tom.leiming@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1290991642-sup-5890@au1.ibm.com>
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index d88137a..ac6692c 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -901,7 +901,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 
-LIBS = $(PERFLIBS) $(EXTLIBS)
+LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive $(EXTLIBS)
 
 BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' ...
From: Ian Munsie
Date: Wednesday, November 24, 2010 - 9:12 pm

From: Ian Munsie <imunsie@au1.ibm.com>

The PowerPC ABI prefixes symbol names with periods, which causes perf
test to fail when it compares the symbol from the vmlinux file with the
symbol names from kallsyms. This patch adds the infrastructure to allow
archs to override how symbol names are compared and implements the
PowerPC function to disregard any prefixed periods, allowing perf test
to pass.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 tools/perf/arch/powerpc/Makefile      |    5 +++++
 tools/perf/arch/powerpc/util/symbol.c |   22 ++++++++++++++++++++++
 tools/perf/builtin-test.c             |    2 +-
 tools/perf/util/symbol.c              |    5 +++++
 tools/perf/util/symbol.h              |    2 ++
 5 files changed, 35 insertions(+), 1 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/symbol.c

diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 15130b5..b3e211e 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -2,3 +2,8 @@ ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
 endif
+
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/symbol.o
+
+$(OUTPUT)arch/powerpc/util/%.o : arch/powerpc/util/%.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -I util/ $<
diff --git a/tools/perf/arch/powerpc/util/symbol.c b/tools/perf/arch/powerpc/util/symbol.c
new file mode 100644
index 0000000..db56913
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/symbol.c
@@ -0,0 +1,22 @@
+/*
+ * Functions to handle PowerPC symbol naming
+ *
+ * Copyright © 2010 Ian Munsie, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <string.h>
+#include <symbol.h>
+
+/* Overrides the definition in the ...
From: Ian Munsie
Date: Thursday, December 9, 2010 - 9:47 pm

Hi Paul,

Can you take a look at this patch and ACK (or NACK) it and push it to
Ingo?  The two other patches in this series are already in
tip/perf/core, we're just missing this one.

Cheers,
-Ian

--

From: Arnaldo Carvalho de Melo
Date: Sunday, December 12, 2010 - 7:39 am

Hi Ian,

	I guess we can apply this one now? Or do you have anything
newer?

- Arnaldo
--

From: Ian Munsie
Date: Sunday, December 12, 2010 - 5:30 pm

Yep, that's good to go in as is.

Thanks,
-Ian
--

From: tip-bot for Ian Munsie
Date: Sunday, November 28, 2010 - 1:34 am

Commit-ID:  9d1faba5fe410558099f13cfada2eab03186769d
Gitweb:     http://git.kernel.org/tip/9d1faba5fe410558099f13cfada2eab03186769d
Author:     Ian Munsie <imunsie@au1.ibm.com>
AuthorDate: Thu, 25 Nov 2010 15:12:53 +1100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 27 Nov 2010 01:32:53 -0200

perf symbols: Correct final kernel map guesses

If a 32bit userspace perf is running on a 64bit kernel, the end of the final
map in the kernel would incorrectly be set to 2^32-1 rather than 2^64-1.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1290658375-10342-1-git-send-email-imunsie@au1.ibm.com>
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c  |    2 +-
 tools/perf/util/symbol.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index dab9e75..7260db7 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -392,7 +392,7 @@ static void event_set_kernel_mmap_len(struct map **maps, event_t *self)
 	 * a zero sized synthesized MMAP event for the kernel.
 	 */
 	if (maps[MAP__FUNCTION]->end == 0)
-		maps[MAP__FUNCTION]->end = ~0UL;
+		maps[MAP__FUNCTION]->end = ~0ULL;
 }
 
 static int event__process_kernel_mmap(event_t *self,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0500895..a348906 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -121,7 +121,7 @@ static void __map_groups__fixup_end(struct map_groups *self, enum map_type type)
 	 * We still haven't the actual symbols, so guess the
 	 * last map final address.
 	 */
-	curr->end = ~0UL;
+	curr->end = ~0ULL;
 }
 
 static void map_groups__fixup_end(struct map_groups *self)
--

Previous thread: Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework by David Brownell on Wednesday, November 24, 2010 - 8:59 pm. (6 messages)

Next thread: [GIT PULL] Samsung fixes for 2.6.37-rc4 by Kukjin Kim on Wednesday, November 24, 2010 - 9:21 pm. (5 messages)