Mic92
commented
7 years ago Is it unambiguous, which library should be linked? Does ELF even allow this?
Open lheckemann opened 7 years ago
Mic92
commented
7 years ago Is it unambiguous, which library should be linked? Does ELF even allow this?
abbradar
commented
7 years ago Another possible issue is if LD_PRELOAD and LD_LIBRARY_PATH would still work with those binaries. With that all cleared however it seems a definite win to me!
lheckemann
commented
7 years ago Does ELF even allow this?
http://www.skyfree.org/linux/references/ELF_Format.pdf not a canonical source as far as I can tell, but the "Shared Object Dependencies" section suggests so.
Another possible issue is if
LD_PRELOADandLD_LIBRARY_PATHwould still work with those binaries.
I don't think LD_LIBRARY_PATH would still work for replacing the libraries that would then be linked through absolute pathnames (but is that desirable? The only case where I think it would be useful is the OpenGL drivers). I think LD_PRELOAD would (based on the fact that the libraries specified there are the first to be loaded before any others), but again I'm not sure.
dezgeg
commented
7 years ago Yes, LD_LIBRARY_PATH won't be searched if the DT_NEEDED entry contains a slash (_dl_map_object in glibc sources). So the current libGL driver selection breaks.
Hopefully we have that converted to use libglvnd some day. Maybe @edolstra knows other places where LD_LIBRARY_PATH is used and/or needed.
dezgeg
commented
7 years ago I whipped up a patch to patchelf to implement this in case anybody wants to try to see what breaks: https://github.com/dezgeg/patchelf/tree/abs
abbradar
commented
7 years ago One example that I can remember is scanner drivers (sane). For example on my system:
LD_LIBRARY_PATH=/nix/store/4m8l0zzdyl5lynsdx5gjml07zplph9gl-sane-config/lib/sane:/run/opengl-driver/lib:/run/opengl-driver-32/libI have a libglvnd branch in progress but wanted to wait some more until Fedora ships its support.
dezgeg
commented
7 years ago But are the sane plugin .so:s referenced by DT_NEEDED and not dlopen():ed? I don't think why dlopen() with LD_LIBRARY_PATH wouldn't work anymore.
abbradar
commented
7 years ago @dezgeg True! That leaves us with libGL (libglvnd to the rescue but AMD proprietary drivers don't support it currently), libEGL, libGLES and others (some of those are still not supported bylibglvnd, I don't remember exactly which, but they are planned) and, last but not least, cases when people want to force their library paths. Not sure about latter but it definitely could happen.
vcunat
commented
7 years ago Very interesting. I didn't know the loader can be so easily convinced to absolute path searches (without changing sonames). It would really be a significant change to basic nix(pkgs) principles, maybe worth of an RFC before we switch (but I'd favor more investigation+testing before RFC).
libGL: we could patch that selectively – either converting it to non-absolute or removing the R(UN)PATH item completely.
~Looking at the comment about DT_RUNPATH, I wonder what would happen if two instances of the same library are attempted to be loaded into a single process (same sonames but different /nix/store paths). For example, libGL may often have been linked against a different instance of libc...~ EDIT: now I see this part has been used all the time, if I understand it correctly.
dezgeg
commented
7 years ago If I read the logic in glibc correctly, yes, loading libraries (by both DT_NEEDED or dlopen()) that have same sonames but different /nix/store paths will load both of them wheras the current RUNPATH-based approach would load only the first copy.
Profpatsch
commented
7 years ago philip@katara ~/c/n/cabal2nix (master)> strace -e trace=file result/bin/hoogle 2>&1 | grep "open.*so.*= 3.*" | wc -l
128
philip@katara ~/c/n/cabal2nix (master)> strace -e trace=file result/bin/hoogle 2>&1 | grep "open.*so.*= -1.*" | wc -l
8829
philip@katara ~/c/n/cabal2nix (master)> strace -e trace=file result/bin/purs 2>&1 | grep "open.*so.*= 3.*" | wc -l
166
philip@katara ~/c/n/cabal2nix (master)> strace -e trace=file result/bin/purs 2>&1 | grep "open.*so.*= -1.*" | wc -l
14615That’s the current ratio of .so stat hits to misses for hoogle and the purescript compiler. hoogle has a startup time delay of 2s and purs of >4s on my SSD-based system. When compiled statically, the startup of both programs is instantaneous. cc @peti
Profpatsch
commented
7 years ago Profpatsch | Ah, I understand now!
Profpatsch | The problem with rpath.
Profpatsch | So you have one rpath entry for each haskell Dependency.
Profpatsch | And it tries to find foobar.so
dtzWill | and looks through all the things
Profpatsch | It will linearly stat *every* rpath entry until it finds the so.
Profpatsch | which means n^2
Profpatsch | Or probably (n^2)/2 in the mean case.
hodapp | Still O(n^2) :P
Profpatsch | purs has 168 deps
Profpatsch | -5 for the normal .so’s the Haskell runtime needs
Profpatsch | 163^2 = 26.5k
Profpatsch | /2 and we arrive at the 13k misses I measured before.
Profpatsch | Science!
dtzWill | ٩(^ᴗ^)۶
hodapp | hah Yeah, exactly. Although I'm not sure your statistics for hits are correct though — It's possible that all of the open calls result in the FD 3 but not guaranteed I think, so rather than grep "open.*so.*= 3.*" you'd probably want to grep "open.*so.*= [^-].*" so that every non-negative return value is considered a success.
copumpkin
commented
7 years ago On Darwin, all of our Nix library references are absolute by default, even though we do use rpath in some places. Just FYI for "prior art" 😄
vcunat
commented
7 years ago No! Now I can't patent the technique ;-)
Profpatsch
commented
7 years ago I wrote a WIP patch for haskellPackages, using @dezgeg’s patch for patchelf.
https://github.com/Profpatsch/nixpkgs/commit/93cfe6eb53364d3b8ca617fb9323c6f1b46ad284
It fails because of broken ELF binaries, though. Probably a bug in the patchelf .dynstr size handling.
lheckemann
commented
7 years ago FWIW this would break this hack for changing freetype behaviour — at least in its current form. It should of course still work using LD_PRELOAD (which may be a better idea in general).
lheckemann
commented
7 years ago The primary approach for implementing this should probably be making libraries and executables link with the absolute paths from the get-go rather than patching them post-hoc, right? Does anyone know how this would be done?
Profpatsch
commented
7 years ago Does anyone know how this would be done?
I experimented with that a few days in April; the GNU linker describes how it will skip search heuristics if it finds absolute paths in the ELF header. Now comes the fun part: the GNU linker also does compile time linking and (as far as I could see) they do not provide the ability to link in absolute paths. There simply are no options that do that. I think I never facepalmed so hard before.
vcunat
commented
7 years ago One advantage for a fixup-phase approach: it should work even on binary packages. (Though I personally don't consider that a significant pro.)
lheckemann
commented
7 years ago It's certainly more versatile, but it still feels wrong: why compile something the wrong way then fix it rather than compiling it right in the first place?
vcunat
commented
7 years ago I understand that sentiment, but if our default linker doesn't allow it, we'd have to patch the linker to add this (optional) functionality, I guess, and that feels rather cumbersome to maintain. (not mentioning the additional complications if someone wants to use a different linker, etc.) Still, it's possible the other option won't turn out easy either.
dtzWill
commented
7 years ago As for doing it w/o fixup, I think we can replace -lfoo with /path/to/libfoo.so in the ld wrapper?
Shouldn't be too much trouble since we already have misc support for basically gathering all the "-L" paths and locating each library anyway.
FWIW a quick look into this particular issue leads to these relevant discussions:
catkin discussion
binutils issue about intentionally removing/"fixing" support for -l:/path/to/libfoo.so
Doing this with a fixup sounds good to me as well, maybe give both a try?
EDIT: Absolute path for libc.so has problems because it's really a script. Can special-case this, but might happen elsewhere. IIRC libc++.so does this sort of thing, at least optionally. This suggests a fixup might be easier?
(Err I suppose this "only" happened because I also removed all "-L" switches.)
ben0x539
commented
7 years ago I don't think passing absolute paths to ld actually makes those paths show up in the generated binary?
dtzWill
commented
7 years ago @ben0x539 seems to for me? Here's a quick test:
with import <nixpkgs> {};
rec {
foo = stdenv.mkDerivation {
name = "libfoo";
buildCommand = ''
echo "int foo() { return 5; }" >> foo.c
cc foo.c -o libfoo.so -shared
mkdir -p $out/lib
mv libfoo.so $out/lib
'';
};
main = stdenv.mkDerivation {
name = "abspath-test";
buildInputs = [ foo ];
buildCommand = ''
cat >>main.c <<EOF
#include <stdio.h>
int foo();
int main() {
printf("foo: %d\n", foo());
return 0;
}
EOF
cc main.c -o main -lfoo
mkdir -p $out/bin
mv main $out/bin/
'';
};
main-abs = stdenv.mkDerivation {
name = "abspath-test";
buildInputs = [ foo ];
buildCommand = ''
cat >>main.c <<EOF
#include <stdio.h>
int foo();
int main() {
printf("foo: %d\n", foo());
return 0;
}
EOF
cc main.c -o main ${foo}/lib/libfoo.so
mkdir -p $out/bin
mv main $out/bin/
'';
};
}
$ nix-build -A main -o main
$ nix-build -A main-abs -o main-abs
$ readelf -d main/bin/main
Dynamic section at offset 0xde8 contains 25 entries:
Tag Type Name/Value
0x0000000000000001 (NEEDED) Shared library: [libfoo.so]
0x0000000000000001 (NEEDED) Shared library: [libc.so.6]
0x000000000000001d (RUNPATH) Library runpath: [/nix/store/nirdkvlp2wim11mh0fj0bbpmdq6m5rg4-abspath-test/lib64:/nix/store/nirdkvlp2wim11mh0fj0bbpmdq6m5rg4-abspath-test/lib:/nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/libexec/gcc/x86_64-unknown-linux-gnu/5.4.0:/nix/store/h6np4yvv6j7qcp3ikh4gcgj49zgjsb7n-libfoo/lib:/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib:/nix/store/89bjrnjb8c9vvhzb92fz832x6fwg3kbj-gcc-5.4.0-lib/lib]
0x000000000000000c (INIT) 0x4006c8
0x000000000000000d (FINI) 0x4008a4
0x0000000000000019 (INIT_ARRAY) 0x600dd0
0x000000000000001b (INIT_ARRAYSZ) 8 (bytes)
0x000000000000001a (FINI_ARRAY) 0x600dd8
0x000000000000001c (FINI_ARRAYSZ) 8 (bytes)
0x0000000000000004 (HASH) 0x4002e0
0x0000000000000005 (STRTAB) 0x400410
0x0000000000000006 (SYMTAB) 0x400320
0x000000000000000a (STRSZ) 528 (bytes)
0x000000000000000b (SYMENT) 24 (bytes)
0x0000000000000015 (DEBUG) 0x0
0x0000000000000003 (PLTGOT) 0x600fc8
0x0000000000000007 (RELA) 0x400668
0x0000000000000008 (RELASZ) 96 (bytes)
0x0000000000000009 (RELAENT) 24 (bytes)
0x000000000000001e (FLAGS) BIND_NOW
0x000000006ffffffb (FLAGS_1) Flags: NOW
0x000000006ffffffe (VERNEED) 0x400638
0x000000006fffffff (VERNEEDNUM) 1
0x000000006ffffff0 (VERSYM) 0x400620
0x0000000000000000 (NULL) 0x0
$ readelf -d main-abs/bin/main
Dynamic section at offset 0xde8 contains 25 entries:
Tag Type Name/Value
0x0000000000000001 (NEEDED) Shared library: [/nix/store/h6np4yvv6j7qcp3ikh4gcgj49zgjsb7n-libfoo/lib/libfoo.so]
0x0000000000000001 (NEEDED) Shared library: [libc.so.6]
0x000000000000001d (RUNPATH) Library runpath: [/nix/store/2x61jy86dxzzyp88vah6hk1s0556bx1z-abspath-test/lib64:/nix/store/2x61jy86dxzzyp88vah6hk1s0556bx1z-abspath-test/lib:/nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/libexec/gcc/x86_64-unknown-linux-gnu/5.4.0:/nix/store/h6np4yvv6j7qcp3ikh4gcgj49zgjsb7n-libfoo/lib:/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib:/nix/store/89bjrnjb8c9vvhzb92fz832x6fwg3kbj-gcc-5.4.0-lib/lib]
0x000000000000000c (INIT) 0x400700
0x000000000000000d (FINI) 0x4008e4
0x0000000000000019 (INIT_ARRAY) 0x600dd0
0x000000000000001b (INIT_ARRAYSZ) 8 (bytes)
0x000000000000001a (FINI_ARRAY) 0x600dd8
0x000000000000001c (FINI_ARRAYSZ) 8 (bytes)
0x0000000000000004 (HASH) 0x4002e0
0x0000000000000005 (STRTAB) 0x400410
0x0000000000000006 (SYMTAB) 0x400320
0x000000000000000a (STRSZ) 583 (bytes)
0x000000000000000b (SYMENT) 24 (bytes)
0x0000000000000015 (DEBUG) 0x0
0x0000000000000003 (PLTGOT) 0x600fc8
0x0000000000000007 (RELA) 0x4006a0
0x0000000000000008 (RELASZ) 96 (bytes)
0x0000000000000009 (RELAENT) 24 (bytes)
0x000000000000001e (FLAGS) BIND_NOW
0x000000006ffffffb (FLAGS_1) Flags: NOW
0x000000006ffffffe (VERNEED) 0x400670
0x000000006fffffff (VERNEEDNUM) 1
0x000000006ffffff0 (VERSYM) 0x400658
0x0000000000000000 (NULL) 0x0Exploring the idea of doing this by modifying ld-wrapper in this branch.
YMMV, let's see if this gets an absolute-path-linked gcc :).
I'd feel better if we had a fixup phase regardless, to either catch and abort if things aren't absolute or to handle cases missed for whatever reason.
Currently I'm handling libc.so-style things by preserving the "-L" flags so that the libraries it mentions (libc.so.6, libc_nonshared.a, and the dynamic linker) are successfully resolved. This could be cleaned up by the fixup phase?
Or maybe just doing it all in the fixup phase would be simpler? (Especially if a fixup phase is used anyway...?)
Update: branch currently gets mostly through gcc, but dies when it attempts to link against a shared object when -static is given. I'm not sure how "worth" it is to re-implement the library search behavior like this, or if there are likely other flags that cause problems.
Solving this with a fixup has the appeal of being straightforward to implement, and as @vcunat mentioned would work on pre-compiled binaries as well.
ben0x539
commented
7 years ago ... right, I was just using some random existing .so file and that had the SONAME set, so it was linked by that rather than path. Whoops.
Thanks for clarifying!
On Sat, 2017-06-24 at 10:36 -0700, Will Dietz wrote:
@ben0x539 seems to for me? Here's a quick test:
with import <nixpkgs> {}; rec { foo = stdenv.mkDerivation { name = "libfoo"; buildCommand = '' echo "int foo() { return 5; }" >> foo.c cc foo.c -o libfoo.so -shared mkdir -p $out/lib mv libfoo.so $out/lib ''; }; main = stdenv.mkDerivation { name = "abspath-test"; buildInputs = [ foo ]; buildCommand = '' cat >>main.c <<EOF #include <stdio.h> int foo(); int main() { printf("foo: %d\n", foo()); return 0; } EOF cc main.c -o main -lfoo mkdir -p $out/bin mv main $out/bin/ ''; }; main-abs = stdenv.mkDerivation { name = "abspath-test"; buildInputs = [ foo ]; buildCommand = '' cat >>main.c <<EOF #include <stdio.h> int foo(); int main() { printf("foo: %d\n", foo()); return 0; } EOF cc main.c -o main ${foo}/lib/libfoo.so mkdir -p $out/bin mv main $out/bin/ ''; }; }
Profpatsch
commented
7 years ago but if our default linker doesn't allow it, we'd have to patch the linker to add this (optional) functionality, I guess, and that feels rather cumbersome to maintain.
Somebody else should take a look at it first, I might have overlooked a flag with ld(1); of course every linker probably has a different option for that. So we should at least add a check that emits a warning in fixup phase if some ELF paths are not absolute.
dtzWill
commented
7 years ago @ben0x539 ah, you're right and since most libraries' set soname to something short that does mean the simple ld-wrapper approach wouldn't do the trick.
That said, it /does/ appear that if you set soname to the absolute path then when using '-lfoo' the absolute path is added to NEEDED which I think is what we want anyway.
Is it possible to patch libraries' soname's after-the-fact without too much trouble?
EDIT: looks like patchelf has a "--set-soname" option? :grin:
Profpatsch
commented
7 years ago seems to for me? Here's a quick test:
Oh, so it’s possible? Interesting. How does cc forward absolute .so paths to ld?
lheckemann
commented
7 years ago Adding -v to the flags shows that collect2 still gets the absolute path, and calling collect2 manually and adding -v to its flags shows that ld also gets the absolute path:
/nix/store/b43ww9wkpwjxha7ql7mizxpa2vb6yk5r-gcc-wrapper-5.4.0/bin/ld -v -plugin /nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/libexec/gcc/x86_64-unknown-linux-gnu/5.4.0/liblto_plugin.so -plugin-opt=/nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/libexec/gcc/x86_64-unknown-linux-gnu/5.4.0/lto-wrapper -plugin-opt=-fresolution=/run/user/1000/ccR5OZQi.res -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s --eh-frame-hdr -m elf_x86_64 -dynamic-linker /nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib64/ld-linux-x86-64.so.2 -z relro -z now /nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/crt1.o /nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/crti.o /nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/crtbegin.o -L/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib -L/nix/store/89bjrnjb8c9vvhzb92fz832x6fwg3kbj-gcc-5.4.0-lib/lib -L/nix/store/89bjrnjb8c9vvhzb92fz832x6fwg3kbj-gcc-5.4.0-lib/lib -L/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib -L/nix/store/b43ww9wkpwjxha7ql7mizxpa2vb6yk5r-gcc-wrapper-5.4.0/bin -L/nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0 -L/nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../../../lib64-L/nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/../../.. -dynamic-linker /nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/ld-linux-x86-64.so.2 test.o result/lib/libfoo.so -rpath /nix/store/jdhfkxysk2li6h67wih3z6ikbqzmn30r-shell/lib64 -rpath /nix/store/jdhfkxysk2li6h67wih3z6ikbqzmn30r-shell/lib -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /nix/store/my9f454r82wy6jv3x7bvvpx6hffcahsj-gcc-5.4.0/lib/gcc/x86_64-unknown-linux-gnu/5.4.0/crtend.o /nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/crtn.oFor a less obtuse and more readable way of seeing this, you can do...
$ nix-build test.nix
$ gcc -x c -c -o test.o - <<<"int foo(); int main() {return foo();}"
$ ld test.o
/nix/store/iz98xp5p48jayn034wq89y30plpx5xns-binutils-2.27/bin/ld: warning: cannot find entry symbol _start; defaulting to 00000000004000f0
test.o: In function `main':
:(.text.startup+0x3): undefined reference to `foo'
$ ld test.o result/lib/libfoo.so
/nix/store/iz98xp5p48jayn034wq89y30plpx5xns-binutils-2.27/bin/ld: warning: ld-linux-x86-64.so.2, needed by /nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6, not found (try using -rpath or -rpath-link)
/nix/store/iz98xp5p48jayn034wq89y30plpx5xns-binutils-2.27/bin/ld: warning: cannot find entry symbol _start; defaulting to 00000000004003d0
/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6: undefined reference to `_dl_argv@GLIBC_PRIVATE'
/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6: undefined reference to `_dl_find_dso_for_object@GLIBC_PRIVATE'
/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6: undefined reference to `__libc_enable_secure@GLIBC_PRIVATE'
/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6: undefined reference to `_rtld_global@GLIBC_PRIVATE'
/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6: undefined reference to `__tls_get_addr@GLIBC_2.3'
/nix/store/7crrmih8c52r8fbnqb933dxrsp44md93-glibc-2.25/lib/libc.so.6: undefined reference to `_rtld_global_ro@GLIBC_PRIVATE'(ok, it doesn't work properly, but it does illustrate that the reference to foo is resolved)
Ericson2314
commented
7 years ago Somewhat relatedly, I patched cctools in https://github.com/tpoechtrager/cctools-port/pull/34 as a step towards better solving the sierra linking problems. If we likewise patch binutils, then ld-wrapper can leverage ld to resolve libraries rather than fake it on its own.
CC @orivej
copumpkin
commented
7 years ago Just for another update, we've now killed rpath almost entirely in nixpkgs on Darwin in #30150. I have faith that Linux can pull it off too! 😄
ento
commented
6 years ago As a quick n' dirty workaround for https://github.com/NixOS/nixpkgs/issues/327854, I wrote a script that scans all ELF files in /nix/store and replaces their NEEDEDs with their absolute paths (if found under one of the RUNPATHs), and then found out that nix-env -i doesn't work (as in segfaults) when libc.so.6 is linked through an absolute path because of an internal check performed inside libc. I've issued it in glibc's bug tracker.
lheckemann
commented
6 years ago I've opened #45105 which implements the change based on DT_SONAME, and it seems to work for the most part. Still running into some issues with building initrds, any help would be appreciated!
wmertens
commented
5 years ago Reminds me of prelinking, but the problem with that is that each library needs to be assigned a fixed address space and the order of inserting packages in the store determines the location, so it's not stateless.
On macOS the linker also prelinks but the link information is stored separately from the binaries and is auto-updated. I wonder if it would be hard to make our own linker that does that.
ambrop72
commented
5 years ago What no one here has pointed out before is that for a library L and library path P, the loader check for the library at the following paths (the haswell part surely depends on the runtime CPU, not sure about tls):
P/tls/haswell/x86_64/LP/tls/haswell/LP/tls/x86_64/LP/tls/LP/haswell/x86_64/LP/haswell/LP/x86_64/LP/L (where the library is most likely to be found)My impression is that few or no packages in NixOS have libraries in these machine directories (this is something to be checked over the entire nixpkgs). If this is indeed completely unused, then we can patch or configure glibc to not look in machine directories.
Another interesting thing is that for every failing open() call it also calls stat() on the corresponding directory. From the code we can see that it does this in order to remember whether the directory exists. If it does not it will not look in this directory again. This can easily be seen in strace, all those nonexisting machine directories will only be consulted once for every library path. This means that eliminating machine directory checks would improve performance of library searching less than 8x, but I think it would still be very significant.
Another thing to be aware of is that absolute linking can only be done when the linked library does not have variants in machine directories (either by actually checking all potential machine directories, or just assuming nothing in NixOS uses them).
I think I will try making glibc not check machine directories and see what happens.
ambrop72
commented
5 years ago If we do this then we could also try disabling the directory-existence check and caching. Because once we're not checking machine directories, almost all candidate library directories do exist (because of RUNPATH stripping for example), so doing the stat calls and maintaining the cache is probably costlier than just checking for the library in all candidate directories.
dtzWill
commented
5 years ago On Mon, 06 May 2019 14:07:09 -0700, Ambroz Bizjak notifications@github.com wrote:
If we do this then we could also try disabling the directory-existence check and caching. Because once we're not checking machine directories, almost all candidate library directories do exist (because of
RUNPATHstripping for example), so doing the stat calls and maintaining the cache is probably costlier than just checking for the library in all candidate directories.
:+1: and especially for dropping the arch bits. We can use an env var to help with any compat issues, if things progress to that point.
For some of these searches, it almost seems possible we could bind-mount the paths together (directory "union"? :D) -- but that might be slow or problematic.
Anyway thanks for pushing this, please share any/all findings/results!
<3
-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/NixOS/nixpkgs/issues/24844#issuecomment-489776899Non-text part: text/html
ambrop72
commented
5 years ago I tried doing the changes suggested above on my desktop system. The patch is here. After a long mass rebuild it booted successfully with no obvious issues. Verified with strace that the behavior is as intended: no checking of machine directories and no xstat, e.g.:
openat(AT_FDCWD, "/nix/store/d9sxqvvcacd00nimmmnfgvsflqcvwpqb-acl-2.2.53/lib/libcrypto.so.1.0.0", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/nix/store/nla5kgmqwi5hnbr7v2ja7x1pwfadqf8p-attr-2.4.48/lib/libcrypto.so.1.0.0", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/nix/store/fwan169di2kgnvf2ig1ssmj4i2c82nyh-openssl-1.0.2r/lib/libcrypto.so.1.0.0", O_RDONLY|O_CLOEXEC) = 3However, there was no difference in execution time of echo hello and konsole -v, which were 0.004s and 0.085s respectively. So while the library loading was optimized, there was no measurable effect on program startup time on my system at least.
lheckemann
commented
5 years ago @ambrop72 I think you're not making enough samples :) I compared running ${coreutils}/bin/echo hello 1000 times, and it does seem to make a significant difference — the hello from unmodified nixpkgs takes about 1.0s on my server machine and the hello from your patched nixpkgs takes about 0.7s. These figures are quite stable.
That said, this is mostly orthogonal to the general search issue and doesn't fix the quadratic behaviour of the library search, only providing a linear improvement in load speed. It may also have significant detrimental performance effects on software that does provide hwcap-specific libraries.
lheckemann
commented
5 years ago Another idea: rather than going into the deep, dark corners of the loader's behaviour, we could also generate a directory containing symlinks to all the libraries that libraries and executables inside a derivation's output use and make that the first entry in the rpath.
Advantages:
LD_LIBRARY_PATHDisadvantages:
Any other thoughts on this approach?
edolstra
commented
5 years ago @lheckemann Another disadvantage is that it might end up being quite expensive in terms of disk space. Symlinks are not free, depending on the file system each symlink might well take 4 KiB. With Nix these symlink trees really add up, e.g. the systemPaths in NixOS can easily consume a few gigabytes.
It could also lead to more I/O at runtime, perhaps exceeding the speedup from reducing the number of file lookups.
lheckemann
commented
5 years ago Indeed…
It could also lead to more I/O at runtime, perhaps exceeding the speedup from reducing the number of file lookups.
I don't understand how?
edolstra
commented
5 years ago Well, every executable (or at least every package) will have its own lib symlinks tree, which all need to be read from disk. Whereas in the current situation, you get a bunch of stat/open calls for paths that are already likely to be in the dentry cache (e.g. ${glibc}/lib/libfoo.so).
ambrop72
commented
5 years ago I agree with @edolstra about the problems with symlink trees. Every package would need such a symlink tree, programs and libraries. Even if it would improve performance, I think we should be looking for less hacky solutions.
I understand that the following were attempted separately without conclusive results:
DT_NEEDED to absolute paths using patchelf.DT_NEEDED.What I dislike about both of the above is that virtually nobody does it that way in the existing Linux ecosystem. I suspect that even if we make a good effort to make it work, we will keep having problems, because software just doesn't expect this (in many cases it will come down to the different-versions problem). Note that these solutions also prevent overriding libraries (which is a hack but sometimes necessary).
I think we should try to restrict the search path for each DT_NEEDED entry individually. The problem with RUNPATH is that it affects all DT_NEEDED entries, resulting in quadratic time. We could extend the dynamic linker to allow a DT_NEEDED entry to itself specify search paths, for example by putting the search path after the library name like this: DT_NEEDED=libfoo.so.1:/dir1/lib:/dir2/lib.
The current automatic RUNPATH stripping in fixupPhase (patchelf --shrink-rpath) could be replaced with migrating RUNPATH directories to DT_NEEDED entries that need it. This would always remove RUNPATH so binaries generally wouldn't have RUNPATH, except when needed to find libraries loaded using dlopen.
ambrop72
commented
5 years ago In essence this would be a library-specific RUNPATH:
RUNPATH or RPATH for that DT_NEEDED entry.RUNPATH.LD_LIBRARY_PATH would still work the same way.I will try implementing this in glibc to evaluate the performance improvement. Anyway I think if we are to fix the performance problem we will end up having to make glibc changes, we shouldn't be dissuaded by this.
lheckemann
commented
5 years ago we will keep having problems, because software just doesn't expect this
We already have this problem all the time because we don't put everything in /usr :)
Note that these solutions also prevent overriding libraries
Only with LD_LIBRARY_PATH. LD_PRELOAD should still work, I think.
As for patching glibc, I think that's even further in the domain of "virtually nobody does it that way". Not to say that I'm opposed to it :grin:
One other option that might be worth considering is using the existing ld.so.cache mechanism, but generating per-package caches instead of a global shared cache.
ambrop72
commented
5 years ago We already have this problem all the time because we don't put everything in
/usr:)
But this problem has been addressed since the beginning of NixOS, and we got used to dealing with it. Fundamentally changing how libraries are linked could cause breakages over the entire nixpkgs and increase maintenance effort. We would also lose the machine library directory capability with absolute linking in general.
One other option that might be worth considering is using the existing
ld.so.cachemechanism, but generating per-package caches instead of a global shared cache.
Looks possible, but:
ldconfig would need to be patched to work for this case. I can't even figure out how to generate a cache file for a trivial case with a directory containing a single library. Looks like it is trying to chroot and failing (since it's not root). And restricting to a certain set of libraries would need to be implemented.We should also strive to make the simplest solution and not deviate too much from how things are done now. Making fundamental changes causes breakage and increases maintenance effort if we deviate from conventions.
As far as my proposal is concerned, from looking at the code, I now think that instead of adding search directories to DT_NEEDED, we should add library names to DT_RUNPATH entries. For example like this: DT_RUNPATH=/foo/lib[libfoo.so.1,libfoo-extra.so.1]:/bar/lib[libbar.so.2]. So the first path would only be considered when libfoo.so.1 or libfoo-extra.so.1 is being loaded. I think that would be simpler to implement and closer to existing conventions.
What would be needed:
DT_RUNPATH extension in glibc. The changes needed are localized to search-path code (r_search_path_struct, decompose_rpath, open_path), mostly just adding new code and no major refactoring. Note that this syntax would also work for RPATH and LD_LIBRARY_PATH because the same code is used.patchelf so that --shrink-rpath adds these [sonames] to each directory in DT_RUNPATH. It already knows what they are because it checks DT_NEEDED to see if the directory should be stripped, so this should be simple.DT_RUNPATH, but appending/prepending to DT_RUNPATH (--print-rpath+--set-rpath) will be fine.
Issue description
echo's dynamic section currently looks like this...Because we don't have any caching as far as I understand, this means that when echo is run, a whole lot of fairly superfluous system calls are made:
This issue is more pronounced for programs that link to lots of libraries, as the linker will go through the RPATH for each library linked to. As far as I understand it, the
DT_NEEDEDentries in the dynamic relocation section could contain absolute paths, which would reduce the amount of searching necessary. Additionally, this ties the library reference more closely to the derivation's build inputs.Any thoughts? I haven't got any hard data on the performance impact this has (though the fact that most distros use the ld.so.cache mechanism to speed up loading suggests that it is relevant at least in some contexts), or any idea what other consequences of using absolute paths would be, but having executable loading be O(n²) for the number of referenced store components just somewhat bothers me.
Technical details