briansmith
Conflicting `C(XX)FLAGS` when cross-compiling to `aarch64-pc-windows-msvc`
First of all thanks for landing #1339 in some crates.io release! Would be great if this repository had git tags to follow along where commits are reachable :sweat_smile:
Setup
When cross-compiling (currently from a Linux setup) to Windows, it's common to use https://jake-shadle.github.io/xwin/ / https://github.com/Jake-Shadle/xwin to set up headers and libraries.
When running this in my home directory (/home/marijn):
$ xwin --arch x86_64,aarch64 --sdk-version 10.0.22621 splat
I add the following to my ~/.cargo/config.toml to support cross-compiling C(++) sources in our Rust tree, and ultimately linking a binary:
[target.aarch64-pc-windows-msvc]
linker = "lld-link"
rustflags = [
"-Lnative=/home/marijn/.xwin-cache/splat/crt/lib/aarch64",
"-Lnative=/home/marijn/.xwin-cache/splat/sdk/lib/um/aarch64",
"-Lnative=/home/marijn/.xwin-cache/splat/sdk/lib/ucrt/aarch64",
]
[env]
CC_aarch64-pc-windows-msvc = "clang-cl"
CXX_aarch64-pc-windows-msvc = "clang-cl"
AR_aarch64-pc-windows-msvc = "llvm-lib"
CFLAGS_aarch64-pc-windows-msvc = "-Wno-unused-command-line-argument -fuse-ld=lld-link -vctoolsdir/home/marijn/.xwin-cache/splat/crt -winsdkdir/home/marijn/.xwin-cache/splat/sdk"
CXXFLAGS_aarch64-pc-windows-msvc = "-Wno-unused-command-line-argument -fuse-ld=lld-link -vctoolsdir/home/marijn/.xwin-cache/splat/crt -winsdkdir/home/marijn/.xwin-cache/splat/sdk"
Problem statement
Unfortunately the Windows ARM64 support PR (#1339) overwrites the compiler with clang:
https://github.com/briansmith/ring/blob/d2e401ff0b5bf0d056e553075e16458b0d8a882b/build.rs#L549-L556
This code doesn't modify the flags, and leaves what my environment has in CFLAGS_aarch64-pc-windows-msvc in place. Without using the clang-cl driver, clang doesn't understand the above commands and compiling ring errors out with a bunch of errors like:
warning: [email protected]: clang: error: unknown argument: '-vctoolsdir/home/marijn/.xwin-cache/splat/crt'
warning: [email protected]: clang: error: unknown argument: '-winsdkdir/home/marijn/.xwin-cache/splat/sdk'
warning: [email protected]: ToolExecError: Command "clang" "-O3" "--target=aarch64-pc-windows-msvc" "-ffunction-sections" "-fdata-sections" "-g" "-fno-omit-frame-pointer" "--target=aarch64-pc-windows-msvc" "-I" "ring/include" "-I" "myproject/target/aarch64-pc-windows-msvc/debug/build/ring-81f4360e84b9e6a3/out" "-Wno-unused-command-line-argument" "-fuse-ld=lld-link" "-vctoolsdir/home/marijn/.xwin-cache/splat/crt" "-winsdkdir/home/marijn/.xwin-cache/splat/sdk" "-fvisibility=hidden" "-std=c1x" "-Wall" "-Wbad-function-cast" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wnested-externs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wstrict-prototypes" "-Wundef" "-Wuninitialized" "-g3" "-Werror" "-o" "myproject/target/aarch64-pc-windows-msvc/debug/build/ring-81f4360e84b9e6a3/out/8551bf18c762c50d-curve25519.o" "-c" "ring/crypto/curve25519/curve25519.c" with args clang did not execute successfully (status code exit status: 1).
Proposed solution
As mentioned at https://github.com/briansmith/ring/pull/1339#issuecomment-899678208 clang-cl works, albeit currently (on LLVM 18.1.8) with lots of warnings of the form:
error: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]
error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage]
I'd be happy to PR a change to not overwrite the compiler when it is clang-cl like, next to the existing ... && !compiler.is_clang_like(). cc-rs already tracks if it's clang_cl but doesn't currently expose that publicly, so we'll have to add that first.
Thanks!
@briansmith these are the errors that show up when compiling with cl.exe from MSVC 14.41.34120 for aarch64-pc-windows-msvc (cross-compiled from x64 Windows host):
ring\crypto\fipsmodule\bn\internal.h(191): error C2065: 'BN_ULLONG': undeclared identifier
ring\crypto\fipsmodule\bn\internal.h(191): error C2146: syntax error: missing ';' before identifier 'result'
ring\crypto\fipsmodule\bn\internal.h(191): warning C4555: result of expression not used
ring\crypto\fipsmodule\bn\internal.h(191): error C2065: 'result': undeclared identifier
ring\crypto\fipsmodule\bn\internal.h(191): error C2065: 'BN_ULLONG': undeclared identifier
ring\crypto\fipsmodule\bn\internal.h(191): error C2146: syntax error: missing ';' before identifier 'a'
ring\crypto\fipsmodule\bn\internal.h(191): warning C4552: '*': result of expression not used
ring\crypto\fipsmodule\bn\internal.h(192): error C2065: 'result': undeclared identifier
ring\crypto\fipsmodule\bn\internal.h(193): error C2065: 'result': undeclared identifier
ring\crypto\fipsmodule\bn\internal.h(193): warning C4293: '>>': shift count negative or too big, undefined behavior
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(38): warning C4163: '_addcarry_u64': not available as an intrinsic function
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(38): warning C4163: '_subborrow_u64': not available as an intrinsic function
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(62): warning C4013: '_addcarry_u64' undefined; assuming extern returning int
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(62): warning C4242: '=': conversion from 'int' to 'Carry', possible loss of data
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(76): warning C4242: '=': conversion from 'int' to 'Carry', possible loss of data
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(92): warning C4013: '_subborrow_u64' undefined; assuming extern returning int
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(92): warning C4242: '=': conversion from 'int' to 'Carry', possible loss of data
ring\crypto\fipsmodule\bn\../../limbs/limbs.inl(106): warning C4242: '=': conversion from 'int' to 'Carry', possible loss of data
The first ones in internal.h are trivial, we have 3e6526575ac2349a44a04a0bbc7acb917fab5a0b that documents that uint128_t isn't defined by MSVC, and this custom code that uses the missing typedef but doesn't have a fallback for non-x86 with MSVC:
https://github.com/briansmith/ring/blob/0223acbbd86d433389288553e17ad14d0930fc24/crypto/fipsmodule/bn/internal.h#L186-L195
A fix for this is described here: https://github.com/briansmith/ring/pull/1339#discussion_r685578968 https://github.com/google/boringssl/blob/47c5f9d2f6c294622baf07c3373de752fd518b03/crypto/fipsmodule/bn/internal.h#L418
The second _addcarry_u64 / _subborrow_u64 intrinsics are behind a #if defined(_MSC_VER) && !defined(__clang__) and probably need to check for the architecture. If they're not defined, a C implementation seems to be used. The #else case here however also uses uint128_t:
https://github.com/briansmith/ring/blob/0223acbbd86d433389288553e17ad14d0930fc24/crypto/limbs/limbs.inl#L47-L54
@briansmith can we reopen this to track the changes necessary for making ring compile for Windows Aarch64?
Unfortunately the Windows ARM64 support PR (https://github.com/briansmith/ring/pull/1339) overwrites the compiler with clang:
Here's a PR to remove that workaround and enable things to be built with MSVC for arm64. That should resolve the issues you're seeing here! Would you mind testing it?
@dennisameling Thanks for the PR. Have you tested whether upstream BoringSSL builds with MSVC for AArch64? At least the last time I checked, they were still using clang for all AArch64 Windows builds.
There are two reasons why we force the use of clang for aarch64-pc-windows-msvc:
- That was the easiest to get working.
- That is what BoringSSL had tested.
In particular, BoringSSL does there side channel "constant-time" tests in their CI, and we lean on that testing. So If they're not testing the MSVC output for AArch64, it's hard for us to support MSVC for AArch64.
I noticed that recently BoringSSL has made some changes related to MSVC. I will try to merge those to see how they help with (a) switching to clang-cl and/or (b) supporting MSVC, for this target.
Unfortunately the Windows ARM64 support PR (#1339) overwrites the compiler with clang:
Here's a PR to remove that workaround and enable things to be built with MSVC for arm64. That should resolve the issues you're seeing here! Would you mind testing it?
As linked above, we already have a solution available at #2216. This quote is an old problem.
I will try to merge those to see how they help with (a) switching to clang-cl
@briansmith this was likely a typo, but note that compiling ring for Windows ARM64 using clang-cl is already "supported" per #2216. It was merely blocked on moving the logic to prefer clang over MSVC (with compatible clang-cl interface used for cl.exe flags) into the cc-rs crate: https://github.com/rust-lang/cc-rs/issues/882 / https://github.com/rust-lang/cc-rs/pull/1367.
Thank you for your investigation. I have also successfully cross compiled the executable file of target x86_64 pc windows msvc using xwin.
