keremet/rpm-ostree
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Makefile.am: Link with --enable-new-dtags

Browse Source 
When g-ir-scanner runs, it uses a stub binary to help introspect the
target rpm-ostree library. That binary needs to link to the library, and
of course, transitively, to our bundled libdnf. Since we run the scanner
uninstalled, we use `LD_LIBRARY_PATH` to point the stub at the libdnf
from the build directory.

When we compile rpm-ostree, we use `-rpath` to point it at our libdnf,
which emits a `DT_RPATH` attribute. However, during dynamic linking at
runtime, it turns out that `LD_LIBRARY_PATH` has *lower* precedence than
`DT_RPATH`. This means that if there is already a libdnf at
`/usr/lib64/rpm-ostree`, it takes precedence.

This subtlety is mostly fine to ignore usually, because the stub doesn't
actually run any rpm-ostree business logic, so it doesn't really matter
that it runs against the "technically wrong" `libdnf.so.2`.

Where it becomes obvious something is off however is if we've just
built a new libdnf, and we have new code which references symbols from
the new libdnf that aren't in the stale libdnf at `$libdir/rpm-ostree`.

And this is exactly what was happening in CI (though it's of course
possible to reproduce this locally as well): we were using a new symbol,
`hy_goal_favor`, and building inside the cosa buildroot image, which
already has rpm-ostree installed, and so `ld.so` chose the bundled
libdnf of the installed rpm-ostree when loading the `g-ir-scanner` stub.
Thus why it failed with:

```
./tmp-introspectzh0n2cga/.libs/lt-RpmOstree-1.0: symbol lookup error: .libs/librpmostree-1.so.1: undefined symbol: hy_goal_favor
```

And of course, the root cause here has nothing to do with the stub in
itself. It's actually trivially easy to see the behaviour difference wrt
`LD_LIBRARY_PATH` on installed binaries. Using a shared object
from `python3-libdnf` which links to libdnf without using rpath, we can
see that `LD_LIBRARY_PATH` has an effect:

```
$ ldd /usr/lib64/python3.7/site-packages/libdnf/_transaction.so | grep libdnf
        libdnf.so.2 => /lib64/libdnf.so.2 (0x00007fdde789e000)
$ LD_LIBRARY_PATH=./libdnf-build/libdnf ldd /usr/lib64/python3.7/site-packages/libdnf/_transaction.so | grep libdnf
        libdnf.so.2 => ./libdnf-build/libdnf/libdnf.so.2 (0x00007fa615048000)
```

Whereas with rpm-ostree:

```
$ ldd /usr/bin/rpm-ostree | grep libdnf
        libdnf.so.2 => /usr/lib64/rpm-ostree/libdnf.so.2 (0x00007f7da5271000)
$ LD_LIBRARY_PATH=./libdnf-build/libdnf ldd /usr/bin/rpm-ostree | grep libdnf
        libdnf.so.2 => /usr/lib64/rpm-ostree/libdnf.so.2 (0x00007fc905dbd000)
```

And going further down the rabbit hole, `DT_RPATH` is in fact considered
deprecated for this reason; it makes it harder to run with uninstalled
libraries or whatever reason one has to want to override a library (one
can still use `LD_PRELOAD`, though the semantics are different, and it's
less commonly used than `LD_LIBRARY_PATH`). Instead, we should use
`DT_RUNPATH`, which does have lower precedence than `LD_LIBRARY_PATH`,
matching the usual behaviour.

To make the linker emit `DT_RUNPATH` attributes, we have to use the
`--enable-new-dtags` flag. This also then fixes the g-ir-scanner issue
described above.
This commit is contained in:
Jonathan Lebon 2020-05-11 14:08:45 -04:00 committed by OpenShift Merge Robot
parent 2b52b33e19
commit 92a83ad722
2 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Makefile-lib.am
View File

@ -36,7 +36,9 @@ librpmostree_1_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libp
librpmostree_1_la_LDFLAGS = $(AM_LDFLAGS) -version-number 1:0:0 -Bsymbolic-functions librpmostree_1_la_LDFLAGS = $(AM_LDFLAGS) -version-number 1:0:0 -Bsymbolic-functions
librpmostree_1_la_LIBADD = $(PKGDEP_RPMOSTREE_LIBS) librpmostreepriv.la $(librpmostree_rust_path) librpmostree_1_la_LIBADD = $(PKGDEP_RPMOSTREE_LIBS) librpmostreepriv.la $(librpmostree_rust_path)
# bundled libdnf # The g-ir-scanner creates a stub executable (to help introspect type information) which
# links to our stuff. We want to make sure it picks up our fresh libdnf and not a possibly
# stale one from a previously installed rpm-ostree's bundled libdnf.
INTROSPECTION_SCANNER_ENV = env LD_LIBRARY_PATH=$(top_builddir)/libdnf-build/libdnf INTROSPECTION_SCANNER_ENV = env LD_LIBRARY_PATH=$(top_builddir)/libdnf-build/libdnf
# XXX: work around clang being passed -fstack-clash-protection which it doesn't understand # XXX: work around clang being passed -fstack-clash-protection which it doesn't understand

5
Makefile.am
View File

@ -44,8 +44,9 @@ AM_CPPFLAGS += -DDATADIR='"$(datadir)"' \
# Keep this in sync with the AM_CFLAGS in libostree; see # Keep this in sync with the AM_CFLAGS in libostree; see
# that project for more information about e.g. -fno-strict-aliasing # that project for more information about e.g. -fno-strict-aliasing
AM_CFLAGS += -std=gnu11 -fno-strict-aliasing $(WARN_CFLAGS) AM_CFLAGS += -std=gnu11 -fno-strict-aliasing $(WARN_CFLAGS)
# bundled libdnf # bundled libdnf; --enable-new-dtags emits DT_RUNPATH instead of deprecated DT_RPATH, which
AM_LDFLAGS += "-Wl,-rpath=$(libdir)/rpm-ostree" # can't be overridden by LD_LIBRARY_PATH; which is useful for uninstalled runs/tests
AM_LDFLAGS += "-Wl,-rpath=$(libdir)/rpm-ostree,--enable-new-dtags"
EXTRA_DIST += autogen.sh COPYING EXTRA_DIST += autogen.sh COPYING