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.

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

binutils: fix an error handling bug in the Gold linker #101666

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

Conversation

@utsl42
Copy link
Contributor

@utsl42 utsl42 commented Oct 25, 2020

Motivation for this change

Fixes #101490, a bug found in gold error handling, that causes gold to fail to link at all if built for musl, and the filesystem used doesn't support fallocate. (ZFS, for example.)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@utsl42
Copy link
Contributor Author

@utsl42 utsl42 commented Oct 26, 2020

It may also be reasonable to simply go through and disable gold on musl. It's alleged that it's poorly maintained upstream, with Google's interest shifted to lld, and there do seem to be other issues with it. In combination with #101606, this fixes building multiple versions of llvm, but I was also able to fix their builds by disabling the gold plugin. This approach would not require the same mass rebuilding that fixing this bug in binutils does.

Testing this, I found that ld.gold and ld.bfd differ in handling -static-pie. ld.gold appears to take -static to override -pie, while ld.bfd does not, requiring -static-pie. ld.gold throws an error for -static-pie, so the combination of these two changes could create a problem for pkgsStatic.

@FRidh FRidh changed the base branch from master to staging Nov 18, 2020
@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 23, 2020

@utsl42 Can you resolve the merge conflict?

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Dec 28, 2020

Please do not merge the target branch into PRs and rebase and resolve the merge conflict.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Dec 28, 2020

@ofborg eval

@Gaelan
Copy link
Contributor

@Gaelan Gaelan commented Dec 28, 2020

Have we tried upstreaming this fix? If so, it would probably be good to link to that effort in a comment, so we know when the patch can be removed.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 11, 2021

@utsl42 please fix the merge conflict.

@stale
Copy link

@stale stale bot commented Jul 21, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment