2015-04-12 16:01:07

by Mathias Krause

[permalink] [raw]
Subject: [PATCH] perf: Ensure symbols for plugins are exported

When building perf with perl or python support it implicitly gets linked
with the -export-dynamic linker option through the additional linker
flags, namely with -Wl,-E via perl or -Xlinker -export-dynamic via
python. That flag is essential for the traceevent plugin support so we
shouldn't rely on adding it implicitly.

Ensure perf's exported symbols can be used by dlopen()ed plugins by
unconditionally adding this flag when linking perf. Otherwise plugins
won't be able to access symbols in the perf binary.

This fixes the following warning / bug when trying to load plugins:

Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_xen.so'
/home/minipli/.traceevent/plugins/plugin_xen.so: undefined symbol: trace_seq_printf
Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_function.so'
/home/minipli/.traceevent/plugins/plugin_function.so: undefined symbol: warning
Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_sched_switch.so'
/home/minipli/.traceevent/plugins/plugin_sched_switch.so: undefined symbol: pevent_unregister_event_handler
[...]

Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Mathias Krause <[email protected]>
---
This patch should go on top of tip.git#perf/core

tools/perf/Makefile.perf | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index c43a20517591..2ab8525f366b 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -250,7 +250,8 @@ ifdef ASCIIDOC8
export ASCIIDOC8
endif

-LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive -Wl,--start-group $(EXTLIBS) -Wl,--end-group
+LIBS = -Wl,--export-dynamic -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive
+LIBS += -Wl,--start-group $(EXTLIBS) -Wl,--end-group

export INSTALL SHELL_PATH

--
1.7.10.4


2015-04-17 15:34:53

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: Ensure symbols for plugins are exported

On Sun, Apr 12, 2015 at 06:00:51PM +0200, Mathias Krause wrote:
> When building perf with perl or python support it implicitly gets linked
> with the -export-dynamic linker option through the additional linker
> flags, namely with -Wl,-E via perl or -Xlinker -export-dynamic via
> python. That flag is essential for the traceevent plugin support so we
> shouldn't rely on adding it implicitly.
>
> Ensure perf's exported symbols can be used by dlopen()ed plugins by
> unconditionally adding this flag when linking perf. Otherwise plugins
> won't be able to access symbols in the perf binary.
>
> This fixes the following warning / bug when trying to load plugins:
>
> Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_xen.so'
> /home/minipli/.traceevent/plugins/plugin_xen.so: undefined symbol: trace_seq_printf
> Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_function.so'
> /home/minipli/.traceevent/plugins/plugin_function.so: undefined symbol: warning
> Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_sched_switch.so'
> /home/minipli/.traceevent/plugins/plugin_sched_switch.so: undefined symbol: pevent_unregister_event_handler
> [...]

hum, not sure now how -export-dynamic works but should this
be rather in traceevent lib then?

jirka

2015-04-17 21:01:22

by Mathias Krause

[permalink] [raw]
Subject: Re: [PATCH] perf: Ensure symbols for plugins are exported

On 17 April 2015 at 17:34, Jiri Olsa <[email protected]> wrote:
> On Sun, Apr 12, 2015 at 06:00:51PM +0200, Mathias Krause wrote:
>> When building perf with perl or python support it implicitly gets linked
>> with the -export-dynamic linker option through the additional linker
>> flags, namely with -Wl,-E via perl or -Xlinker -export-dynamic via
>> python. That flag is essential for the traceevent plugin support so we
>> shouldn't rely on adding it implicitly.
>>
>> Ensure perf's exported symbols can be used by dlopen()ed plugins by
>> unconditionally adding this flag when linking perf. Otherwise plugins
>> won't be able to access symbols in the perf binary.
>>
>> This fixes the following warning / bug when trying to load plugins:
>>
>> Warning: could not load plugin '/home/minipli/.traceevent/plugins/plugin_xen.so'
>> /home/minipli/.traceevent/plugins/plugin_xen.so: undefined symbol: trace_seq_printf
>> [...]
>
> hum, not sure now how -export-dynamic works but should this
> be rather in traceevent lib then?

Nope. Here's the relevant excerpt from ld(1):

--export-dynamic
[...]
If you use "dlopen" to load a dynamic object which needs to
refer back to the symbols defined by the program, rather
than some other dynamic object, then you will probably need
to use this option when linking the program itself.

So that flag has to be in the linker call for perf, as the plugins
(which are dlopen()ed) want to access symbols within the perf binary
(or more specific, within libperf.a / libtraceevent.a statically
linked into perf).


Regards,
Mathias

2015-04-18 15:46:38

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: Ensure symbols for plugins are exported

On Fri, Apr 17, 2015 at 11:01:07PM +0200, Mathias Krause wrote:
> On 17 April 2015 at 17:34, Jiri Olsa <[email protected]> wrote:
> > On Sun, Apr 12, 2015 at 06:00:51PM +0200, Mathias Krause wrote:
> >> When building perf with perl or python support it implicitly gets linked
> >> with the -export-dynamic linker option through the additional linker
> >> flags, namely with -Wl,-E via perl or -Xlinker -export-dynamic via
> >> python. That flag is essential for the traceevent plugin support so we
> >> shouldn't rely on adding it implicitly.

I dont see the -E flag being added for perl on my setup,
but I guess that could be different on each distro

maybe we should look for proper fix and export only
needed symbols, anyway for this bugfix:

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

thanks,
jirka

2015-04-22 20:12:32

by Mathias Krause

[permalink] [raw]
Subject: Re: [PATCH] perf: Ensure symbols for plugins are exported

On 18 April 2015 at 17:46, Jiri Olsa <[email protected]> wrote:
> On Fri, Apr 17, 2015 at 11:01:07PM +0200, Mathias Krause wrote:
>> On 17 April 2015 at 17:34, Jiri Olsa <[email protected]> wrote:
>> > On Sun, Apr 12, 2015 at 06:00:51PM +0200, Mathias Krause wrote:
>> >> When building perf with perl or python support it implicitly gets linked
>> >> with the -export-dynamic linker option through the additional linker
>> >> flags, namely with -Wl,-E via perl or -Xlinker -export-dynamic via
>> >> python. That flag is essential for the traceevent plugin support so we
>> >> shouldn't rely on adding it implicitly.
>
> I dont see the -E flag being added for perl on my setup,
> but I guess that could be different on each distro

Yeah, seems so. On Debian I get the following:

$ perl -MExtUtils::Embed -e ldopts
-Wl,-E -fstack-protector -L/usr/local/lib
-L/usr/lib/x86_64-linux-gnu/perl/5.20/CORE -lperl -ldl -lm -lpthread
-lc -lcrypt

$ python-config --ldflags
-L/usr/lib/python2.7/config-x86_64-linux-gnu -L/usr/lib -lpython2.7
-lpthread -ldl -lutil -lm -Xlinker -export-dynamic -Wl,-O1
-Wl,-Bsymbolic-functions

So, the -export-dynamic linker flag gets added by both -- perl and python.

However on CentOS 7 I don't see the -Wl,-E for perl either. But python
still adds it. So the patch is even more needed as leaving out perf
python support on such a distro would make it miss the flag, too.

> maybe we should look for proper fix and export only
> needed symbols

Well, that would require maintaining a symbol file. Don't know if it's
worth it :/

> , anyway for this bugfix:
>
> Acked-by: Jiri Olsa <[email protected]>

Thanks,
Mathias