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

Compilation failure on LLVM 19 with *-gnullvm #1167

Closed
kleisauke opened this issue Jul 29, 2024 · 14 comments · Fixed by #1176
Closed

Compilation failure on LLVM 19 with *-gnullvm #1167

kleisauke opened this issue Jul 29, 2024 · 14 comments · Fixed by #1176

Comments

@kleisauke
Copy link

The logic at:

cc-rs/src/lib.rs

Lines 1930 to 1935 in dcd8ed3

if cmd.is_like_clang() && target.contains("windows") {
// Disambiguate mingw and msvc on Windows. Problem is that
// depending on the origin clang can default to a mismatchig
// run-time.
cmd.push_cc_arg(format!("--target={}", target).into());
}

(introduced in PR #825)

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

Details
The following warnings were emitted during compilation:

warning: [email protected]: 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
Copy link
Collaborator

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

@kleisauke
Copy link
Author

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
--- 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
Copy link
Collaborator

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
Copy link
Contributor

mati865 commented Jul 29, 2024

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
Copy link
Member

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
Copy link
Collaborator

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

generate_riscv_arch_mapping(&mut f, &target_specs);

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
Copy link
Contributor

mati865 commented Jul 30, 2024

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

@NobodyXu
Copy link
Collaborator

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
Copy link
Contributor

mati865 commented Jul 30, 2024

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

@mati865
Copy link
Contributor

mati865 commented Aug 10, 2024

@kleisauke how can I test this issue easily?
I think #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
Copy link
Author

kleisauke commented Aug 10, 2024

@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 mesonbuild/meson#13236)

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

cc-rs/src/lib.rs

Lines 1941 to 1946 in 5863b31

if cmd.is_like_clang() && target.contains("windows") {
// Disambiguate mingw and msvc on Windows. Problem is that
// depending on the origin clang can default to a mismatchig
// run-time.
cmd.push_cc_arg(format!("--target={}", target).into());
}

(perhaps this whole block can be removed?)

@mati865
Copy link
Contributor

mati865 commented Aug 10, 2024

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 msys2/MINGW-packages#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
Copy link
Author

I can confirm PR #1176 fixes this, tested with commit libvips/build-win64-mxe@5b050d1.

Import errors were discussed a bit in msys2/MINGW-packages#21017, it affects all windows targets other than windows-gnu.

Interesting, thanks for linking and investigating that issue.

@mati865
Copy link
Contributor

mati865 commented Aug 10, 2024

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants