1. Patchew
  2. QEMU
  3. View series

[PATCH for 6.1] plugins: do not limit exported symbols if modules are active

Paolo Bonzini posted 1 patch 2 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210811100550.54714-1-pbonzini@redhat.com
configure           |  5 ++---
plugins/meson.build | 14 ++++++++------
2 files changed, 10 insertions(+), 9 deletions(-)
[PATCH for 6.1] plugins: do not limit exported symbols if modules are active
Posted by Paolo Bonzini 2 years, 9 months ago 
On Mac --enable-modules and --enable-plugins are currently incompatible, because the
Apple -Wl,-exported_symbols_list command line options prevents the export of any
symbols needed by the modules.  On x86 -Wl,--dynamic-list does not have this effect,
but only because the -Wl,--export-dynamic option provided by gmodule-2.0.pc overrides
it.  On Apple there is no -Wl,--export-dynamic, because it is the default, and thus
no override.

Either way, when modules are active there is no reason to include the plugin_ldflags.
While at it, avoid the useless -Wl,--export-dynamic when --enable-plugins is
specified but --enable-modules is not; this way, the GNU and Apple configurations
are more similar.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/516
Cc: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure           |  5 ++---
 plugins/meson.build | 14 ++++++++------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 9a79a004d7..a8721601ea 100755
--- a/configure
+++ b/configure
@@ -3187,9 +3187,8 @@ glib_req_ver=2.56
 glib_modules=gthread-2.0
 if test "$modules" = yes; then
     glib_modules="$glib_modules gmodule-export-2.0"
-fi
-if test "$plugins" = "yes"; then
-    glib_modules="$glib_modules gmodule-2.0"
+elif test "$plugins" = "yes"; then
+    glib_modules="$glib_modules gmodule-noexport-2.0"
 fi
 
 for i in $glib_modules; do
