Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] binutils, build-fhs-userenv: make use of ld fallback #59595

Draft
wants to merge 2 commits into
base: staging
Choose a base branch
from
Draft

[WIP] binutils, build-fhs-userenv: make use of ld fallback #59595

wants to merge 2 commits into from

Conversation

timokau
Copy link
Member

@timokau timokau commented Apr 15, 2019 •
edited

Motivation for this change

Currently build-fhs-userenv uses LD_LIBRARY_PATH to instruct the dynamic linker to look at /usr/lib. This is necessary since nix explicitly forbids its dynamic linker to look in that default location.

This doesn't perfectly emulate the "normal" behaviour however, since LD_LIBRARY_PATH has precedence over a binaries own runpath. In contrast, the default directories (/usr/lib) are normally checked after the runpath.

This can lead to confusing bugs when system libraries are preferred over runpath in self compiled binaries.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/how-does-dynamic-linking-in-nix-work/2685/5

@Ericson2314
Copy link
Member

Can we make NATIVE_LIB_DIRS configurable so we aren't baking in a policy so deeply (with respect to mass rebuilds?) I would like this a lot better then.

@timokau
glibc: read /etc/nix-ld.so.conf for dynamic linker config
e25186d
@timokau
build-fhs-userenv: make use of dynamic linker config
4615d3b 
This ensures the right order. When using LD_LIBRARY_PATH, /usr/lib is
checked *first* instead of last (which is normal behaviour on non-nixos
systems).

This can lead to issues. One example is a self-compiled python, which
links to libpython2.7.so. It may have /path/to/custom/compiled/python in
its `runpath`, but it will link to /usr/lib/libpython2.7 anyway since
/usr/lib is in LD_LIBRARY_PATH. This means the wrong python version will
be executed, leading to very confusing behavior.
@timokau
Copy link
Member Author

timokau commented Apr 16, 2019

It turns out this doesn't actually work, since it changes the configuration of the static linker (ld) instead of the dynamic one (ld.so, apparently part of glibc). I'm currently trying a solution that changes the dynamic linker.

The dynamic linker usually reads /etc/ld.so.conf for its default search path. That is disabled on nixos. On my current attempt I instead make it look at /etc/nix-ld.so.conf. That avoids the risk of accidentally picking up a non-nix configuration file while still allowing buildUserFHSEnv and similar to opt in.

What do you think about that?

@Ericson2314
Copy link
Member

This is better, thanks! Could you make both files ld-nix or nix-ld so they match, and delete the old patches?

Longer term I would like to build a different dynamic loader for build FHS so the default one can truly use no config files.

@oxij
Copy link
Member

oxij commented Apr 17, 2019 via email

@Ericson2314
Copy link
Member

Yes let's upstream that. Now, I might still want to ban all sorts of ambient authority at build time, be it config files or environment variables, but it just seems inconsistent on upstream's to make one only file controlled when the other isn't.

@timokau
Copy link
Member Author

timokau commented Apr 20, 2019

Yes, that is exactly what I want! Thanks for linking that thread, very interesting. I was basically experimenting with the nix-ld.so.cache option, but the env var does seem cleaner and doesn't require ldconfig.

@timokau
Copy link
Member Author

timokau commented Apr 21, 2019

I have tested your patch and it does fix my issue :)

@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@DavHau
Copy link
Member

DavHau commented Jun 8, 2020

@oxij Your patch seems quite huge and I imagine the longterm costs of maintaining it will be higher compared to the solution proposed in the current PR. Also it introduces a new environment variable which I feel is unnecessary.
A fallback mechanism is already built into ld-linux. It has just been disabled by the nix community. So why create a totally new mechanism if we can just enable the builtin mechanism (which is what this PR does as far as i understand).

Am I not seeing something here? Please let me know.

I'd like to help and bring this topic to an end. It seems to be around since years, with several different PRs which all are stale.

@timokau I rebased this PR onto unstable and the build fails with:
/build/build/./gcc/xgcc -B/build/build/./gcc/ -xc++ -nostdinc /dev/null -S -o /dev/null -fself-test=../../gcc-9.3.0/gcc/testsuite/selftests /nix/store/p4s4jf7aq6v6z9iazll1aiqwb34aqxq9-bootstrap-tools/bin/bash: /build/build/./gcc/xgcc: No such file or directory
Any idea what could go wrong?

@DavHau DavHau mentioned this pull request Jun 8, 2020
Closed
8 tasks
@timokau
Copy link
Member Author

timokau commented Jun 8, 2020

Unfortunately not, the error doesn't make much sense to me. I don't exactly remember what I did here though, and its not currently a priority for me. So if you want to move this along, please feel free to fork the PR and link to the new one here :)

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 8, 2020
@oxij
Copy link
Member

oxij commented Jun 9, 2020 via email

@ryantm ryantm marked this pull request as draft October 23, 2020 03:07
@jbalme jbalme mentioned this pull request Jan 4, 2021
Closed
@stale
Copy link

stale bot commented Apr 26, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 26, 2021
@BBBSnowball
Copy link
Contributor

BBBSnowball commented Jan 4, 2022 •
edited

I stumbled over this issue when trying to build GrapheneOS in an fhs-userenv. The Android build system removes all environment variables for parts of the build (to make it reproducible) so any patch that uses special NIX_* environment variables won't work for this.

I tried using a different ld.so for the fhs-userenv (similar to this suggestion) but the other one is still used. I think that I would have to patch all the binaries that use the nixpkgs ld.so (or rebuild the world). It fails for other reasons, namely /etc/ld.so.cache doesn't contain the relevant alias for the library.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 4, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 10, 2022
@oxij oxij mentioned this pull request Aug 11, 2023
Open
@oxij
Copy link
Member

oxij commented Aug 11, 2023

I PRed my glibc patch at #248547.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 11, 2023
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@vcunat vcunat Awaiting requested review from vcunat

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: work-in-progress 10.rebuild-darwin: 0 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@timokau @nixos-discourse @Ericson2314 @oxij @DavHau @BBBSnowball @ryantm @samueldr @wegank @GrahamcOfBorg