MSVC sanitizer build

.github/workflows/windows-native.yml

@@ -63,7 +63,7 @@ env:
 
 jobs:
   build_and_test:
-    name: Windows-2019 [arch ${{ matrix.arch.name }}, toolset ${{ matrix.toolset }}, backend ${{ matrix.backend }}, build shared libs ${{ matrix.shared_libs }}, use CMake prefix path ${{ matrix.use_cmake_prefix_path }}]
+    name: Windows-2019 [arch ${{ matrix.arch.name }}, toolset ${{ matrix.toolset }}, backend ${{ matrix.backend }}, build shared libs ${{ matrix.shared_libs }}, use CMake prefix path ${{ matrix.use_cmake_prefix_path }}, sanitizers ${{ matrix.sanitizers }}]
     runs-on: windows-2019
     if: "!contains(github.event.head_commit.message, 'skip ci')"
     strategy:
@@ -74,17 +74,26 @@ jobs:
         backend: [ 'botan', 'openssl' ]
         shared_libs: [ 'off']
         use_cmake_prefix_path: [ 'on', 'off' ]
+        sanitizers: [ 'off' ]
         include:
+          - arch:  { name: 'x64',    triplet: 'x64-windows' }
+            toolset:                'v142'
+            backend:                'botan'
+            use_cmake_prefix_path:  'off'
+            shared_libs:            'off'
+            sanitizers:             'on'
           - arch:  { name: 'Win32',  triplet: 'x86-windows' }
             toolset:                'ClangCL'
             backend:                'botan'
             use_cmake_prefix_path:  'on'
             shared_libs:            'off'
+            sanitizers:             'off'
           - arch:  { name: 'Win32',  triplet: 'x86-windows' }
             toolset:                'v142'
             backend:                'openssl'
             use_cmake_prefix_path:  'off'
             shared_libs:            'off'
+            sanitizers:             'off'
 
     steps:
       - name: Checkout
@@ -139,8 +148,9 @@ jobs:
           cmake -B build   -G "Visual Studio 16 2019"  \
                            -A ${{ matrix.arch.name }}  \
                            -T ${{ matrix.toolset }}    \
-                           -DBUILD_SHARED_LIBS=${{ matrix.shared_libs}} \
-                           -DCRYPTO_BACKEND=${{ matrix.backend }}      \
+                           -DBUILD_SHARED_LIBS=${{ matrix.shared_libs}}            \
+                           -DENABLE_SANITIZERS=${{ matrix.sanitizers }} \
+                           -DCRYPTO_BACKEND=${{ matrix.backend }}                  \
                            -DCMAKE_TOOLCHAIN_FILE=${{ env.VCPKG_DIR }}/scripts/buildsystems/vcpkg.cmake .
 
       - name: Configure using CMake prefix path
@@ -152,15 +162,16 @@ jobs:
           cmake -B build   -G "Visual Studio 16 2019"  \
                            -A ${{ matrix.arch.name }}  \
                            -T ${{ matrix.toolset }}    \
-                           -DBUILD_SHARED_LIBS=${{ matrix.shared_libs}} \
-                           -DCRYPTO_BACKEND=${{ matrix.backend }}      \
+                           -DBUILD_SHARED_LIBS=${{ matrix.shared_libs}}            \
+                           -DENABLE_SANITIZERS=${{ matrix.sanitizers }} \
+                           -DCRYPTO_BACKEND=${{ matrix.backend }}                  \
                            -DCMAKE_PREFIX_PATH=${{ env.VCPKG_DIR }}/installed/${{ matrix.arch.triplet }} .
 
       - name: Build
         shell: bash
         run: |
           export PATH=${{ env.VCPKG_DIR_U }}/installed/${{ matrix.arch.triplet }}/bin:$PATH
-          cmake --build build --config "Release" --parallel ${{ env.CORES }}
+          cmake --build build --parallel ${{ env.CORES }} --config Release
 
       - name: Test
         shell: bash

CMakeLists.txt

@@ -128,8 +128,16 @@ else()
   message(FATAL_ERROR "Invalid crypto backend: ${CRYPTO_BACKEND}")
 endif()
 
