qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>, qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	rjones@redhat.com, fche@redhat.com, kraxel@redhat.com,
	wcohen@redhat.com, mrezanin@redhat.com, ddepaula@redhat.com
Subject: Re: [PATCH-for-5.2 v2] trace: use STAP_SDT_V2 to work around symbol visibility
Date: Thu, 19 Nov 2020 12:44:47 +0100	[thread overview]
Message-ID: <fbe8f975-45a7-02e0-4765-b958630f6f2d@redhat.com> (raw)
In-Reply-To: <20201119112704.837423-1-stefanha@redhat.com>

On 11/19/20 12:27 PM, 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 dtrace(1) produces a .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>
> ---
> v2:
>  * Define STAP_SDT_V2 everywhere [danpb]
> ---
>  configure         | 1 +
>  trace/meson.build | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 714e75b5d8..5d91d49c7b 100755
> --- a/configure
> +++ b/configure
> @@ -4832,6 +4832,7 @@ if have_backend "dtrace"; then
>    trace_backend_stap="no"
>    if has 'stap' ; then
>      trace_backend_stap="yes"

Maybe add a comment? (no need to repost if you agree):

       # Workaround to avoid dtrace(1) produces file with 'hidden'
       # symbol visibility, define STAP_SDT_V2 to produce 'default'
       # symbol visibility instead.

> +    QEMU_CFLAGS="$QEMU_CFLAGS -DSTAP_SDT_V2"

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>    fi
>  fi
>  
> diff --git a/trace/meson.build b/trace/meson.build
> index d5fc45c628..843ea14495 100644
> --- a/trace/meson.build
> +++ b/trace/meson.build
> @@ -38,13 +38,13 @@ foreach dir : [ '.' ] + trace_events_subdirs
>      trace_dtrace_h = custom_target(fmt.format('trace-dtrace', 'h'),
>                                     output: fmt.format('trace-dtrace', 'h'),
>                                     input: trace_dtrace,
> -                                   command: [ 'dtrace', '-o', '@OUTPUT@', '-h', '-s', '@INPUT@' ])
> +                                   command: [ 'dtrace', '-DSTAP_SDT_V2', '-o', '@OUTPUT@', '-h', '-s', '@INPUT@' ])
>      trace_ss.add(trace_dtrace_h)
>      if host_machine.system() != 'darwin'
>        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
>  
> 



  parent reply	other threads:[~2020-11-19 11:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-19 11:27 [PATCH v2] trace: use STAP_SDT_V2 to work around symbol visibility Stefan Hajnoczi
2020-11-19 11:31 ` Daniel P. Berrangé
2020-11-19 11:44 ` Philippe Mathieu-Daudé [this message]
2020-11-19 11:58   ` [PATCH-for-5.2 " Stefan Hajnoczi
2020-11-19 14:38     ` Frank Ch. Eigler
2020-11-19 14:47       ` Stefan Hajnoczi
2020-11-19 12:45 ` [PATCH " Miroslav Rezanina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fbe8f975-45a7-02e0-4765-b958630f6f2d@redhat.com \
    --to=philmd@redhat.com \
    --cc=berrange@redhat.com \
    --cc=ddepaula@redhat.com \
    --cc=fche@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mrezanin@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=wcohen@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).