Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC
18.03.2024 10:35, Marc-André Lureau wrote:
..
dbus-display1 is also used with static linking for the unit test.
It looks like the simplest is to let the actual target decide how it
is built, even if it is compiled multiple time then:
- dbus_display1_lib = static_library('dbus-display1', dbus_display1,
dependencies: gio)
- dbus_display1_dep = declare_dependency(link_with:
dbus_display1_lib, sources: dbus_display1[0])
+ dbus_display1_dep = declare_dependency(sources: dbus_display1,
dependencies: gio)
This makes commit 186acfbaf7 ("tests/qtest: Depend on
dbus_display1_dep") no longer relevant, as dbus-display1.c will be
recompiled.
I definitely prefer this approach.
Alternatively, we could always build with pic: true (or pic:
enable_modules), but that's not ideal either.
Here, we as well might enable pic unconditionally for just
everything. But this is not for this change, and it needs
to be discussed first. So far the consensus was to only
enable pic for shared objects. Also, with --enable-static
it has other implications.
/mjt
Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC
Mon, 18 Mar 2024 11:35:54 +0400 Marc-André Lureau : > Alternatively, we could always build with pic: true (or pic: > enable_modules), but that's not ideal either. I'm sure that unconditionally building with -fPIC has no downsides in this context. Olaf pgpGUQPNKGONs.pgp Description: Digitale Signatur von OpenPGP
Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC
Hi
On Sun, Mar 17, 2024 at 11:10 AM Michael Tokarev wrote:
>
> 17.03.2024 01:19, Olaf Hering:
> > Sat, 16 Mar 2024 22:40:14 +0300 Michael Tokarev :
> >
> >> meson: ensure dbus-display generated code is built before other units
> >> (cherry picked from commit 1222070e772833c6875e0ca63565db12c22df39e)
> >
> > "static_library" is used often. Some use the 'pic' option, which fixes the
> > issue.
> >
> > I think every usage that ends up in a shared library requires 'pic:true'.
>
> The prob here seems to be that while fixing other issue (header file isn't
> generated
> early enough), we all forgot about the fact dbus-display1.o can be used
> inside a
> shared/loadable object when qemu is built with --enable-modules.
>
dbus-display1 is also used with static linking for the unit test.
It looks like the simplest is to let the actual target decide how it
is built, even if it is compiled multiple time then:
- dbus_display1_lib = static_library('dbus-display1', dbus_display1,
dependencies: gio)
- dbus_display1_dep = declare_dependency(link_with:
dbus_display1_lib, sources: dbus_display1[0])
+ dbus_display1_dep = declare_dependency(sources: dbus_display1,
dependencies: gio)
This makes commit 186acfbaf7 ("tests/qtest: Depend on
dbus_display1_dep") no longer relevant, as dbus-display1.c will be
recompiled.
Alternatively, we could always build with pic: true (or pic:
enable_modules), but that's not ideal either.
--
Marc-André Lureau
Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC
17.03.2024 01:19, Olaf Hering: Sat, 16 Mar 2024 22:40:14 +0300 Michael Tokarev : meson: ensure dbus-display generated code is built before other units (cherry picked from commit 1222070e772833c6875e0ca63565db12c22df39e) "static_library" is used often. Some use the 'pic' option, which fixes the issue. I think every usage that ends up in a shared library requires 'pic:true'. The prob here seems to be that while fixing other issue (header file isn't generated early enough), we all forgot about the fact dbus-display1.o can be used inside a shared/loadable object when qemu is built with --enable-modules. /mjt
Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC
Sat, 16 Mar 2024 22:40:14 +0300 Michael Tokarev : > meson: ensure dbus-display generated code is built before other units > (cherry picked from commit 1222070e772833c6875e0ca63565db12c22df39e) "static_library" is used often. Some use the 'pic' option, which fixes the issue. I think every usage that ends up in a shared library requires 'pic:true'. Olaf pgpFRuboYgLKH.pgp Description: Digitale Signatur von OpenPGP
Re: Regression in v7.2.10 - ui-dbus.so requires -fPIC
15.03.2024 00:00, Olaf Hering wrote: ui-dbus.so is a shared library. But it is apparently handled differently than all the other shared libraries: it is not compiled with -fPIC. As a result it fails to link. Not sure why this happens only here. Everything up to v7.2.9 was fine. Looking at some random other library like libui-spice-core.a, every object is compiled with -fPIC. But ui/dbus-display1.c is compiled with -fPIE instead. Is this intentional? Olaf ld: ui/libdbus-display1.a.p/meson-generated_.._dbus-display1.c.o: warning: relocation against `qemu_dbus_display1_audio_get_type' in read-only section `.text' Hi! This seems to be the following patch which I picked up for 7.2.10: commit c172136ea3320fa285c39bfb07298bbe1a14ba5e Author: Marc-André Lureau Date: Thu Aug 11 15:59:40 2022 +0400 meson: ensure dbus-display generated code is built before other units It's simply by luck that dbus-display header is built first before the other units using it. With sourceset, I can't find an easier way out than declaring an extra dependency for dbus-display1 generate code. Signed-off-by: Marc-André Lureau Reviewed-by: Daniel P. Berrangé (cherry picked from commit 1222070e772833c6875e0ca63565db12c22df39e) Signed-off-by: Michael Tokarev This has bitten me multiple times, when dbus-display1.c isn't generated promptly. So I thought it's a good pick. Apparently not :) And indeed, on master the same issue exists now, - after the above commit, dbus-display1.o is being built with -fPIE instead of -fPIC. Reverting this commit makes it use -fPIC again. Apparently we don't build with --enable-modules and with a recent enough compiler to catch this (my gcc-12.2 on debian bookworm does not care about this stuff). Cc'ing Marc-André. An interesting side-effect :) Thanks, /mjt
Regression in v7.2.10 - ui-dbus.so requires -fPIC
ui-dbus.so is a shared library. But it is apparently handled differently than all the other shared libraries: it is not compiled with -fPIC. As a result it fails to link. Not sure why this happens only here. Everything up to v7.2.9 was fine. Looking at some random other library like libui-spice-core.a, every object is compiled with -fPIC. But ui/dbus-display1.c is compiled with -fPIE instead. Is this intentional? Olaf ld: ui/libdbus-display1.a.p/meson-generated_.._dbus-display1.c.o: warning: relocation against `qemu_dbus_display1_audio_get_type' in read-only section `.text' pgpwtSSADV5iy.pgp Description: Digitale Signatur von OpenPGP
