Skip to content

mold: wrap with ld-wrapper#172452

Closed
tobim wants to merge 3 commits intoNixOS:masterfrom
tobim:mold-wrapped
Closed

mold: wrap with ld-wrapper#172452
tobim wants to merge 3 commits intoNixOS:masterfrom
tobim:mold-wrapped

Conversation

@tobim
Copy link
Contributor

@tobim tobim commented May 11, 2022

Description of changes

Wrapping mold so it can find libraries from the nix build environment.

See #24744 for additional context. The situation with lld is very similar, but mold was easier to fix.

Demo:

$ printf '#include <iostream>\nint main(){std::cout << "Woo!\\\n";}\n' \
  | c++ -B$(nix build --print-out-paths .#mold)/bin -x c++ - \
  && readelf -p .comment a.out \
  && ./a.out

String dump of section '.comment':
  [     0]  GCC: (GNU) 10.3.0
  [    12]  mold 1.2.1 (compatible with GNU ld)
  [    37]  GCC: (GNU) 8.3.0

Woo!
  • check in a nix-shell with additional dependencies

  • verify that this doesn't involuntarily break linking when used with rust

  • Built on platform(s)

    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)

  • Tested, as applicable:

  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage

  • Tested basic functionality of all binary files (usually in ./result/bin/)

  • 22.05 Release Notes (or backporting 21.11 Release notes)

    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from nitsky May 11, 2022 06:13
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels May 11, 2022
@cmm
Copy link
Member

cmm commented Jun 5, 2022

would be totally neat if this got merged

@a-kenji
Copy link
Contributor

a-kenji commented Jul 17, 2022

I tested the following under x86_64-linux:

  • check in a nix-shell with additional dependencies
  • verify that this doesn't involuntarily break linking when used with rust: I can't verify that, but I tested it with various projects that link with shared libraries (sqlite3, openssl, gtk, adwaita) and it worked flawlessly in my tests.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1100

Comment on lines +43 to +45
mv $out/bin/mold $out/bin/.mold

export prog=$out/bin/.mold
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use wrapProgram or makeWrapper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't understand how those tools would help solve the problem in a better way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would help here.

tobim and others added 2 commits August 16, 2022 23:22
@sternenseemann
Copy link
Member

Linkers in nixpkgs are left unwrapped intentionally – our wrapper scripts are quite invasive and we want users to be able to deal with more low-level situations manually as well – outside of stdenv the wrapper scripts are useless anyways.

Therefore, you work should be split into mold and mold-unwrapped or similar. Additionally, the wrapped version should probably utilise wrapBintools (not sure where to pull the other bintools from best, maybe llvm?), so it can be used in a custom stdenv.

@RaitoBezarius
Copy link
Member

Can I help to get this merged?

@tobim
Copy link
Contributor Author

tobim commented Dec 1, 2022

I don't have any capacity to work on this at the moment. Fell free to open a new PR and work in the suggestions from @sternenseemann.

@lf-
Copy link
Member

lf- commented Dec 2, 2022

@RaitoBezarius I've poked at using wrapBintools for this for a proof of concept for work. One big issue with it is that it's less than helpful on macOS without some patching. Further, it's hard to test since changing the original file will be a world rebuild. We should possibly introduce a new parameter to it or use a postInstall hook or something to work around this issue.

{ mold
, stdenv
, libcCross
, callPackage
, darwin
}:
let wrapBintoolsWith2 =
  { bintools
  , libc ? if stdenv.targetPlatform != stdenv.hostPlatform then libcCross else stdenv.cc.libc
  , ...
  } @ extraArgs:
  callPackage ./pkgs/build-support/bintools-wrapper2 (
    let self = {
      nativeTools = stdenv.targetPlatform == stdenv.hostPlatform && stdenv.cc.nativeTools or false;
      nativeLibc = stdenv.targetPlatform == stdenv.hostPlatform && stdenv.cc.nativeLibc or false;
      nativePrefix = stdenv.cc.nativePrefix or "";

      noLibc = (self.libc == null);

      inherit bintools libc;
      inherit (darwin) postLinkSignHook signingUtils;
    } // extraArgs; in self
  );
in
wrapBintoolsWith2 {
  bintools = mold;
}

I've patched binutils-wrapper2/default.nix like so:

      # added ld.mold ld64.mold
      for variant in ld.gold ld.bfd ld.lld ld.mold ld64.mold; do
        local underlying=$ldPath/${targetPrefix}$variant
        [[ -e "$underlying" ]] || continue
        wrap ${targetPrefix}$variant ${./ld-wrapper.sh} $underlying
      done

@lf-
Copy link
Member

lf- commented Dec 2, 2022

Here is the updated self contained version of the hack that people can vendor (don't use path if you're in nixpkgs). But the problem I have now is that there is pervasive assumption throughout the scripts that ld is the platform-specific linker; that is, ${mold}/bin/ld is ld64.mold.

 » clang -fuse-ld=mold test.c                    
mold: fatal: unknown command line option: -z
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

The wrapper injected that flag because it was supported on the ELF targeting one, which is wrong. Cause:
https://github.com/nixos/nixpkgs/blob/549e08c8e8a0e79952227c3300702d3d0fbb71a8/pkgs/build-support/bintools-wrapper/default.nix#L272-L281

# Kinda evil wrapper for mold to get all the nix flags but whatever, it works
{ mold
, wrapBintoolsWith
, path
}:
(wrapBintoolsWith {
  bintools = mold;
}).overrideAttrs (old: {
  # for baffling reasons postInstall is not getting run. no idea, buddy.
  installPhase = old.installPhase + ''
      for variant in ld.mold ld64.mold; do
        # XXX: probably broken with cross compilation, w/e
        local underlying=$ldPath/$variant
        [[ -e "$underlying" ]] || continue
        wrap $variant ${path}/pkgs/build-support/bintools-wrapper/ld-wrapper.sh $underlying
      done
  '';
})

@lf-
Copy link
Member

lf- commented Dec 7, 2022

fyi: on Linux you don't need any of that override stuff, and I think that Linux-only is an ok starting point for now!

@9999years
Copy link
Contributor

Anything blocking this?

@Artturin
Copy link
Member

adding mold to the variant list #216383

@lf- postInstall isn't being run because you're overriding installPhase and not using runHook postInstall

@Artturin
Copy link
Member

i think #216383 replaces the need for this pr

@Artturin Artturin closed this Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.