msquic icon indicating copy to clipboard operation
msquic copied to clipboard

clang/LLVM for Windows build failures

Open walbourn opened this issue 2 years ago • 5 comments

Describe the bug

Building with clang/LLVM for Windows 15.0.1 fails with the following error on every file:

C:\PROGRA~1\MICROS~3\2022\COMMUN~1\VC\Tools\Llvm\x64\bin\clang-cl.exe   -DCXPLAT_TLS_SECRETS_SUPPORT=1 -DQUIC_DISABLE_CLIENT_CERT_TESTS -DQUIC_EVENTS_STUB -DQUIC_LOGS_STUB -DSECURITY_WIN32 -DVER_BUILD_ID=0 -DVER_SUFFIX=-private -DWIN32_LEAN_AND_MEAN -ID:\vcpkg\buildtrees\ms-quic\src\v1.2.0-19ce393c24.clean\src\inc /nologo /DWIN32 /D_WINDOWS /W3 /utf-8  -m64 /D_DEBUG /MDd /Z7 /Ob0 /Od /RTC1  -MDd /analyze /W4 /sdl /showIncludes /Fosrc\core\CMakeFiles\core.dir\listener.c.obj /Fdsrc\core\CMakeFiles\core.dir\core.pdb -c -- D:\vcpkg\buildtrees\ms-quic\src\v1.2.0-19ce393c24.clean\src\core\listener.c
clang-cl: error: no such file or directory: '/analyze'; did you mean '/analyze-'?

The problem is that in the CMake files, you are assuming /analyze is supported by all 'MSVC-like' compilers which is not the case.

Change in your three CMake project files:

if (MSVC)
    target_compile_options(core PRIVATE /analyze)
endif()
...
if (MSVC AND (QUIC_TLS STREQUAL "openssl" OR QUIC_TLS STREQUAL "schannel"))
    target_compile_options(platform PRIVATE /analyze)
endif()

to

if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
    target_compile_options(core PRIVATE /analyze)
endif()
...
if ((CMAKE_CXX_COMPILER_ID MATCHES "MSVC") AND (QUIC_TLS STREQUAL "openssl" OR QUIC_TLS STREQUAL "schannel"))
    target_compile_options(platform PRIVATE /analyze)
endif()

Affected OS

  • [X] Windows
  • [ ] Linux
  • [ ] macOS
  • [ ] Other (specify below)

Additional OS information

No response

MsQuic version

release/1.1

Steps taken to reproduce bug

Using clang-cl to build the library with VCPKG. Note it's actually using "v1.2.0" but the issue is present in the mainline as well.

Expected behavior

Should build.

Actual outcome

Fails to build.

Additional details

No response

walbourn avatar Apr 25 '23 03:04 walbourn

Later versions all use the QUIC_ENABLE_SANITIZERS flag to gate enabling these:

if (MSVC AND NOT QUIC_ENABLE_SANITIZERS AND BUILD_SHARED_LIBS)
    target_compile_options(msquic PRIVATE /analyze)
endif()
...
if (MSVC AND NOT QUIC_ENABLE_SANITIZERS)
    target_compile_options(core PRIVATE /analyze)
endif()
...
if (MSVC AND (QUIC_TLS STREQUAL "openssl" OR QUIC_TLS STREQUAL "schannel") AND NOT QUIC_ENABLE_SANITIZERS)
    target_compile_options(platform PRIVATE /analyze)
endif()

Would you be willing/able to update to the latest?

nibanks avatar Apr 25 '23 11:04 nibanks

The VCPKG port is still on 1.2, but can be updated with a PR to the latest release.

Note there are a number of patches in the VCPKG port. Have you looked at integrating them back?

https://github.com/microsoft/vcpkg/pull/18225/files#diff-95eb529fdc4a632216fe17da3b1ec2558d32adec9e519fda851782375ec0bd15

walbourn avatar Apr 25 '23 17:04 walbourn

Someone is actively working on updating vcpkg to 2.1: https://github.com/microsoft/vcpkg/pull/29568. Once that is complete, we can look at integrating any changes as necessary back.

nibanks avatar Apr 25 '23 19:04 nibanks

The main changes to CMake are:

  • Use include(GNUInstallDirs) to get CMAKE_INSTALL_INCLUDEDIR, CMAKE_INSTALL_BINDIR, and CMAKE_INSTALL_LIBDIR instead of hard-coding the install paths.

  • Update your processor selection logic to check VCPKG_TARGET_ARCHITECTURE if defined. I'd recommend something like:

if(DEFINED VCPKG_TARGET_ARCHITECTURE)
    set(MSQUIC_ARCH ${VCPKG_TARGET_ARCHITECTURE})
elseif(CMAKE_GENERATOR_PLATFORM MATCHES "^[Ww][Ii][Nn]32$")
    set(MSQUIC_ARCH x86)
elseif(CMAKE_GENERATOR_PLATFORM MATCHES "^[Xx]64$")
    set(MSQUIC_ARCH x64)
elseif(CMAKE_GENERATOR_PLATFORM MATCHES "^[Aa][Rr][Mm]$")
    set(MSQUIC_ARCH arm)
elseif(CMAKE_GENERATOR_PLATFORM MATCHES "^[Aa][Rr][Mm]64$")
    set(MSQUIC_ARCH arm64)
else()
    # Use SYSTEM_PROCESSOR logic otherwise
endif()

# Use ${MSQUIC_ARCH} to set the appropriate values for QUIC_OPENSSL_WIN_ARCH.

walbourn avatar Apr 25 '23 23:04 walbourn

@walbourn we'd appreciate if you could make the PR to fix!

nibanks avatar Apr 26 '23 11:04 nibanks