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

[Stefan Hajnoczi]

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

signature.asc
Description: PGP signature