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

Windows CI is borked #88796

Closed
ehuss opened this issue Sep 9, 2021 · 13 comments
Closed

Windows CI is borked #88796

ehuss opened this issue Sep 9, 2021 · 13 comments
Labels
O-windows Operating system: Windows

Comments

@ehuss
Copy link
Contributor

ehuss commented Sep 9, 2021

Windows CI has stopped working due to an image update on GitHub's side. The new image is win19/20210907.4 (included software). The previous image was win19/20210903.7 (included software). The image diff is here.

There are two errors showing up described below.

I think the most likely culprit is the addition of Windows 10 SDK 20348. Previously, the most recent SDK installed was 19041.

There are some interesting release notes for 20348 here:

Clang/LLVM for Windows v11 targeting ARM64 is not compatible with the latest winnt.h

As a workaround, use the previous version of the Windows 10 SDK (build 19041), or clang/LLVM for Windows v10 when targeting ARM64 platforms

I don't know what "LLVM for Windows v11" means in this context, but it seems fishy.

dist-aarch64-msvc fails with undeclared identifier '__umulh'

While building profiler_builtins there is an error:

cargo:warning=In file included from ../../src/llvm-project/compiler-rt\lib\profile\InstrProfilingRuntime.cpp:11:
cargo:warning=In file included from ../../src/llvm-project/compiler-rt\lib\profile/InstrProfiling.h:12:
cargo:warning=In file included from ../../src/llvm-project/compiler-rt\lib\profile/InstrProfilingPort.h:65:
cargo:warning=In file included from C:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\um\windows.h:171:
cargo:warning=In file included from C:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\shared\windef.h:24:
cargo:warning=In file included from C:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\shared\minwindef.h:182:
cargo:warning=C:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\um\winnt.h(6370,20): error: use of undeclared identifier '__umulh'
cargo:warning=    *HighProduct = UnsignedMultiplyHigh(Multiplier, Multiplicand);
cargo:warning=                   ^
cargo:warning=C:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\um\winnt.h(6236,30): note: expanded from macro 'UnsignedMultiplyHigh'
cargo:warning=#define UnsignedMultiplyHigh __umulh
cargo:warning=                             ^
cargo:warning=1 error generated.
exit code: 1
 
--- stderr
 
 
error occurred: Command "D:/a/rust/rust/citools/clang-rust/bin/clang-cl.exe" "-nologo" "-MT" "-O2" "-Z7" "-Brepro" "--target=aarch64-pc-windows-msvc" "-I" "../../src/llvm-project/compiler-rt\\include" "/Zl" "-Dstrdup=_strdup" "-Dopen=_open" "-Dfdopen=_fdopen" "-Dgetpid=_getpid" "-Dfileno=_fileno" "-DCOMPILER_RT_HAS_ATOMICS=1" "-FoD:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage1-std\\aarch64-pc-windows-msvc\\release\\build\\profiler_builtins-7ff61bd0c2dd5988\\out\\../../src/llvm-project/compiler-rt\\lib\\profile\\InstrProfilingRuntime.o" "-c" "../../src/llvm-project/compiler-rt\\lib\\profile\\InstrProfilingRuntime.cpp" with args "clang-cl.exe" did not execute successfully (status code exit code: 1).

Full log example here: https://github.com/rust-lang-ci/rust/actions/runs/1218329003

NatVis error

There are three debuginfo tests that are failing:

failures:
    [debuginfo-cdb] debuginfo\basic-types.rs
    [debuginfo-cdb] debuginfo\msvc-pretty-enums.rs
    [debuginfo-cdb] debuginfo\pretty-std.rs

These are failing because some Rust types are not displaying correctly (like strings).

Full log example here: https://github.com/rust-lang-ci/rust/actions/runs/1218098190

Failing tests report Windows Debugger Version 10.0.20348.1.

Comments

I don't know of the proper way of reverting to a previous Windows SDK is. My understanding is that is usually done by calling vcvarsall.bat, where you can specify the SDK version (and sets a bunch of environment variables). Calling that on CI is awkward (it only really works with cmd, or do weird things like this).

Regarding the cdb errors, I'm also wondering if that might have been caused by the update to Windows SDK. It is not really clear to me the relationship between the cdb.exe and the Windows Kit (but cdb is installed in Windows Kits\10\Debuggers).

@ehuss
Copy link
Contributor Author

ehuss commented Sep 9, 2021

@m-ou-se pointed out https://bugs.llvm.org/show_bug.cgi?id=51128 which seems relevant here. That commit is included on our master branch, but not on beta.

cc @rust-lang/wg-llvm

@ehuss
Copy link
Contributor Author

ehuss commented Sep 9, 2021

@rustbot ping windows

@rustbot
Copy link
Collaborator

rustbot commented Sep 9, 2021

Hey Windows Group! This bug has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @nico-abram @retep998 @rylev @sivadeilra @wesleywiser

@rustbot rustbot added the O-windows Operating system: Windows label Sep 9, 2021
@nikic
Copy link
Contributor

nikic commented Sep 9, 2021

@m-ou-se pointed out https://bugs.llvm.org/show_bug.cgi?id=51128 which seems relevant here. That commit is included on our master branch, but not on beta.

Our LLVM fork isn't used for the host compiler, I believe that would be what is installed in