-# set warning flags at the top level
-if(NOT MSVC)
+if(MSVC)
+  # This works both for MSVC and CL on Windows
+  # Recent version of MSVC toolset issues c++17 deprecation warning even if we use /std:c++11
+  add_compile_definitions(
+    _CRT_SECURE_NO_WARNINGS
+    _CRT_NONSTDC_NO_DEPRECATE
+    _SILENCE_CXX17_C_HEADER_DEPRECATION_WARNING
+  )
+else(MSVC)
+  # set warning flags at the top level
   add_compile_options(
     -Wall -Wextra
     -Wunreachable-code -Wpointer-arith
@@ -142,12 +150,7 @@ if(NOT MSVC)
     -Wno-unused-parameter
     -Wno-missing-field-initializers
   )
-endif(NOT MSVC)
-
-# This works both for MSVC and CL on Windows
-if(WIN32)
-  add_compile_definitions(_CRT_SECURE_NO_WARNINGS _CRT_NONSTDC_NO_DEPRECATE)
-endif(WIN32)
+endif(MSVC)
 
 # set a few other things at the top level to prevent incompatibilities
 set(CMAKE_C_STANDARD 99)
@@ -156,28 +159,32 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 add_definitions(-D_GNU_SOURCE)
 
-if (ENABLE_COVERAGE OR ENABLE_SANITIZERS)
+if((ENABLE_COVERAGE OR ENABLE_SANITIZERS) AND NOT GENERATOR_IS_MULTI_CONFIG)
   message("Forcing build type to Debug (for code coverage or sanitizers).")
   set(CMAKE_BUILD_TYPE Debug CACHE STRING "Build type. Forced to Debug." FORCE)
-endif()
+endif((ENABLE_COVERAGE OR ENABLE_SANITIZERS) AND NOT GENERATOR_IS_MULTI_CONFIG)
 
 # coverage
-if (ENABLE_COVERAGE)
-  if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+if(ENABLE_COVERAGE)
+  if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
     message(FATAL_ERROR "Coverage has only been tested with the GNU compiler.")
   endif()
   add_compile_options(--coverage -O0)
   link_libraries(--coverage)
-endif()
+endif(ENABLE_COVERAGE)
 
 # sanitizers
