Windows arm64: use cc-rs' prefer_clang_cl_over_msvc#2699
Windows arm64: use cc-rs' prefer_clang_cl_over_msvc#2699dennisameling wants to merge 2 commits intobriansmith:mainfrom
prefer_clang_cl_over_msvc#2699Conversation
| "/Zc:forScope", | ||
| "/Zc:inline", | ||
| // Warnings. | ||
| "/Wall", |
There was a problem hiding this comment.
This seems to be triggering the warnings (which are then treated as errors) that @MarijnS95 reported here. Even when adding /std:c11, they keep showing up:
warning: [email protected]: In file included from C:\repos\ring\crypto/curve25519/curve25519.c:22:
warning: [email protected]: In file included from C:\repos\ring\crypto/curve25519\internal.h:18:
warning: [email protected]: C:\repos\ring\include\ring-core/base.h(60,1): error: '_Static_assert' is incompatible with C standards before C11 [-Werror,-Wpre-c11-compat]
warning: [email protected]: 60 | OPENSSL_STATIC_ASSERT(sizeof(int32_t) == sizeof(int), "int isn't 32 bits.");
warning: [email protected]: | ^
warning: [email protected]: C:\repos\ring\include\ring-core/type_check.h(29,42): note: expanded from macro 'OPENSSL_STATIC_ASSERT'
warning: [email protected]: 29 | #define OPENSSL_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
warning: [email protected]: | ^
warning: [email protected]: In file included from C:\repos\ring\crypto/curve25519/curve25519.c:22:
warning: [email protected]: In file included from C:\repos\ring\crypto/curve25519\internal.h:18:
warning: [email protected]: C:\repos\ring\include\ring-core/base.h(61,1): error: '_Static_assert' is incompatible with C standards before C11 [-Werror,-Wpre-c11-compat]
warning: [email protected]: 61 | OPENSSL_STATIC_ASSERT(sizeof(uint32_t) == sizeof(unsigned int), "unsigned int isn't 32 bits.");
warning: [email protected]: | ^
warning: [email protected]: C:\repos\ring\include\ring-core/type_check.h(29,42): note: expanded from macro 'OPENSSL_STATIC_ASSERT'
warning: [email protected]: 29 | #define OPENSSL_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
warning: [email protected]: | ^
warning: [email protected]: In file included from C:\repos\ring\crypto/curve25519/curve25519.c:22:
warning: [email protected]: In file included from C:\repos\ring\crypto/curve25519\internal.h:18:
warning: [email protected]: C:\repos\ring\include\ring-core/base.h(62,1): error: '_Static_assert' is incompatible with C standards before C11 [-Werror,-Wpre-c11-compat]
warning: [email protected]: 62 | OPENSSL_STATIC_ASSERT(sizeof(size_t) == sizeof(uintptr_t), "uintptr_t and size_t differ.");
warning: [email protected]: | ^
warning: [email protected]: C:\repos\ring\include\ring-core/type_check.h(29,42): note: expanded from macro 'OPENSSL_STATIC_ASSERT'
warning: [email protected]: 29 | #define OPENSSL_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
warning: [email protected]: | ^
Removing the /Wall flag fixes the build. This looks like a bug in clang-cl.exe to me and I could reproduce the behavior in a minimal example. Reported that here: https://developercommunity.visualstudio.com/t/Calling-clang-clexe-with-Wall-reports/10961806
There was a problem hiding this comment.
This is not a bug in clang-cl - /Wall in CL parlance translates to -Weverything in Clang parlance. That includes these spurious warnings designed to help users migrating code forward to avoid pitfalls when their code base uses multiple C versions simultaneously.
There was a problem hiding this comment.
See PR #2802, which reduces the warning level with MSVC to /W4 on all targets.
| Alternatively, if the host machine is already a Windows ARM64 then use: | ||
| ``` | ||
| $env:Path += ";C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\ARM64\bin" | ||
| ``` |
| }; | ||
| // On Windows AArch64 we currently must use Clang to compile C code. | ||
| // clang-cl.exe has support for MSVC-style command-line arguments. | ||
| if target.os == WINDOWS && target.arch == AARCH64 && !compiler.is_like_clang() { |
There was a problem hiding this comment.
Is !compiler.is_like_clang() necessary? I would expect cc-rs to already check that itself.
There was a problem hiding this comment.
IIRC, this was in place because there's technically two flavors of Clang for Windows:
- GNU LLVM like
mingw-w64-clang-aarch64-clangfrom MSYS2. This is based on Cygwin, which in turn tries to provide broad POSIX API compatibility for offering GNU tools on Windows. This one doesn't have aclang-cl.exebinary. - The Clang build that Microsoft offers as an optional feature in Visual Studio. This one does have
clang-cl.exeto offer compatibility with MSVC-style flags.
If I understand things correctly, cc-rs will look at the target first (e.g. aarch64-pc-windows-msvc or aarch64-pc-windows-gnullvm) and based on that finds the appropriate compiler. It defaults to cl.exe for the former, and to clang.exe for the latter. The prefer_clang_cl_over_msvc flag just forces the use of clang-cl.exe instead of cl.exe, but looking at the logic shouldn't force clang-cl.exe if the compiling for targets like aarch64-pc-windows-gnullvm.
TL;DR: I think it should be OK to leave it out here, but we'd need to test compiling for the aarch64-pc-windows-gnullvm target to be sure. I'm a bit short on time this week, but will try to squeeze in some test runs.
There was a problem hiding this comment.
Looks like it indeed works just fine in your updated setup. Tested with cargo test --target aarch64-pc-windows-gnullvm in MSYS2 on arm64 Windows, as well as cargo test --target aarch64-pc-windows-msvc in regular Windows 11 ARM with Visual Studio 2022 + Clang installed.
I also tried the latter by explicitly removing Clang from the Visual Studio 2022 installation, which then failed with a clear error message. I'll work with the cc-rs team to get a dedicated entry in that documentation URL for clang-cl.exe not being found, indicating that users need to install the Clang tools for Visual Studio.
cargo:warning=Compiler family detection failed due to error: ToolNotFound: failed to find tool "clang-cl.exe": program not found (see https://docs.rs/cc/latest/cc/#compile-time-requirements for help)
--- stderr
running "perl" "C:/repos/ring/crypto/chacha/asm/chacha-armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/chacha-armv8-win64.S"
running "perl" "C:/repos/ring/crypto/cipher/asm/chacha20_poly1305_armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/chacha20_poly1305_armv8-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/aes/asm/aesv8-armx.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/aesv8-armx-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/aes/asm/aesv8-gcm-armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/aesv8-gcm-armv8-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/aes/asm/ghash-neon-armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/ghash-neon-armv8-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/aes/asm/ghashv8-armx.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/ghashv8-armx-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/aes/asm/vpaes-armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/vpaes-armv8-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/bn/asm/armv8-mont.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/armv8-mont-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/ec/asm/p256-armv8-asm.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/p256-armv8-asm-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/sha/asm/sha512-armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/sha512-armv8-win64.S"
running "perl" "C:/repos/ring/crypto/fipsmodule/sha/asm/sha512-armv8.pl" "win64" "C:/repos/ring/target/aarch64-pc-windows-msvc/debug/build/ring-a8b6d2989661e2f1/out/sha256-armv8-win64.S"
error occurred in cc-rs: failed to find tool "clang-cl.exe": program not found (see https://docs.rs/cc/latest/cc/#compile-time-requirements for help)
|
If you can rebase this on top of the current main branch then I can review it this week and get it merged. Please check whether the |
|
I took a stab at doing the same thing in PR #2803, cleaning up the logic a bit. PTAL. |
|
Thanks for submitting this. A slightly different version of these changes was merged in PR #2803. |
3 participants
This is a new attempt to address #2215, and might supersede #2216 - I'll leave that up to you.
cc-rsversion 1.2.35 introduced support forprefer_clang_cl_over_msvc, which automatically findsclang-clin the Visual Studio installation directory and uses it instead ofcl.exe. Sinceclang-clsupports MSVC-style flags, this should address the reported issue.Confirming this is working as expected on Windows arm64. In this CI run you can see that
clang-cl.exeis invoked as expected.