llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.36k stars 12.14k forks source link

libcxx's `copy_move.pass.cpp` unexpectly passed on Windows with clang-cl when targeting `x86_64-pc-windows-msvc` #63442

Closed zeroomega closed 1 year ago

zeroomega commented 1 year ago

We are working on enable check-runtimes-x86_64-pc-windows-msvc test on our Clang Windows bots and we noticed that std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp which is expected to fail when targeting x86_64-pc-windows-msvc while using libc++, is passing under this environment.

failing bot run: https://luci-milo.appspot.com/raw/build/logs.chromium.org/fuchsia/led/trev_google.com/e3b2824e614fce4acbd79a4b580878bc10a4fa9ac26086aaf5189b0bb88b51aa/+/build.proto

and error message (locally reproduced):

> C:\src\llvm-project\build\bin\llvm-lit.py c:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\libcxx\test  -a -vv --filter .*std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp 
-- Testing: 1 of 10074 tests, 1 workers --
XPASS: llvm-libc++-static-clangcl.cfg.in :: std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp (1 of 1)
******************** TEST 'llvm-libc++-static-clangcl.cfg.in :: std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp' FAILED ********************
Script:
--
: 'COMPILED WITH';  C:/src/llvm-project/build/./bin/clang-cl.exe C:\src\llvm-project\src\libcxx\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\copy_move.pass.cpp --driver-mode=g++ --target=x86_64-pc-windows-msvc -nostdinc++ -I C:/src/llvm-project/build/include/c++/v1 -I C:/src/llvm-project/build/include/x86_64-pc-windows-msvc/c++/v1 -I C:/src/llvm-project/src/libcxx/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  -llibc++experimental -nostdlib -L C:/src/llvm-project/build/./lib/x86_64-pc-windows-msvc -llibc++ -lmsvcrt -lmsvcprt -loldnames -o C:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\Output\copy_move.pass.cpp.dir\t.tmp.exe
: 'EXECUTED AS';  "C:/tools/python3/bin/python.exe" C:/src/llvm-project/src/libcxx/test/../utils/run.py --execdir C:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\Output\copy_move.pass.cpp.dir --  C:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\Output\copy_move.pass.cpp.dir\t.tmp.exe
--
Exit Code: 0

Command Output (stdout):
--
$ ":" "COMPILED WITH"
$ "C:/src/llvm-project/build/./bin/clang-cl.exe" "C:\src\llvm-project\src\libcxx\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\copy_move.pass.cpp" "--driver-mode=g++" "--target=x86_64-pc-windows-msvc" "-nostdinc++" "-I" "C:/src/llvm-project/build/include/c++/v1" "-I" "C:/src/llvm-project/build/include/x86_64-pc-windows-msvc/c++/v1" "-I" "C:/src/llvm-project/src/libcxx/test/support" "-D_CRT_SECURE_NO_WARNINGS" "-D_CRT_NONSTDC_NO_WARNINGS" "-D_CRT_STDIO_ISO_WIDE_SPECIFIERS" "-DNOMINMAX" "-std=c++26" "-Werror" "-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" "-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wno-user-defined-literals" "-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-Wno-local-type-template-args" "-Wno-c++11-extensions" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" "-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" "-Wuser-defined-warnings" "-llibc++experimental" "-nostdlib" "-L" "C:/src/llvm-project/build/./lib/x86_64-pc-windows-msvc" "-llibc++" "-lmsvcrt" "-lmsvcprt" "-loldnames" "-o" "C:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\Output\copy_move.pass.cpp.dir\t.tmp.exe"
$ ":" "EXECUTED AS"
$ "C:/tools/python3/bin/python.exe" "C:/src/llvm-project/src/libcxx/test/../utils/run.py" "--execdir" "C:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\Output\copy_move.pass.cpp.dir" "--" "C:\src\llvm-project\build\runtimes\runtimes-x86_64-pc-windows-msvc-bins\test\std\utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.con\Output\copy_move.pass.cpp.dir\t.tmp.exe"

--

********************
********************
Unexpectedly Passed Tests (1):
  llvm-libc++-static-clangcl.cfg.in :: std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp

Testing Time: 2.05s
  Excluded           : 10073
  Unexpectedly Passed:     1

We have some suspicion that it might be related to https://devblogs.microsoft.com/cppblog/improving-copy-and-move-elision/ , however, while we use the Windows SDK came with the VS2022 to build the clang on Windows, we use the clang-cl to build our stage 1 Clang for Windows instead of using the cl from msvc, so in theory, this msvc side change shouldn't affect us.

Is it a behavior change from clang-cl side? Should we update this test to expect to PASS to match the actual behavior we saw?

zeroomega commented 1 year ago

C.C. @petrhosek @mstorsjo

mstorsjo commented 1 year ago

I tried to reproduce this, by running the libcxx tests with a freshly built clang-cl from git main, but I don't hit this issue. So it's most probably not a code change in upstream clang-cl.

It's possible that it's caused by a newer MSVC than the one used in the CI environment (and in my test environment). The MSVC used to compile clang doesn't affect things, but libc++ in Clang-cl environments builds on top of parts of the MSVC STL and vcruntime, so it's plausible that upstream changes there affects the situation. Let me see if I can try that somehow easily...

mstorsjo commented 1 year ago

Even if linking against the most recent MSVC libraries, I can't reproduce your situation.

I also tested building the test executable with the exact command from COMPILED WITH above in your example, but it still fails expectedly. Since you can reproduce it in two separate locations, it sounds like it is some configuration that differs, that seem to affect this.

Can you share the __config_site for your libc++ build? I doubt that it contains something of relevance though...

As it seems to be some difference in how it is configured (systematically) in your builds, can you try building in plain standalone mode, with the most basic default configuration? Adapted from the steps at https://libcxx.llvm.org/BuildingLibcxx.html#cmake-ninja-msvc, can you do e.g. this:

> cd llvm-project
> set PATH=c:\path\to\recent\build\bin;%PATH%
> mkdir build-libcxx
> cd build-libcxx
> cmake -G Ninja ../runtimes -DCMAKE_C_COMPILER=clang-cl -DCMAKE_CXX_COMPILER=clang-cl -DLLVM_ENABLE_RUNTIMES=libcxx
> ninja
> python bin/llvm-lit.py -a ../libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp

If that works, you can use that as basis to try to pinpoint the difference to the build/test environment in your full runtimes build.

zeroomega commented 1 year ago

Thanks for looking into this.

I put the __config_site content to https://gist.github.com/zeroomega/e0cce833375b8e0c99828dca870571c2 . This file is also available in our clang toolchain package for Windows x64: https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/clang/windows-amd64/+/git_revision:ebd0b8a0472b865b7eb6e1a32af97ae31d829033

I will try to build libcxx in standalone mode and see if I can reproduce it.

mstorsjo commented 1 year ago

I see what the relevant bit is now; if building libcxx with LIBCXX_ABI_VERSION=2, then this test does pass unexpectedly.

zeroomega commented 1 year ago

Shall we modify the test or disable the test if LIBCXX_ABI_VERSION=2?

mstorsjo commented 1 year ago

Overall, for non-MSVC environments, we definitely should keep testing this with LIBCXX_ABI_VERSION=2. If it were possible, I guess the best thing would be to change the xfail into XFAIL: target=x86_64-pc-windows-msvc && stdlib=libc++ && libcxx-abi-version-1. I don't know if we have such a test tag, but it should be fairly straightforward to define it if we don't have it yet.