-if (ENABLE_SANITIZERS)
-  if (NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-    message(FATAL_ERROR "Sanitizers have only been tested with the clang compiler.")
+if(ENABLE_SANITIZERS)
+  if(MSVC)
+    add_compile_options(/fsanitize=address /Zi "$<$<CONFIG:Release>:/MT>" "$<$<CONFIG:Debug>:/MTd>")
+    add_link_options(/INCREMENTAL:NO /DEBUG)
+  elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+    add_compile_options(-fsanitize=leak,address,undefined -fno-omit-frame-pointer -fno-common -O1)
+    link_libraries(-fsanitize=leak,address,undefined)
+  else()
+    message(FATAL_ERROR "Sanitizers have only been tested with the Clang compiler or Microsoft Visual Studio.")
   endif()
-  add_compile_options(-fsanitize=leak,address,undefined -fno-omit-frame-pointer -fno-common -O1)
-  link_libraries(-fsanitize=leak,address,undefined)
-endif()
+endif(ENABLE_SANITIZERS)
 
 # adoc for man generation
 if (ENABLE_DOC)

docs/installation.adoc

@@ -177,6 +177,11 @@ make && make install
 
 === Using Microsoft Visual Studio 2019 and vcpkg
 
+Use appropriate command propmpt for your target platform, for example "x64 Native Tools Command Prompt for VS 2019"
+```
+cmd /k "C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvars64.bat"
+```
+
 Install `vcpkg` according to
 https://docs.microsoft.com/en-us/cpp/build/install-vcpkg?view=msvc-160&tabs=windows[these instructions]:
 
@@ -203,8 +208,8 @@ If you need to target 32-bit platform you'll need to to replace `x64-windows` wi
 ----
 cmake -B build -G "Visual Studio 16 2019" -A x64 -DCMAKE_TOOLCHAIN_FILE=%VCPKG_ROOT%\scripts\buildsystems\vcpkg.cmake \
       -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=off -DCRYPTO_BACKEND="botan" .
-cmake --build . --config Release
-cmake --install .
+cmake --build build --config Release
+cmake --install build
 ----
 --
 Replace CRYPTO_BACKEND parameter to "openssl" if you target this backend.
@@ -220,6 +225,11 @@ Ensure that the following dependencies are available on path:
 
 === Using Microsoft Visual Studio 2019 and pre-installed libraries
 
+Use appropriate command propmpt for your target platform, for example "x64 Native Tools Command Prompt for VS 2019"
+```
+cmd /k "C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvars64.bat"
+```
+
 Install dependencies and make them available either on PATH or using CMAKE_TARGET_PREFIX parameter:
 
 * Botan(2.14+) or Crypto (OpenSSL 1.1.1+) depending on target backend
@@ -238,8 +248,8 @@ In such case use OPENSSL_ROOT_DIR.
 ----
 cmake -B build -G "Visual Studio 16 2019" -A x64 -DOPENSSL_ROOT_DIR=<openssl root> -DCMAKE_TARGET_PREFIX=<target prefix> \
       -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=off -DCRYPTO_BACKEND="botan" .
-cmake --build . --config Release
-cmake --install .
+cmake --build build --config Release
+cmake --install build
 ----
 --
 Replace CRYPTO_BACKEND parameter to "openssl" if you target this backend, use OPENSSL_ROOT_DIR and CMAKE_TARGET_PREFIX optionally as explained above

src/lib/config.h.in

@@ -76,7 +76,7 @@
 
 /* do not use the statement for old MSVC versions */
 #if (!defined(_MSVC_LANG) || _MSVC_LANG >= 201703L)
-# define FALLTHROUGH_STATEMENT [[fallthrough]];
+# define FALLTHROUGH_STATEMENT [[fallthrough]]
 #else
 # define FALLTHROUGH_STATEMENT
-#endif
+#endif

src/tests/CMakeLists.txt

@@ -150,11 +150,11 @@ set(RNP_TEST_SOURCES
   )
 
 if(ENABLE_CRYPTO_REFRESH)
-  list(APPEND RNP_TEST_SOURCES 
+  list(APPEND RNP_TEST_SOURCES
     hkdf.cpp)
 endif()
 if(ENABLE_PQC)
-  list(APPEND RNP_TEST_SOURCES 
+  list(APPEND RNP_TEST_SOURCES
     pqc.cpp)
 endif()
 
@@ -163,7 +163,7 @@ add_executable(rnp_tests ${RNP_TEST_SOURCES})
 if(MSVC)
   find_package(WindowsSDK)
   GetUMWindowsSDKLibraryDir(WIN_LIBRARY_DIR)
-  message (STATUS "WIN_LIBRARY_DIR: ${WIN_LIBRARY_DIR}")
+  message (STATUS "Using Windows SDK library directory: ${WIN_LIBRARY_DIR}")
   find_library(SHLWAPI_LIBRARY
     PATHS
        ${WIN_LIBRARY_DIR}

src/tests/cli.cpp

@@ -838,7 +838,13 @@ TEST_F(rnp_tests, test_cli_dump)
 
 TEST_F(rnp_tests, test_cli_logname)
 {
+    // getenv function is not required to be thread-safe.
+    // Another call to getenv, as well as a call to the POSIX functions setenv(), unsetenv(),
+    // and putenv() may invalidate the pointer returned by a previous call or modify the string
+    // obtained from a previous call.
     char *      logname = getenv("LOGNAME");
+    std::string saved_logname(logname ? logname : "");
+
     char *      user = getenv("USER");
     std::string testname(user ? user : "user");
     testname.append("-test-user");
@@ -851,7 +857,7 @@ TEST_F(rnp_tests, test_cli_logname)
     }
 
     if (logname) {
-        setenv("LOGNAME", logname, 1);
+        setenv("LOGNAME", saved_logname.c_str(), 1);
     } else {
         unsetenv("LOGNAME");
     }