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.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
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/devel@lists.fedoraproject.org

Reply via email to