corrosion-rs / corrosion

Marrying Rust and CMake - Easy Rust and C/C++ Integration!
https://corrosion-rs.github.io/corrosion/
MIT License
1.11k stars 106 forks source link

Fix gnullvm and test gnullvm with msys2 in CI #553

Closed jschwe closed 2 months ago

jschwe commented 3 months ago

List of problems / fixes related to gnullvm:

mati865 commented 3 months ago

There is no LLVM toolchain preinstalled in GHA images. You can use https://github.com/mstorsjo/llvm-mingw or CLANG* environments from MSYS2: https://github.com/msys2/setup-msys2

PieroV commented 3 months ago
  • [ ] How to invoke the linker for Rust shared libraries?

    • [ ] If corrosion uses the c/c++ linker as the linker driver like we normally do, then -lunwind is not found. Is the default MinGW in CI suitable?
    • [ ] If we don't tell Rust which linker to use, it looks for x86_64-w64-mingw32-clang, which is not in PATH, and also not the same as our C/C++ target triple. Again, raises questions on the configuration of our test case.

In my tests I added the toolchain to $PATH, but I guess the right thing would be having Cmake telling Cargo/Rust to use that? Apart from that, that linker should be correct.

Anyway, I can any needed tests. However, it'll take me a bit longer if I need to build a patched cargo :sweat_smile:.

I'm using Corrosion for personal projects, but I'm using the artifacts from Tor Browser's build system (I'm a Tor Browser developer during daytime).

jschwe commented 3 months ago

There is no LLVM toolchain preinstalled in GHA images

When looking at the action logs it seems that CMake does detect clang installed under C:/Program Files/LLVM/bin/clang.exe

-- The C compiler identification is Clang 18.1.8
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/LLVM/bin/clang.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done

Is there any difference between that LLVM and what llvm-mingw / msys2-llvm provide? The official runner software list lists Microsoft.VisualStudio.Component.VC.Llvm.Clang. So I'm wondering what the difference between microsoft LLVM and other LLVM toolchains is (and if I can detect that from CMake to decide on the toolchain)

PieroV commented 3 months ago

Is there any difference between that LLVM and what llvm-mingw / msys2-llvm provide? The official runner software list lists Microsoft.VisualStudio.Component.VC.Llvm.Clang. So I'm wondering what the difference between microsoft LLVM and other LLVM toolchains is (and if I can detect that from CMake to decide on the toolchain)

As far as I know, the binaries provided by LLVM include only the compilers and other tools (linker, windres, etc...), they don't include any runtime and/or standard library implementation. They will look for an existing Visual Studio installation by default on Windows, but it should work also with mingw or even be able to cross compile with the proper incantation (a mix of --sysroot, --target, and similar arguments).

The llvm-mingw binaries provide an LLVM toolchain plus mingw, libc++ (LLVM's C++ standard library implementation), libunwind, compiler-rt, and maybne some other runtime. Theoretically, you could use the official binaries with llvm-mingw's runtimes, but I don't know if there's a reason to prefer it over using only llvm-mingw for this use-case.

mati865 commented 3 months ago

should work also with mingw or even be able to cross compile with the proper incantation (a mix of --sysroot, --target, and similar arguments).

Some features will be missing when using mingw-w64 built with GCC though, like Control Flow Guard support. Also, libraries like winpthreads won't be fully compatible because of different TLS and unwinders. This should be fine for most users, but it's better to use purposely built mingw-w64 if doable.

jschwe commented 2 months ago

It looks like the upstream cargo fix will still take a while to hit stable, so in the meantime I'm fine with merging the quickfix I added in this branch (which copies the file out of the deps dir). Once the upstream fix has reached stable, I would remove this workaround though, and instead just raise the minimum rust version corrosion requires for gnu-llvm targets.

PieroV commented 2 months ago

Thank you very much for working on this!