Re: [PATCH] trace: use STAP_SDT_V2 to work around symbol visibility
On Wed, Nov 18, 2020 at 05:51:28PM +, Daniel P. Berrangé wrote: > On Wed, Nov 18, 2020 at 05:48:09PM +, Stefan Hajnoczi wrote: > > QEMU binaries no longer launch successfully with recent SystemTap > > releases. This is because modular QEMU builds link the sdt semaphores > > into the main binary instead of into the shared objects where they are > > used. The symbol visibility of semaphores is 'hidden' and the dynamic > > linker prints an error during module loading: > > > > $ ./configure --enable-trace-backends=dtrace --enable-modules ... > > ... > > Failed to open module: > > /builddir/build/BUILD/qemu-4.2.0/s390x-softmmu/../block-curl.so: undefined > > symbol: qemu_curl_close_semaphore > > > > The long-term solution is to generate per-module dtrace .o files and > > link them into the module instead of the main binary. > > > > In the short term we can define STAP_SDT_V2 so /usr/bin/dtrace produces > > an .o file with 'default' symbol visibility instead of 'hidden'. This > > workaround is small and easier to merge for QEMU 5.2. > > > > Cc: Daniel P. Berrangé > > Cc: wco...@redhat.com > > Cc: f...@redhat.com > > Cc: kra...@redhat.com > > Cc: rjo...@redhat.com > > Cc: mreza...@redhat.com > > Cc: ddepa...@redhat.com > > Signed-off-by: Stefan Hajnoczi > > --- > > trace/meson.build | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/trace/meson.build b/trace/meson.build > > index d5fc45c628..52be7c5b2c 100644 > > --- a/trace/meson.build > > +++ b/trace/meson.build > > @@ -44,7 +44,7 @@ foreach dir : [ '.' ] + trace_events_subdirs > >trace_dtrace_o = custom_target(fmt.format('trace-dtrace', 'o'), > > output: fmt.format('trace-dtrace', > > 'o'), > > input: trace_dtrace, > > - command: [ 'dtrace', '-o', > > '@OUTPUT@', '-G', '-s', '@INPUT@' ]) > > + command: [ 'dtrace', '-DSTAP_SDT_V2', > > '-o', '@OUTPUT@', '-G', '-s', '@INPUT@' ]) > > I'm a little concerned that we're not also setting this macro before > including the generated trace.h headers, because those headers do > check this STAP_SDT_V1 symbol. > > Currently the generated headers have same code for V2 and V3 (the default), > so we won't break, but I'm concerned we could break if they introduce a > future V4 and that impacts the generated headers. > > So I think the safe thing todo is set -DSTAP_SDT_V2 as a global compile > arg for QEMU too, so all trace.h files see the symbol that matches the > trace.o files I have grepped systemtap.git and setting STAP_SDT_V2 globally seems fine. Originally I wasn't sure of its side-effects, but I'm more confident about using it after checking the SystemTap code. Will send a v2. Input from Frank or William would be appreciated though! Stefan signature.asc Description: PGP signature
Re: [PATCH] trace: use STAP_SDT_V2 to work around symbol visibility
On Wed, Nov 18, 2020 at 05:48:09PM +, Stefan Hajnoczi wrote: > QEMU binaries no longer launch successfully with recent SystemTap > releases. This is because modular QEMU builds link the sdt semaphores > into the main binary instead of into the shared objects where they are > used. The symbol visibility of semaphores is 'hidden' and the dynamic > linker prints an error during module loading: > > $ ./configure --enable-trace-backends=dtrace --enable-modules ... > ... > Failed to open module: > /builddir/build/BUILD/qemu-4.2.0/s390x-softmmu/../block-curl.so: undefined > symbol: qemu_curl_close_semaphore > > The long-term solution is to generate per-module dtrace .o files and > link them into the module instead of the main binary. > > In the short term we can define STAP_SDT_V2 so /usr/bin/dtrace produces > an .o file with 'default' symbol visibility instead of 'hidden'. This > workaround is small and easier to merge for QEMU 5.2. > > Cc: Daniel P. Berrangé > Cc: wco...@redhat.com > Cc: f...@redhat.com > Cc: kra...@redhat.com > Cc: rjo...@redhat.com > Cc: mreza...@redhat.com > Cc: ddepa...@redhat.com > Signed-off-by: Stefan Hajnoczi > --- > trace/meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/trace/meson.build b/trace/meson.build > index d5fc45c628..52be7c5b2c 100644 > --- a/trace/meson.build > +++ b/trace/meson.build > @@ -44,7 +44,7 @@ foreach dir : [ '.' ] + trace_events_subdirs >trace_dtrace_o = custom_target(fmt.format('trace-dtrace', 'o'), > output: fmt.format('trace-dtrace', 'o'), > input: trace_dtrace, > - command: [ 'dtrace', '-o', '@OUTPUT@', > '-G', '-s', '@INPUT@' ]) > + command: [ 'dtrace', '-DSTAP_SDT_V2', > '-o', '@OUTPUT@', '-G', '-s', '@INPUT@' ]) I'm a little concerned that we're not also setting this macro before including the generated trace.h headers, because those headers do check this STAP_SDT_V1 symbol. Currently the generated headers have same code for V2 and V3 (the default), so we won't break, but I'm concerned we could break if they introduce a future V4 and that impacts the generated headers. So I think the safe thing todo is set -DSTAP_SDT_V2 as a global compile arg for QEMU too, so all trace.h files see the symbol that matches the trace.o files Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] trace: use STAP_SDT_V2 to work around symbol visibility
QEMU binaries no longer launch successfully with recent SystemTap releases. This is because modular QEMU builds link the sdt semaphores into the main binary instead of into the shared objects where they are used. The symbol visibility of semaphores is 'hidden' and the dynamic linker prints an error during module loading: $ ./configure --enable-trace-backends=dtrace --enable-modules ... ... Failed to open module: /builddir/build/BUILD/qemu-4.2.0/s390x-softmmu/../block-curl.so: undefined symbol: qemu_curl_close_semaphore The long-term solution is to generate per-module dtrace .o files and link them into the module instead of the main binary. In the short term we can define STAP_SDT_V2 so /usr/bin/dtrace produces an .o file with 'default' symbol visibility instead of 'hidden'. This workaround is small and easier to merge for QEMU 5.2. Cc: Daniel P. Berrangé Cc: wco...@redhat.com Cc: f...@redhat.com Cc: kra...@redhat.com Cc: rjo...@redhat.com Cc: mreza...@redhat.com Cc: ddepa...@redhat.com Signed-off-by: Stefan Hajnoczi --- trace/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trace/meson.build b/trace/meson.build index d5fc45c628..52be7c5b2c 100644 --- a/trace/meson.build +++ b/trace/meson.build @@ -44,7 +44,7 @@ foreach dir : [ '.' ] + trace_events_subdirs trace_dtrace_o = custom_target(fmt.format('trace-dtrace', 'o'), output: fmt.format('trace-dtrace', 'o'), input: trace_dtrace, - command: [ 'dtrace', '-o', '@OUTPUT@', '-G', '-s', '@INPUT@' ]) + command: [ 'dtrace', '-DSTAP_SDT_V2', '-o', '@OUTPUT@', '-G', '-s', '@INPUT@' ]) trace_ss.add(trace_dtrace_o) endif -- 2.28.0