[PATCH] perf: Ensure symbols for plugins are exported

[PATCH] perf: Ensure symbols for plugins are exported

[Mathias Krause]

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 <acme@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
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

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

[Jiri Olsa]

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

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

[Mathias Krause]

On 17 April 2015 at 17:34, Jiri Olsa <jolsa@redhat.com> 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

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

[Jiri Olsa]

On Fri, Apr 17, 2015 at 11:01:07PM +0200, Mathias Krause wrote:
> On 17 April 2015 at 17:34, Jiri Olsa <jolsa@redhat.com> 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 <jolsa@kernel.org>

thanks,
jirka

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

[Mathias Krause]

On 18 April 2015 at 17:46, Jiri Olsa <jolsa@redhat.com> wrote:
> On Fri, Apr 17, 2015 at 11:01:07PM +0200, Mathias Krause wrote:
>> On 17 April 2015 at 17:34, Jiri Olsa <jolsa@redhat.com> 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 <jolsa@kernel.org>

Thanks,
Mathias