NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
16.45k stars 12.95k forks source link

Build failure: rustc on staging #318674

Open reckenrode opened 3 weeks ago

reckenrode commented 3 weeks ago

Steps To Reproduce

Steps to reproduce the behavior:

  1. nix build 'github:NixOS/nixpkgs?ref=afa876d6feea138d23310cc19662b0d3364570d9#rustc' (tested only on Darwin).

Build log

https://gist.github.com/reckenrode/53c181f0c8fff37707e719734250024c

Additional context

Possibly due to https://github.com/NixOS/nixpkgs/pull/316761. https://github.com/NixOS/nixpkgs/pull/317273 may be related to the fix, but it targets machine flags not hardening flags.

Clang does provide hardeningUnsupportedFlagsByTargetPlatform, but if Rust is not invoking a wasm32-unknown-unknown target clang, I don’t think that will apply.

Notify maintainers

@risicle @alyssais

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"aarch64-darwin"`
 - host os: `Darwin 23.5.0, macOS 14.5`
 - multi-user?: `no`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.2`
 - channels(root): `"nixpkgs"`
 - nixpkgs: `/etc/nix/inputs/nixpkgs`

I actually have the Darwin sandbox disabled, so I don’t know what’s up with nix-info.


Add a :+1: reaction to issues you find important.

alyssais commented 3 weeks ago

Sounds like maybe we should not enable this by default for clang stdenv until we have a mechanism to filter Clang hardening flags by target?

risicle commented 3 weeks ago

That's weird, because cc-wrapper is full of things that assume a single specific target platform, which is why I decided we could get away with hardeningUnsupportedFlagsByTargetPlatform operating at the meta level instead of having to sniff target platform at compiler invocation time (please god no). So really, I'm amazed it works at all.

The simple thing to do here is hardeningDisable = ["zerocallusedregs"] for rustc, no?

I'm sure you can try it faster than me, already having a half-built toolchain - it'll take me ~12h to bootstrap darwin staging.

(also I really thought I built past rustc on macos x86_64 before I merged this...)

reckenrode commented 3 weeks ago

Setting hardeningDisable =["zerocallusedregs"] leads to a different error:

  cargo:warning=clang: warning: argument unused during compilation: '-mmacos-version-min=10.12' [-Wunused-command-line-argument]
  cargo:warning=clang: error: unsupported option '-arch' for target 'wasm32-unknown-unknown'

Is it possible to set CC_wasm32_unknown_unknown to a clang with the right flags? I tried setting it to clang-unwrapped, but that didn’t work due to not finding libc headers.

alyssais commented 2 weeks ago

That's weird, because cc-wrapper is full of things that assume a single specific target platform, which is why I decided we could get away with hardeningUnsupportedFlagsByTargetPlatform operating at the meta level instead of having to sniff target platform at compiler invocation time (please god no). So really, I'm amazed it works at all.

Most of them are just irrelevant for weird targets like bpf or wasm32-unknown-unknown, and get ignored. It's only recently that Clang has started producing errors rather than either warning or ignoring them. I think it's a good experience if we're able to support at least those targets, because it's not like there's a bpf stdenv.

risicle commented 2 weeks ago

My preference would probably be to guide people towards using an unwrapped compiler in this case then - it's not clear to me what value wrapping a wasm compiler adds, and it would be nicer to avoid the complexity and not start giving people any expectations of a wrapped compiler working as a multi-target compiler. In fact I'd probably advocate the wrapper at least emitting a warning if it detects the --target flag being used. If not throw an error entirely.

I'm not sure what's going on with this new rustc error, but I can't see it being related to the zerocallusedregs addition.

risicle commented 1 week ago

Guessing the file we might need to hack for this could be https://github.com/rust-lang/rust/blob/05468cf1241ae661c84e43d6a41d7a0ba317eebf/src/bootstrap/src/utils/cc_detect.rs#L163

risicle commented 1 week ago

Indeed if we patch that with

--- a/src/bootstrap/src/utils/cc_detect.rs        2024-06-23 14:29:18.395349230 +0100
+++ b/src/bootstrap/src/utils/cc_detect.rs    2024-06-23 14:35:42.303268205 +0100
@@ -174,6 +174,14 @@
             build.config.android_ndk.as_ref().map(|ndk| ndk_compiler(compiler, &target.triple, ndk))
         }