elif isWindows && [[ ${CUSTOM_MINGW-0} -ne 1 ]]; then
for clang-cl. So that version would have to support the builtin.

@m-ou-se
Copy link
Member

m-ou-se commented Sep 10, 2021

After @slurps-mad-rips, @ehuss and I tried a bunch of things, we realized after about 13 attempts that the problem is that clang-cl doesn't react at all to vcvars nor does it (the version we have) have a flag for specifying the sdk version. It just always picks the latest.

So it seems like the only way to downgrade, is to uninstall sdk 20348.

Just rm -rf'ing the 20348 directory didn't work. Apparently other parts of the installation are still referring to that directory. clang-cl couldn't find some standard c header. There's probably a proper way to uninstall it. But I don't know it.

What I did for now is renaming the 19041 sdk directory to 20348. An ugly hack. But it seems to work. (This is blocking our entire CI, so unblocking it asap is of high priority.)

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 10, 2021
Work around CI issue with windows sdk 10.0.20348.0.

See rust-lang#88796 (comment)
@danielframpton
Copy link
Contributor

danielframpton commented Sep 10, 2021

Was clang-cl picking it up directly, or was it through environment variables supplied by cc-rs?

Looking here there is logic to get the newest SDK that contains a specific file. If it is cc-rs finding the sdk, then adding support there to have an environment variable (e.g., WindowsSDKVersion) to override the version could work.

@bruxisma
Copy link

I would like to clarify a few things here, as I think there is a misunderstanding of clang and clang-cl.

clang-cl is an MSVC-like frontend that works as if it were cl.exe. It was at one point supported by visual studio. However Microsoft dropped support for it, and instead worked on making clang (the gcc style driver) work with visual studio, the STL, Windows SDK, and CMake. In fact, CMake has explicit support for clang's gcc style interface but running on windows and targeting the MSVC toolchain. This is also the recommended way to use clang on windows.

clang-cl does have a flag that can override what tuples it searches for. This flag is /vctoolsdir and can be pointed to the exact version that is needed. I did point this out to @m-ou-se, but it was also 3:30 AM her time when I did, so I understand that she might have missed it. There is also a /winsdkdir. You can see examples of this when searching for vctoolsdir

That said, I would recommend either passing /vctoolsdir when using clang-cl, or kick clang-cl as a compiler out and rely on the gcc style driver instead.

@m-ou-se
Copy link
Member

m-ou-se commented Sep 10, 2021

That winsdkdir option does not exist in the version of clang-cl that we use. It was added later: https://reviews.llvm.org/D95472

@bruxisma
Copy link

Then I continue to recommend not using clang-cl at all and rely on plain clang.

@wesleywiser
Copy link
Member

Just fyi, I am looking at the natvis errors in the debugger tests.

@ChrisDenton
Copy link
Member

Should the two issues be tracked separately now that there is a work around for the immediate problem?

@wesleywiser
Copy link
Member

PR is up to fix most of the debuginfo tests but I'm still looking into the issue with str.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 1, 2023
…ark-Simulacrum

aarch64-msvc: remove CI hack for bad Windows SDK version

This removes hack which manually replaced windows sdk version, as it looks like useless now, as CI uses newer version: https://github.com/rust-lang-ci/rust/actions/runs/5596259246/jobs/10233070602#step:24:929
`C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\corecrt_io.h` (look at version)

related rust-lang#88796

It's nice to have some way to assert bad version, but i don't see anything except checking env rust-lang/cc-rs#646
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Aug 2, 2023
…acrum

aarch64-msvc: remove CI hack for bad Windows SDK version

This removes hack which manually replaced windows sdk version, as it looks like useless now, as CI uses newer version: https://github.com/rust-lang-ci/rust/actions/runs/5596259246/jobs/10233070602#step:24:929
`C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\corecrt_io.h` (look at version)

related rust-lang/rust#88796

It's nice to have some way to assert bad version, but i don't see anything except checking env rust-lang/cc-rs#646
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
…acrum

aarch64-msvc: remove CI hack for bad Windows SDK version

This removes hack which manually replaced windows sdk version, as it looks like useless now, as CI uses newer version: https://github.com/rust-lang-ci/rust/actions/runs/5596259246/jobs/10233070602#step:24:929
`C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\corecrt_io.h` (look at version)

related rust-lang/rust#88796

It's nice to have some way to assert bad version, but i don't see anything except checking env rust-lang/cc-rs#646
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…acrum

aarch64-msvc: remove CI hack for bad Windows SDK version

This removes hack which manually replaced windows sdk version, as it looks like useless now, as CI uses newer version: https://github.com/rust-lang-ci/rust/actions/runs/5596259246/jobs/10233070602#step:24:929
`C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\corecrt_io.h` (look at version)

related rust-lang/rust#88796

It's nice to have some way to assert bad version, but i don't see anything except checking env rust-lang/cc-rs#646
@ehuss
Copy link
Contributor Author

ehuss commented Sep 8, 2024

I'm going to close as I believe this is now resolved. I didn't investigate, but I assume that as newer versions of clang are being used, that seems to have resolved the issue. #113854 removed the hack, and AFAIK everything is working.

@ehuss ehuss closed this as completed Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

8 participants