From b41933a1be1ea0f9bddad4ef5eed6d0ed61a2510 Mon Sep 17 00:00:00 2001 From: Rahul Butani Date: Sat, 25 Feb 2023 12:02:24 -0600 Subject: [PATCH 1/3] bintools-wrapper: specify SHA1 as the `build-id` hash style explicitly NixOS/nixpkgs#146275 has more discussion on this; the abridged version is that `lld` defaults to using `--build-id=fast` while GNU `ld` defaults to `--build-id=sha1`. These differ in length and so `separate-debug-info.sh`, as of this writing, errors on `lld`'s shorter `--build-id=fast`-generated hashes. `lld` offers the following `build-id` styles: - UUID (random; fast but bad for reproducibility) - fast (xxhash; fast but shorter hashes) - user provided hexstring - SHA1 - MD5 GNU `ld` supports the latter three options, `mold` supports all of these plus SHA256. UUID is out because it's not reproducible, fast isn't supported by GNU `ld` Using a nix provided (sourced from the output base hash) hash as the `build-id` seems tempting but would require a little extra work (we have to include some characteristic of the binary being hashed so that binaries within a derivation still have unique hashes; it seems easy to get this wrong; i.e. a path based approach would make two otherwise identical binaries that reside at different paths have different `build-id` hashes) That leaves SHA1 and MD5 and GNU `ld` already defaults to the former. This commit adds `$NIX_BUILD_ID_STYLE` as an escape hatch, in case any packages have strong opinions about which hash to use. ---- Note that if/when NixOS/nixpkgs#146275 goes through, this change can be reverted if linker speed is a priority. --- pkgs/build-support/bintools-wrapper/ld-wrapper.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkgs/build-support/bintools-wrapper/ld-wrapper.sh b/pkgs/build-support/bintools-wrapper/ld-wrapper.sh index cb1160f9bbf7..0d832bc4a67a 100644 --- a/pkgs/build-support/bintools-wrapper/ld-wrapper.sh +++ b/pkgs/build-support/bintools-wrapper/ld-wrapper.sh @@ -232,8 +232,11 @@ fi # Only add --build-id if this is a final link. FIXME: should build gcc # with --enable-linker-build-id instead? +# +# Note: `lld` interprets `--build-id` to mean `--build-id=fast`; GNU ld defaults +# to SHA1. if [ "$NIX_SET_BUILD_ID_@suffixSalt@" = 1 ] && ! (( "$relocatable" )); then - extraAfter+=(--build-id) + extraAfter+=(--build-id="${NIX_BUILD_ID_STYLE:-sha1}") fi From d364ee8d13c8889aa7a88c5ed87255915af8e45e Mon Sep 17 00:00:00 2001 From: Rahul Butani Date: Sat, 25 Feb 2023 12:18:00 -0600 Subject: [PATCH 2/3] mkDerivation: do not disable `separateDebugInfo` on LLVM stdenvs This was disabled here: https://github.com/NixOS/nixpkgs/commit/b86e62d30d4635ef3294d9c1c308f9c8b0061045#diff-282a02cc3871874f16401347d8fadc90d59d7ab11f6a99eaa5173c3867e1a160 h/t to @teh: https://github.com/NixOS/nixpkgs/commit/b86e62d30d4635ef3294d9c1c308f9c8b0061045#commitcomment-77916294 for pointing out that the failure that @matthewbauer was seeing was caused by the `separate-debug-info.sh` `build-id` length requirement that #146275 will relax `lld` has had `--build-id` support dating back to LLVM4: https://reviews.llvm.org/D18091 This predates every `llvmPackages_` version currently in nixpkgs (and certainly every version actually still used in `useLLVM` stdenvs) so with the previous commit (asking `ld` for sufficiently long SHA1 hashes) I think we can safely enable `separateDebugInfo` when using LLVM bintools. --- pkgs/stdenv/generic/make-derivation.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/stdenv/generic/make-derivation.nix b/pkgs/stdenv/generic/make-derivation.nix index d9672f62e3d3..46b22f337303 100644 --- a/pkgs/stdenv/generic/make-derivation.nix +++ b/pkgs/stdenv/generic/make-derivation.nix @@ -171,7 +171,7 @@ let doCheck' = doCheck && stdenv.buildPlatform.canExecute stdenv.hostPlatform; doInstallCheck' = doInstallCheck && stdenv.buildPlatform.canExecute stdenv.hostPlatform; - separateDebugInfo' = separateDebugInfo && stdenv.hostPlatform.isLinux && !(stdenv.hostPlatform.useLLVM or false); + separateDebugInfo' = separateDebugInfo && stdenv.hostPlatform.isLinux; outputs' = outputs ++ lib.optional separateDebugInfo' "debug"; # Turn a derivation into its outPath without a string context attached. From 45e58731b8ad48d4ffc5c0086e98f4ab942efce1 Mon Sep 17 00:00:00 2001 From: Rahul Butani Date: Sat, 25 Feb 2023 12:26:33 -0600 Subject: [PATCH 3/3] firefox: remove the `separate-debug-info` workaround With the previous two commits, lld/LLVM stdenvs should now produce `build-id`s that satisify `separate-debug-info.sh` --- .../networking/browsers/firefox/common.nix | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/pkgs/applications/networking/browsers/firefox/common.nix b/pkgs/applications/networking/browsers/firefox/common.nix index 948b32172489..b1f79cd4627e 100644 --- a/pkgs/applications/networking/browsers/firefox/common.nix +++ b/pkgs/applications/networking/browsers/firefox/common.nix @@ -500,41 +500,6 @@ buildStdenv.mkDerivation ({ gappsWrapperArgs+=(--argv0 "$out/bin/.${binaryName}-wrapped") ''; - # Workaround: The separateDebugInfo hook skips artifacts whose build ID's length is not 40. - # But we got 16-length build ID here. The function body is mainly copied from pkgs/build-support/setup-hooks/separate-debug-info.sh - # Remove it when https://github.com/NixOS/nixpkgs/pull/146275 is merged. - preFixup = lib.optionalString enableDebugSymbols '' - _separateDebugInfo() { - [ -e "$prefix" ] || return 0 - - local dst="''${debug:-$out}" - if [ "$prefix" = "$dst" ]; then return 0; fi - - dst="$dst/lib/debug/.build-id" - - # Find executables and dynamic libraries. - local i - while IFS= read -r -d $'\0' i; do - if ! isELF "$i"; then continue; fi - - # Extract the Build ID. FIXME: there's probably a cleaner way. - local id="$($READELF -n "$i" | sed 's/.*Build ID: \([0-9a-f]*\).*/\1/; t; d')" - if [[ -z "$id" ]]; then - echo "could not find build ID of $i, skipping" >&2 - continue - fi - - # Extract the debug info. - echo "separating debug info from $i (build ID $id)" - mkdir -p "$dst/''${id:0:2}" - $OBJCOPY --only-keep-debug "$i" "$dst/''${id:0:2}/''${id:2}.debug" - - # Also a create a symlink .debug. - ln -sfn ".build-id/''${id:0:2}/''${id:2}.debug" "$dst/../$(basename "$i")" - done < <(find "$prefix" -type f -print0) - } - ''; - postFixup = lib.optionalString crashreporterSupport '' patchelf --add-rpath "${lib.makeLibraryPath [ curl ]}" $out/lib/${binaryName}/crashreporter '';