+        t if t.starts_with("wasm") => {
+            let root = PathBuf::from("@clangUnwrapped@/bin");
+            match compiler {
+                Language::C => Some(root.join("clang")),
+                Language::CPlusPlus => Some(root.join("clang++")),
+            }
+        }
+
         // The default gcc version from OpenBSD may be too old, try using egcc,
         // which is a gcc version from ports, if this is the case.
         t if t.contains("openbsd") => {

it will use the unwrapped clang. But now it can't find any of its headers :melting_face:

  cargo:warning=In file included from /private/tmp/nix-build-rustc-1.78.0.drv-1/rustc-1.78.0-src/src/llvm-project/compiler-rt/lib/builtins/absvdi2.c:13:
  cargo:warning=/private/tmp/nix-build-rustc-1.78.0.drv-1/rustc-1.78.0-src/src/llvm-project/compiler-rt/lib/builtins/int_lib.h:92:10: fatal error: 'float.h' file not found
  cargo:warning=   92 | #include <float.h>
  cargo:warning=      |          ^~~~~~~~~
  cargo:warning=1 error generated.
alyssais commented 1 week ago

I think the best solution here might be for the clang wrapper to exec unwrapped clang if it sees -target. That way, we still have a multi-target compiler, which is good, because there are packages that rely on that, but we also don't end up using the arguments injected by the wrapper when building for a custom target (which is probably what those packages expect anyway.)

risicle commented 1 week ago

I think that will just put us in the same situation as with the patch - falling back to the unwrapped clang will give you a clang that can't find its stdlib headers. Also not sure how much I like the idea of making --target act like a cliff edge, drastically changing behaviour.

The quickest thing to do here is to add hardeningDisable = [ "zercallusedregs" ] to rustc. That will take us back to the situation before this breakage.

The "most correct" thing is probably to do something like point the build process at pkgsCross.wasi32.stdenv.cc for its wasm compiler, allowing any logic around choice of flags to continue to operate in nix through hardeningUnsupportedFlagsByTargetPlatform. But that adds a lot of new build-time-dependencies.

What we really want I reckon is to re-wrap our existing clang-unwrapped compiler with a wasm platform, requiring fewest additional builds.

What I can't figure out though is why this isn't a problem on linux/gcc.

alyssais commented 1 week ago

The "most correct" thing is probably to do something like point the build process at pkgsCross.wasi32.stdenv.cc for its wasm compiler

It's generally @Ericson2314's and my intention that pkgsCross shouldn't really be used in this way, within a package. It's intended to be a convenience for using Nixpkgs, not use in packages, where it causes performance and layering problems.

reckenrode commented 1 week ago

The "most correct" thing is probably to do something like point the build process at pkgsCross.wasi32.stdenv.cc for its wasm compiler

It's generally @Ericson2314's and my intention that pkgsCross shouldn't really be used in this way, within a package. It's intended to be a convenience for using Nixpkgs, not use in packages, where it causes performance and layering problems.

What would be the intended way to invoke a cross-compiler in a build when its platform isn’t the default target?

alyssais commented 1 week ago

For the common cases — wasm or bpf — there's no libc to build, so we'd just use normal clang or rustc or whatever (with a fixed wrapper), and get the benefits of a multi-arch compiler.

I don't think there are many packages that where part but not all of it needs to be compiled for e.g. aarch64-linux on x86_64-linux. Is that sort of situation something you're concerned about?

reckenrode commented 1 week ago

I don't think there are many packages that where part but not all of it needs to be compiled for e.g. aarch64-linux on x86_64-linux. Is that sort of situation something you're concerned about?

Wine uses MinGW to build PE DLLs. MinGW is GCC, so it has to be built as a cross-compiler.

It’s used via pkgsCross, which causes some problems on Darwin, but it has been worked around in the Wine derivation.

alyssais commented 1 week ago

@Ericson2314 how do you envisage that working?

risicle commented 1 week ago

pkgsCross shouldn't really be used in this way, within a package

Agree with this, it feels dirty.

I think I have something else that will work, but will try it before I elaborate...

Ericson2314 commented 1 week ago

I think for Wine, for now, it would be OK to have a pkgsMingw just as we have a pkgsi686. It is ugly, but is less bad than saying "fuck it, pkgsCross is OK to use". I don't love this at all, of course :)

risicle commented 1 week ago

Hold your stomachs

--- a/pkgs/development/compilers/rust/rustc.nix
+++ b/pkgs/development/compilers/rust/rustc.nix
@@ -18,12 +18,29 @@
 # This only builds std for target and reuses the rustc from build.
 , fastCross
 , lndir
+, substituteAll
 , makeWrapper
 }:

 let
   inherit (lib) optionals optional optionalString concatStringsSep;
   inherit (darwin.apple_sdk.frameworks) Security;
