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

darwin stdenv's ld -no_uuid linker flag breaks normal symbolication for debugging + profiling #178366

Closed
mstone opened this issue Jun 20, 2022 · 7 comments
Labels
0.kind: bug Something is broken 6.topic: darwin Running or building packages on Darwin

Comments

@mstone
Copy link
Contributor

mstone commented Jun 20, 2022

Describe the bug

#77632 made builds on darwin reproducible (yay!) in part by causing the darwin stdenv’s cc wrapper to set the -no_uuid linker flag so as to avoid random uuid build-ids being added to all compiled binaries and libraries.

Unfortunately, while setting this flag was effective for improving reproducibility, I think (and will hopefully soon test) that setting it also had the side-effect of breaking symbolication of binaries built with the resulting toolchain, for example, interfering with profiling them with Instruments or debugging them with lldb.

Desired resolution

Ideally, I imagine we’d find a way to set UUIDs deterministically, I suppose a little bit like how placeholder hash insertion works.

Failing that, in the shorter term, I'd love for there to be a well-documented way to remove the -no_uuid flag so that users who want UUIDs can readily obtain them or, I suppose, to add new UUIDs after the fact?

Anyway, what I've found so far is: in simple situations, overriding via an overlay can help, like so:

...
      final: prev: rec {
        bintools = prev.bintools.overrideAttrs (old: {
          postFixup = builtins.replaceStrings ["-no_uuid"] [""] old.postFixup;
        });
        cc = prev.stdenv.cc.overrideAttrs (old: {
          bintools = bintools;
        });
        stdenv = prev.overrideCC prev.stdenv cc;
        sigtool = stdenv.mkDerivation {
          inherit name;
          src = self;
          nativeBuildInputs = [ pkg-config meson ninja ];
          buildInputs = [ openssl ];
          dontStrip = true;
          separateDebugInfo = true;
          mesonBuildType = "debug";
          ninjaFlags = "-v";
        };
     ...
     }

however, I'm still struggling to make this approach work well with more complex situations, for example, using https://github.com/oxalica/rust-overlay.

(There, trying to use this simple approach runs in to difficulty because it's not clear where and how to override the darwin stdenv: trying to do so alongside a new build isn't quite enough because (at least the way I'm loading it) the rust-overlay captures the wrong stdenv; however, trying to do so via an earlier overlay runs into challenges with the staged structure of the darwin stdenv, such that the cc = prev.stdenv.cc.overrideAttrs override can fail because prev.stdenv.cc is set to the string "/dev/null" in early stages, and hence has no overrideAttrs attr itself.)

Note: you can test whether or not built binaries have uuids in a variety of ways, including

dwarfdump -u <path>

which exit with success and will print nothing when UUIDs are missing, or by using any of a variety of other DWARF tools, searching for the LC_UUID tag.

Notify maintainers

cc: @thefloweringash @LnL7

@mstone mstone added the 0.kind: bug Something is broken label Jun 20, 2022
@mstone
Copy link
Contributor Author

mstone commented Jun 21, 2022

Quick update: I managed to work around this issue again in my current project, via two additional project-specific overrides:

          # rust from oxalica/rust-overlay adds stdenv.cc to propagatedBuildInputs
          # and depsHostHostPropagated; therefore, to ensure that the correct
          # cc is present in downstream consumers, we override stdenv + both these 
          # attrs.
          rust = with final; with pkgs;
            (rust-bin.selectLatestNightlyWith (toolchain: toolchain.minimal.override {
              extensions = [ "rustfmt" ];
              targets = [ "wasm32-unknown-unknown" ];
            })).overrideAttrs (old: {
              inherit stdenv;
              propagatedBuildInputs = [ stdenv.cc ];
              depsHostHostPropagated = [ stdenv.cc ];
            });

          # crane provides a buildPackage helper that calls stdenv.mkDerivation
          # which provides a default builder that sources a "setup" file defined
          # by the stdenv itself (passed as the environment variable "stdenv" that
          # in turn defines a defaultNativeBuildInputs variable that gets added to
          # PATH via the genericBuild initialization code. Therefore, we override
          # crane's stdenv to use our modified cc-wrapper.
          cranelib = crane.lib.${final.system}.overrideScope' (final: prev: {
            inherit stdenv;
          });

but getting all of this worked out was rather painful and involved both quite detailed nix repl, nix show-derivation, and nix-tree-style investigation as well as carefully reading/grepping some long chains of wrapper shell-scripts, e.g.:

less /nix/store/pc3xjsnk3mpsfbiaqnda6di7mlgws9ly-clang-wrapper-11.1.0/bin/ld
less /nix/store/nrw3gmdqqnchxn6dk5hjz75fwm7h4y9n-cctools-binutils-darwin-wrapper-949.0.1/nix-support/add-flags.sh
less /nix/store/nrw3gmdqqnchxn6dk5hjz75fwm7h4y9n-cctools-binutils-darwin-wrapper-949.0.1/nix-support/libc-ldflags-before

and lots of experimentation.

@veprbl veprbl added the 6.topic: darwin Running or building packages on Darwin label Jun 22, 2022
@zhaofengli
Copy link
Member

zhaofengli commented Aug 25, 2022

This is a problem for us. -no_uuid makes debugging so annoying. Some more info from the Chromium/Electron teams:

To quote:

Removing UUIDs will absolutely break crash reporting. The UUID is used on the crash server to match debug information and the original code module up with the crash dump.

The UUIDs also have local value, as they prevent debuggers from incorrectly using debug info that doesn’t match. I’m heavily inclined against -no_uuid anywhere.

LC_UUID is supposed to be deterministic. See ld64 src/ld/OutputFile.cpp ld::tool::OutputFile::computeContentUUID, starting at line 3459. The UUID is based on a MD5 hash of what are supposed to be stable parts of the output. A random UUID is only used in ld::tool::OutputFile::writeOutputFile (line 3693) when the (undocumented) -random_uuid command argument is used (src/ld/Options.cpp Options::parse, line 3354).

There may be a bug in ld64 that causes the UUID to not be deterministic even when the rest of the linker output is, but you’d need to verify this. In the past, the linker parallelized certain operations and didn’t sort their outputs stably, which led to non-deterministic output, but the cases I reported were all fixed. (There may be more, though.)

From my limited testing on aarch64-darwin, the UUID seems to be stable for hello and libuv after removing -no_uuid from the flags. I didn't rebuild stdenv from scratch but just overrode bintools with my modified wrapper.

Edit: Actually scratch that, the UUIDs for libuv.1.dylib do differ from build to build. Need to look deeper into the root cause...

@zhaofengli
Copy link
Member

I took a stab at the issue and opened #188347.

@mstone
Copy link
Contributor Author

mstone commented Aug 31, 2022

@zhaofengli — thanks for working on this issue!

I haven’t had a chance to test your PR yet but it looks like a really promising approach!

@aykevl
Copy link
Contributor

aykevl commented Oct 6, 2023

This bug required an ugly workaround for the tinygo package on Darwin: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/tinygo/0003-Use-out-path-as-build-id-on-darwin.patch

If -no_uuid produces a non-reproducible build then that's a bug in the linker or the build system. It is supposed to be reproducible.

CC @muscaln

@aykevl aykevl mentioned this issue Oct 11, 2023
12 tasks
aykevl added a commit to tinygo-org/tinygo that referenced this issue Oct 15, 2023
This is to support NixOS, who have added -no_uuid to the linker.
Upstream bug report: NixOS/nixpkgs#178366
aykevl added a commit to tinygo-org/tinygo that referenced this issue Oct 15, 2023
This is to support NixOS, who have added -no_uuid to the linker.
Upstream bug report: NixOS/nixpkgs#178366
deadprogram pushed a commit to tinygo-org/tinygo that referenced this issue Oct 15, 2023
This is to support NixOS, who have added -no_uuid to the linker.
Upstream bug report: NixOS/nixpkgs#178366
crypto-smoke pushed a commit to meshnet-gophers/tinygo that referenced this issue Feb 14, 2024
This is to support NixOS, who have added -no_uuid to the linker.
Upstream bug report: NixOS/nixpkgs#178366
@reckenrode
Copy link
Contributor

#188347 is the default since 23.11. Is this issue resolved now?

@reckenrode
Copy link
Contributor

Closing since this should be fixed by #188347.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken 6.topic: darwin Running or building packages on Darwin
Projects
None yet
Development

No branches or pull requests

5 participants