[v1] trace: use STAP_SDT_V2 to work around symbol visibility

[PATCH] trace: use STAP_SDT_V2 to work around symbol visibility

Posted by Stefan Hajnoczi 3 years, 5 months ago

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é <berrange@redhat.com>
Cc: wcohen@redhat.com
Cc: fche@redhat.com
Cc: kraxel@redhat.com
Cc: rjones@redhat.com
Cc: mrezanin@redhat.com
Cc: ddepaula@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 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

Re: [PATCH] trace: use STAP_SDT_V2 to work around symbol visibility

Posted by Daniel P. Berrangé 3 years, 5 months ago

On Wed, Nov 18, 2020 at 05:48:09PM +0000, 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é <berrange@redhat.com>
> Cc: wcohen@redhat.com
> Cc: fche@redhat.com
> Cc: kraxel@redhat.com
> Cc: rjones@redhat.com
> Cc: mrezanin@redhat.com
> Cc: ddepaula@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  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 :|

Re: [PATCH] trace: use STAP_SDT_V2 to work around symbol visibility

Posted by Stefan Hajnoczi 3 years, 5 months ago

On Wed, Nov 18, 2020 at 05:51:28PM +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 18, 2020 at 05:48:09PM +0000, 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é <berrange@redhat.com>
> > Cc: wcohen@redhat.com
> > Cc: fche@redhat.com
> > Cc: kraxel@redhat.com
> > Cc: rjones@redhat.com
> > Cc: mrezanin@redhat.com
> > Cc: ddepaula@redhat.com
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  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