diff --git a/plugins/meson.build b/plugins/meson.build
index e77723010e..bfd5c9822a 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -1,9 +1,11 @@
-if 'CONFIG_HAS_LD_DYNAMIC_LIST' in config_host
-  plugin_ldflags = ['-Wl,--dynamic-list=' + (meson.build_root() / 'qemu-plugins-ld.symbols')]
-elif 'CONFIG_HAS_LD_EXPORTED_SYMBOLS_LIST' in config_host
-  plugin_ldflags = ['-Wl,-exported_symbols_list,' + (meson.build_root() / 'qemu-plugins-ld64.symbols')]
-else
-  plugin_ldflags = []
+plugin_ldflags = []
+# Modules need more symbols than just those in plugins/qemu-plugins.symbols
+if not enable_modules
+  if 'CONFIG_HAS_LD_DYNAMIC_LIST' in config_host
+    plugin_ldflags = ['-Wl,--dynamic-list=' + (meson.build_root() / 'qemu-plugins-ld.symbols')]
+  elif 'CONFIG_HAS_LD_EXPORTED_SYMBOLS_LIST' in config_host
+    plugin_ldflags = ['-Wl,-exported_symbols_list,' + (meson.build_root() / 'qemu-plugins-ld64.symbols')]
+  endif
 endif
 
 specific_ss.add(when: 'CONFIG_PLUGIN', if_true: [files(
-- 
2.31.1
Re: [PATCH for 6.1] plugins: do not limit exported symbols if modules are active
Posted by Alex Bennée 2 years, 8 months ago 
Paolo Bonzini <pbonzini@redhat.com> writes:

> On Mac --enable-modules and --enable-plugins are currently incompatible, because the
> Apple -Wl,-exported_symbols_list command line options prevents the export of any
> symbols needed by the modules.  On x86 -Wl,--dynamic-list does not have this effect,
> but only because the -Wl,--export-dynamic option provided by gmodule-2.0.pc overrides
> it.  On Apple there is no -Wl,--export-dynamic, because it is the default, and thus
> no override.
>
> Either way, when modules are active there is no reason to include the plugin_ldflags.
> While at it, avoid the useless -Wl,--export-dynamic when --enable-plugins is
> specified but --enable-modules is not; this way, the GNU and Apple configurations
> are more similar.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/516
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure           |  5 ++---
>  plugins/meson.build | 14 ++++++++------
>  2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index 9a79a004d7..a8721601ea 100755
> --- a/configure
> +++ b/configure
> @@ -3187,9 +3187,8 @@ glib_req_ver=2.56
>  glib_modules=gthread-2.0
>  if test "$modules" = yes; then
>      glib_modules="$glib_modules gmodule-export-2.0"
> -fi
> -if test "$plugins" = "yes"; then
> -    glib_modules="$glib_modules gmodule-2.0"
> +elif test "$plugins" = "yes"; then
> +    glib_modules="$glib_modules gmodule-noexport-2.0"

This brings in a new dependency because I can't configure now:

  ➜  ../../configure

  ERROR: glib-2.56 gmodule-noexport-2.0 is required to compile QEMU

Should it be gmodule-no-export? Hopefully the different distros aren't
packaging different .pc files.

>  fi
>  
>  for i in $glib_modules; do
> diff --git a/plugins/meson.build b/plugins/meson.build
> index e77723010e..bfd5c9822a 100644
> --- a/plugins/meson.build
> +++ b/plugins/meson.build
> @@ -1,9 +1,11 @@
> -if 'CONFIG_HAS_LD_DYNAMIC_LIST' in config_host
> -  plugin_ldflags = ['-Wl,--dynamic-list=' + (meson.build_root() / 'qemu-plugins-ld.symbols')]
> -elif 'CONFIG_HAS_LD_EXPORTED_SYMBOLS_LIST' in config_host
> -  plugin_ldflags = ['-Wl,-exported_symbols_list,' + (meson.build_root() / 'qemu-plugins-ld64.symbols')]
> -else
> -  plugin_ldflags = []
> +plugin_ldflags = []
> +# Modules need more symbols than just those in plugins/qemu-plugins.symbols
> +if not enable_modules
> +  if 'CONFIG_HAS_LD_DYNAMIC_LIST' in config_host
> +    plugin_ldflags = ['-Wl,--dynamic-list=' + (meson.build_root() / 'qemu-plugins-ld.symbols')]
> +  elif 'CONFIG_HAS_LD_EXPORTED_SYMBOLS_LIST' in config_host
> +    plugin_ldflags = ['-Wl,-exported_symbols_list,' + (meson.build_root() / 'qemu-plugins-ld64.symbols')]
> +  endif
>  endif

Does this mean --enable-modules would allow plugins to access more of
the API space than we intended in the first place?

-- 
Alex Bennée
Re: [PATCH for 6.1] plugins: do not limit exported symbols if modules are active
Posted by Paolo Bonzini 2 years, 8 months ago 
Il gio 12 ago 2021, 11:40 Alex Bennée <alex.bennee@linaro.org> ha scritto:

>   ERROR: glib-2.56 gmodule-noexport-2.0 is required to compile QEMU
>
> Should it be gmodule-no-export? Hopefully the different distros aren't
> packaging different .pc files.
>

My bad. :( It's correct with the dash.

Does this mean --enable-modules would allow plugins to access more of
> the API space than we intended in the first place?
>

Yes, but before it would do so even without --enable-modules due to using
gmodule and not gmodule-no-export.

Paolo


> --
> Alex Bennée
>
>
Re: [PATCH for 6.1] plugins: do not limit exported symbols if modules are active
Posted by Alex Bennée 2 years, 8 months ago 
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il gio 12 ago 2021, 11:40 Alex Bennée <alex.bennee@linaro.org> ha scritto:
>
>    ERROR: glib-2.56 gmodule-noexport-2.0 is required to compile QEMU
>
>  Should it be gmodule-no-export? Hopefully the different distros aren't
>  packaging different .pc files.
>
> My bad. :( It's correct with the dash.

I've patched it locally and it's sitting in my PR branch:

  https://gitlab.com/stsquad/qemu/-/commits/pr/120821-for-6.1-rc4-1

but it looks like it will have to go via stable unless there is a more
compelling reason to cut an rc4.

>  Does this mean --enable-modules would allow plugins to access more of
>  the API space than we intended in the first place?
>
> Yes, but before it would do so even without --enable-modules due to
> using gmodule and not gmodule-no-export.

OK so it's better now. I wonder if this helps with the Windows usage of
modules?

  Subject: [PATCH v6 0/5] Enable plugin support on msys2/mingw
  Date: Tue, 13 Oct 2020 08:28:01 +0800
  Message-Id: <20201013002806.1447-1-luoyonggang@gmail.com>

I really want to avoid the hand hacking of Youngang's proposed changes
so was surprised that glib hadn't papered over the POSIX/Windows cracks.
However like MacOS I don't have easy access to a Windows developer
system to experiment with.

-- 
Alex Bennée

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.