+  clangRewrapped = stdenv.cc.override (old: rec {
+    stdenvNoCC = old.stdenvNoCC.override {
+      targetPlatform = old.stdenvNoCC.targetPlatform // {
+        parsed = {
+          cpu.name = "wasm32";
+          vendor.name = "unknown";
+          kernel.name = "unknown";
+          abi.name = "unknown";
+        };
+        config = "wasm32-unknown-unknown";
+      };
+    };
+    bintools = old.bintools.override {
+      inherit stdenvNoCC;
+    };
+  });
 in stdenv.mkDerivation (finalAttrs: {
   pname = "${targetPackages.stdenv.cc.targetPrefix}rustc";
   inherit version;
@@ -194,7 +211,12 @@ in stdenv.mkDerivation (finalAttrs: {
   # the rust build system complains that nix alters the checksums
   dontFixLibtool = true;

-  inherit patches;
+  patches = patches ++ [
+    (substituteAll {
+      src = ./wasm-clang-unwrapped.patch;
+      clangUnwrapped = clangRewrapped;
+    })
+  ];

   postPatch = ''
     patchShebangs src/etc
--- /dev/null
+++ b/pkgs/development/compilers/rust/wasm-clang-unwrapped.patch
@@ -0,0 +1,19 @@
+--- a/src/bootstrap/src/utils/cc_detect.rs        2024-06-23 14:29:18.395349230 +0100
++++ b/src/bootstrap/src/utils/cc_detect.rs    2024-06-23 14:35:42.303268205 +0100
+@@ -174,6 +174,14 @@
+             build.config.android_ndk.as_ref().map(|ndk| ndk_compiler(compiler, &target.triple, ndk))
+         }
+
++        t if t.starts_with("wasm") => {
++            let root = PathBuf::from("@clangUnwrapped@/bin");
++            match compiler {
++                Language::C => Some(root.join("wasm32-unknown-unknown-clang")),
++                Language::CPlusPlus => Some(root.join("wasm32-unknown-unknown-clang++")),
++            }
++        }
++
+         // The default gcc version from OpenBSD may be too old, try using egcc,
+         // which is a gcc version from ports, if this is the case.
+         t if t.contains("openbsd") => {
+
+

WFM macos 12 x86_64.

Obvs would need something slightly different for non-clang stdenvs - or maybe not because they don't appear to be broken and nobody seems to know why.

risicle commented 2 days ago

The deeper I dig the weirder this gets:

On x86_64 linux, after the build:

$ file build/x86_64-unknown-linux-gnu/stage2-std/wasm32-unknown-unknown/release/build/compiler_builtins-ee0249720c2fbd55/out/f671b77bc351d22a-absvdi2.o
ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped

(same for *.o in that dir)

x86_64 darwin after the build (if zerocallusedregs temporarily disabled to avoid error):

$ file build/x86_64-apple-darwin/stage2-std/wasm32-unknown-unknown/release/build/compiler_builtins-c6397cafd08482ba/out/424303ac06eede49-absvdi2.o
WebAssembly (wasm) binary module version 0x1 (MVP)

(again, same for *.o in that dir)

So.. on linux is it just blindly compiling these compiler builtins for the host platform? Are we actually assured that the end result works for wasm32?

(all on staging-next)

K900 commented 1 day ago

Same issue on nvcc, by the way.

risicle commented 1 day ago

Suggest same stopgap.

K900 commented 1 day ago

Done in #323849

risicle commented 1 day ago

Feels like this issue is becoming a general "make it easy/possible to re-wrap a compiler for a different target" point of reference.

Incidentally @K900 how does the ffmpeg situation react to being given an un-wrapped clang for cuda-llvm mode?