[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2] Revert "Prevent more compiler optimization of mockable functions"

On Wed, Jul 12, 2017 at 01:10:08PM +0200, Martin Kletzander wrote:
> On Wed, Jul 12, 2017 at 11:14:16AM +0100, Daniel P. Berrange wrote:
> > This reverts commit e4b980c853d2114b25fa805a84ea288384416221.
> > 
> > When a binary links against a .a archive (as opposed to a shared library),
> > any symbols which are marked as 'weak' get silently dropped. As a result
> > when the binary later runs, those 'weak' functions have an address of
> > 0x0 and thus crash when run.
> > 
> > This happened with virtlogd and virtlockd because they don't link to
> > libvirt.so, but instead just libvirt_util.a and libvirt_rpc.a. The
> > virRandomBits symbols was weak and so left out of the virtlogd &
> > virtlockd binaries, despite being required by virHashTable functions.
> > 
> > Various other binaries like libvirt_lxc, libvirt_iohelper, etc also
> > link directly to .a files instead of libvirt.so, so are potentially
> > at risk of dropping symbols leading to a later runtime crash.
> > 
> > This is normal linker behaviour because a weak symbol is not treated
> > as undefined, so nothing forces it to be pulled in from the .a You
> > have to force the linker to pull in weak symbols using -u$SYMNAME
> > which is not a practical approach.
> > 
> How is this done by gnulib (or libc) when most their functions are weak
> aliases anyway?  Can't we use the same approach they have?
> virtlo{g,ck}d link with libgnu.la as well and there is no problem with
> that, right?  So I guess this _must_ be solvable somehow, IMHO.

gnulib doesn't use weak functions, it uses pre-processor black magic.
eg gnulib when replacing 'mkdir', gnulib provides a 'rpl_mkdir' function.
Its replacement header file contains a #define mkdir rpl_mkdir, so that
libvirt explicitly calls rpl_mkdir.

glibc's static libc.a file doesn't contain any weak symbols, only
its libc.so does, so that avoids the problem.

So we could avoid it by compiling our code twice, but that is not
acceptable imho because of the time penalty

|: 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 :|

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]