rust-lang / cc-rs

Rust library for build scripts to compile C/C++ code into a Rust library
https://docs.rs/cc
Apache License 2.0
1.8k stars 433 forks source link

Compilation failure on LLVM 19 with `*-gnullvm` #1167

Closed kleisauke closed 4 weeks ago

kleisauke commented 1 month ago

The logic at: https://github.com/rust-lang/cc-rs/blob/dcd8ed3e96d56e3e7f0db93ef5bb49c5c8ba2594/src/lib.rs#L1930-L1935 (introduced in PR https://github.com/rust-lang/cc-rs/pull/825)

Breaks with LLVM 19 (https://github.com/rust-lang/rust/pull/127513) when targeting *-pc-windows-gnullvm due to PR https://github.com/llvm/llvm-project/pull/78655.

Details ``` The following warnings were emitted during compilation: warning: compiler_builtins@0.1.109: clang: error: version 'llvm' in target triple 'x86_64-pc-windows-gnullvm' is invalid error: failed to run custom build command for `compiler_builtins v0.1.109` Caused by: [...] --- stderr error occurred: Command "x86_64-w64-mingw32-clang" "-Oz" "--target=x86_64-pc-windows-gnullvm" "-ffunction-sections" "-fdata-sections" "-m64" "--target=x86_64-pc-windows-gnullvm" "-Os" "-g" "-gcodeview" "-fdata-sections" "-ffunction-sections" "-fno-builtin" "-fvisibility=hidden" "-ffreestanding" "-DVISIBILITY_HIDDEN" "-o" "/var/tmp/tmp-librsvg-x86_64-w64-mingw32/librsvg-2.58.92.build_/target/x86_64-pc-windows-gnullvm/release/build/compiler_builtins-f90e4ff268ddda14/out/978315636d0ac771-absvdi2.o" "-c" "/data/mxe/usr/x86_64-pc-linux-gnu/lib/rustlib/src/rust/src/llvm-project/compiler-rt/lib/builtins/absvdi2.c" with args x86_64-w64-mingw32-clang did not execute successfully (status code exit status: 1). ```

/cc @mati865 FYI.

NobodyXu commented 1 month ago

cc @ChrisDenton Do we want to remove the llvm suffix here?

kleisauke commented 1 month ago

Hmm, it looks like --target=x86_64-pc-windows-gnullvm was added twice to the clang invocation. This patch seems to fix it for me:

Details ```diff --- a/src/lib.rs +++ b/src/lib.rs @@ -1939,7 +1939,10 @@ impl Build { cmd.push_opt_unless_duplicate(format!("-O{}", opt_level).into()); } - if cmd.is_like_clang() && target.contains("windows") { + if cmd.is_like_clang() + && target.contains("windows") + && !target.ends_with("-gnullvm") + { // Disambiguate mingw and msvc on Windows. Problem is that // depending on the origin clang can default to a mismatchig // run-time. @@ -2190,6 +2193,10 @@ impl Build { } cmd.push_cc_arg(format!("--target={}", target).into()); + } else if target.ends_with("-gnullvm") { + cmd.push_cc_arg( + format!("--target={}", target.strip_suffix("llvm").unwrap()).into(), + ); } else { cmd.push_cc_arg(format!("--target={}", target).into()); } @@ -3467,6 +3474,7 @@ impl Build { "hexagon-unknown-linux-musl" => Some("hexagon-linux-musl"), "i586-unknown-linux-musl" => Some("musl"), "i686-pc-windows-gnu" => Some("i686-w64-mingw32"), + "i686-pc-windows-gnullvm" => Some("i686-w64-mingw32"), "i686-uwp-windows-gnu" => Some("i686-w64-mingw32"), "i686-unknown-linux-gnu" => self.find_working_gnu_prefix(&[ "i686-linux-gnu", ``` (the `i686-pc-windows-gnullvm` change should probably be split into a separate commit)
NobodyXu commented 1 month ago

Thanks, that looks good, can you open a PR?

I would review it and also ask @ChrisDenton to review it, since they are more familiar with windows than me.

mati865 commented 1 month ago

I think using Rust triple is unfortunate. Many other triples won't match LLVM, like Win7 triple if we stick to Windows.

That said I don't know if it's possible to obtain LLVM triple from rustc so adding workarounds for all the triples might be the only way.

ChrisDenton commented 1 month ago

On nightly you can use rustc --print target-spec-json but that's not stable yet. A stable way I can think of is to perhaps infer it from the cfg without looking at the Rust target string at all. E.g. the LLVM target is x86_64-pc-windows-msvc if target_arch="x86_64" and target_os="windows" and target_env="msvc".

I guess a test could compare the cc-rs inferred LLVM targets against the nightly target-spec-json to make sure they match. But that's slightly tricker as targets can appear and disappear so there might not be an exact match for any given target.

NobodyXu commented 1 month ago

cc @kleisauke @mati865 @ChrisDenton if it is available in nightly, then we can pre-generate it like we did for riscv:

https://github.com/rust-lang/cc-rs/blob/c2fd8ecd1f7757c3195df028d7e390db1167dcb6/dev-tools/gen-target-info/src/main.rs#L36

gen-target-info is run on CI every week and create a PR if something changes, and it also can be run locally.

The generated info would then be committed and included in crate publish.

mati865 commented 1 month ago

Yeah, it's been available in night for over a year. I don't know how gen-target-info works though.

NobodyXu commented 1 month ago

Yeah, it's been available in night for over a year. I don't know how gen-target-info works though.

Basically, it creates a rust source file in src, which is checked into git and included in cargo-publish.

It's usually run in CI per week, or on the developer machine.

If the generated source file differs, then you need to commit and open a PR.

mati865 commented 1 month ago

Sounds good but I won't be able to look into it probably until the weekend.

mati865 commented 4 weeks ago

@kleisauke how can I test this issue easily? I think https://github.com/rust-lang/cc-rs/pull/1176 should address this issue, but I'd like to test it first. Cross-compiling librsvg from Linux fails on missing cairo pkgconf files. Is it possible for you to try it?

kleisauke commented 4 weeks ago

@mati865 Thanks for opening PR #1176! AFAIK, you can reproduce this error when building a crate with -Zbuild-std, as that tries to build compiler-builtins.

Testing this with librsvg is a bit difficult because you have to cross-compile its dependencies as well. Also, you need to disable any raw-dylib usage internally in Rust via these (now removed) patches: compiler/rustc_codegen_cranelift/patches/0030-stdlib-Revert-use-raw-dylib-for-Windows-futex-APIs.patch compiler/rustc_codegen_cranelift/patches/0029-stdlib-rawdylib-processprng.patch

Otherwise, you'll encounter these errors: https://github.com/libvips/build-win64-mxe/blob/896ccdbf57ff3fcbfa4f535e7cdc81d72d30c836/build/plugins/llvm-mingw/rust.mk#L35-L41 (I think MSVC also has a similar error, see e.g. issue https://github.com/mesonbuild/meson/issues/13236)

Note that in the starting post --target=x86_64-pc-windows-gnullvm was added twice to the clang invocation, due to: https://github.com/rust-lang/cc-rs/blob/5863b315cc9db675761bb00d2cbab5bc2b23a3e2/src/lib.rs#L1941-L1946 (perhaps this whole block can be removed?)

mati865 commented 4 weeks ago

Oh, this complicates things as I won't be able to easily test the fix with -Zbuild-std.

Import errors were discussed a bit in https://github.com/msys2/MINGW-packages/issues/21017, it affects all windows targets other than windows-gnu.

Note that in the starting post --target=x86_64-pc-windows-gnullvm was added twice to the clang invocation

I doubt adding the argument twice causes any problems; the second occurrence should override the first one. It can be probably removed, but I'm not planning to spend time on testing it.

kleisauke commented 4 weeks ago

I can confirm PR #1176 fixes this, tested with commit https://github.com/libvips/build-win64-mxe/commit/5b050d1b4d8b059db149e6743c3ac7ebfbb7f3db.

Import errors were discussed a bit in https://github.com/msys2/MINGW-packages/issues/21017, it affects all windows targets other than windows-gnu.

Interesting, thanks for linking and investigating that issue.

mati865 commented 4 weeks ago

Great, thanks!