On Mon, 2020-08-03 at 17:26 +0100, Daniel P. Berrangé wrote: > On Mon, Aug 03, 2020 at 05:34:47PM +0200, Hans de Goede wrote: > > Hi, > > > > On 8/3/20 5:27 PM, Daniel P. Berrangé wrote: > > > On Mon, Aug 03, 2020 at 05:01:18PM +0200, Florian Weimer wrote: > > > > * Daniel P. Berrangé: > > > > > > > > > Disabling LTO in the RPM spec confirms this and makes things pass > > > > > again. Hacking the makefiles to remove the -fno-lto option when > > > > > building the test suite binaries also fixes things. > > > > > > > > > > I don't see any mention of LD_PRELOAD being impacted by LTO in the > > > > > Fedora feature change page, but I can imagine how it would be. > > > > > > > > LTO should still export the same functions as before, and should not > > > > imply -fno-semantic-interposition by default. The linker plugin > > > > provides the necessary information to GCC. What you are observing could > > > > be the result of a toolchain bug. > > > > > > Libvirt has a test program "qemuhotplugtest". > > > > > > This test links to a shared library libqemutestdriver.so which contains > > > a function "qemuProcessStartManagedPRDaemon". > > > > > > qemuhotplugtest test does not call "qemuProcessStartManagedPRDaemon" > > > directly. It invokes "qemuDomainAttachDeviceDiskLive" which eventually > > > ends up calling "qemuProcessStartManagedPRDaemon" some way further > > > down the stack. > > > > > > > > > Then there is a shared library libqemuhotplugmock.so which contains a > > > replacement "qemuProcessStartManagedPRDaemon" to avoid us spawning > > > external programs. > > > > > > When it starts "qemuhotplugtest" will set LD_PRELOAD=libqemuhotplugmock.so > > > and then execve() itself. > > > > > > So when the test runs, the "qemuProcessStartManagedPRDaemon" impl from > > > the mock library is supposed to be used. > > > > > > If I run with LD_DEBUG=all on a build /without/ LTO, I can see this lookup > > > and override happening: > > > > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/lt-qemuhotplugtest > > > [0] > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libqemutestdriver.so > > > [0] > > > 381018: binding file > > > /home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libqemutestdriver.so > > > [0] to > > > /home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libqemutestdriver.so > > > [0]: normal symbol `qemuProcessStartManagedPRDaemon' > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/lt-qemuhotplugtest > > > [0] > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libvirhostdevmock.so > > > [0] > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libvirpcimock.so > > > [0] > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libdomaincapsmock.so > > > [0] > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libvirprocessmock.so > > > [0] > > > 381018: symbol=qemuProcessStartManagedPRDaemon; lookup in > > > file=/home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libqemuhotplugmock.so > > > [0] > > > 381018: binding file > > > /home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libqemutestdriver.so > > > [0] to > > > /home/berrange/src/fedora/libvirt/libvirt-6.5.0/x86_64-redhat-linux-gnu/tests/.libs/libqemuhotplugmock.so > > > [0]: normal symbol `qemuProcessStartManagedPRDaemon' > > > > > > > > > If I run LD_DEBUG=all on a build /with/ LTO, there are no symbol lookups > > > at all for qemuProcessStartManagedPRDaemon. It looks very much like the > > > call was resolved and bound at link time when built with LTO. > > > > Maybe it was not bound at link time, but inlined at compile time? > > > > I think it might be worthwhile to try and mark the > > qemuProcessStartManagedPRDaemon > > implementation which is used normally (no LD_PRELOAD) with some function > > attribute that it may be never inlined. I'm sure Florian and/or Jakub > > can help with what that function attribute should actually look like... > > We usually mark APIs we mock with G_GNUC_NO_INLINE to prevent such > problems. In this case we forgot to mark qemuProcessStartManagedPRDaemon > but it doesn't actually make a difference to the behaviour if we add the > missing G_GNUC_NO_INLINE annotation. I think the method impl is large enough > that the compiler would not consider it suitable for inlining regardless. THe inlining heuristics are a bit of black magic. I wouldn't be surprised if they were willing to do something like inline a large single use function into its caller. And more importantly, never rely on inlining or the lack thereof for a correctness issue :-)
Anyway, we've clearly got some debugging to do here. I'm actually taking a day of PTO to recharge after the last couple of hell weeks, but this'll be near the top of the queue Tuesday. jeff > _______________________________________________ devel mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected]
