From bfd9d0cd39404d64659630292454fb3dc96ac84b Mon Sep 17 00:00:00 2001 From: Kris Kwiatkowski Date: Sat, 19 Jun 2021 01:11:49 +0100 Subject: [PATCH] Redesign CMakeLists.txt for MemorySanitizer The test programs use googletest and google-benchmark libraries in order to ensure right level of optimizations and proper unit testing. Those two libraries are written in C++ and they use C++ standard library. If you want MemorySanitizer to work properly and not produce any false positives, you must ensure that all the code in your program and in libraries it uses is instrumented. That includes C++ standard library. (see here: https://github.com/google/sanitizers/wiki/MemorySanitizerLibcxxHowTo) With this change, the Memory Sanitizer build (enabled by -DMEMSAN=1) will also build MSan-instrumented libc++ from LLVM and will use it as a standard C++ library when building unit tests and benchmarks. In particular what I do is this: 1. Clone LLVM project and build libcxx and libcxxabi with MSan enabled 2. Build GTEST and GBENCH with -fsanitize=memory and -stdlib=libc++. Additionally link against -lc++abi 3. Then use this special version of libc++ and GTEST/GBENCH in order to build final binaries containing unit/benchmark tests The actuall tests with memory sanitizer are disabled, as I'm getting some errors which need to be investigated first. Additionally I've splitted single build into multiple, for release,debug,clang,gcc and AddressSanitizer. On unrelated note, I've also added flags to ignore some errors which I'm getting when using newer GCC (see GH#10 GH#11). --- .github/workflows/main.yml | 23 +++++-- CMakeLists.txt | 133 +++++++++++++++++++++++-------------- test/bench/CMakeLists.txt | 3 + test/bench/kyber.cc | 4 +- 4 files changed, 104 insertions(+), 59 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2df860ff..c0edecfc 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,7 +22,6 @@ jobs: gcc-debug-build, clang-debug-build, clang-release-asan-build, - clang-release-msan-build, ] include: @@ -46,10 +45,6 @@ jobs: cc: clang cxx: clang++ flags: -DCMAKE_BUILD_TYPE=Release -DADDRSAN=1 - - name: clang-release-msan-build - cc: clang - cxx: clang++ - flags: -DCMAKE_BUILD_TYPE=Release -DMEMSAN=1 steps: - uses: actions/checkout@v1 with: @@ -81,7 +76,7 @@ jobs: make - name: run tests run: | - cd build && ./ut + cd build && ./ut ${FILTER_TESTS} - name: Build Rust bindings run: | cd src/rustapi/pqc-sys && cargo build @@ -91,3 +86,19 @@ jobs: curl http://amongbytes.com/~flowher/permalinks/kat.zip --output kat.zip unzip kat.zip cargo run --release -- --katdir KAT +# MEMSAN: +# name: Memory Sanitizer build +# runs-on: [ubuntu-20.04] +# steps: +# - uses: actions/checkout@v1 +# with: +# submodules: true +# - name: build +# run: | +# mkdir -p build +# cd build +# CC=clang CXX=clang++ cmake -DCMAKE_BUILD_TYPE=Release -DMEMSAN=1 .. +# make +# - name: run tests +# run: | +# cd build && ./ut --gtest_filter="NONE" diff --git a/CMakeLists.txt b/CMakeLists.txt index 796f4e6d..46ef593d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,7 @@ cmake_minimum_required(VERSION 3.13) project(cryptocore VERSION 0.0.1 LANGUAGES C) include(FetchContent) +include(ExternalProject) set(CMAKE_CXX_STANDARD 20) set(CMAKE_C_STANDARD 99) @@ -10,33 +11,6 @@ enable_language(C) enable_language(CXX) enable_language(ASM) -# Dependencies -FetchContent_Declare( - gtest - SOURCE_DIR ${PROJECT_SOURCE_DIR}/3rd/gtest - GIT_REPOSITORY https://github.com/google/googletest.git - GIT_TAG a3460d1aeeaa43fdf137a6adefef10ba0b59fe4b -) -FetchContent_Populate(gtest) - -FetchContent_Declare( - cpu_features - SOURCE_DIR ${PROJECT_SOURCE_DIR}/3rd/cpu_features - GIT_REPOSITORY https://github.com/google/cpu_features.git - GIT_TAG bc2846e78faeb26b8a46c17df369d4e5f1f9e2bb - GIT_SHALLOW TRUE -) -FetchContent_Populate(cpu_features) - -FetchContent_Declare( - gbench - SOURCE_DIR ${PROJECT_SOURCE_DIR}/3rd/gbench - GIT_REPOSITORY https://github.com/kriskwiatkowski/benchmark.git - GIT_TAG 49862ab56b6b7c3afd87b80bd5d787ed78ce3b96 - GIT_SHALLOW TRUE -) -FetchContent_Populate(gbench) - if(MEMSAN) # PQC_MEMSAN enables usage of some internals from clang if (NOT CMAKE_C_COMPILER_ID MATCHES "Clang") @@ -46,10 +20,49 @@ if(MEMSAN) message(FATAL_ERROR "Can't use MSAN and ASAN") endif() include(.cmake/libstd-memcheck.mk) - #set(C_CXX_FLAGS "${C_CXX_FLAGS} -DPQC_MEMSAN=1 -fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer") + + # LLVM project location + set(LLVM_PRJ ${CMAKE_CURRENT_BINARY_DIR}/3rd/llvm-project) + set(LLVM_PRJ_LIB ${LLVM_PRJ}/usr/local/lib) + set(LLVM_PRJ_INC ${LLVM_PRJ}/usr/local/include) + + # Add memory sanitizer instrumented libraries + set(CMAKE_ARGS_MEMCHECK_LIB "-stdlib=libc++ -L${LLVM_PRJ_LIB} -lc++abi -Wl,-rpath,${LLVM_PRJ_LIB}") + set(CMAKE_ARGS_MEMCHECK_INC "-isystem -I${LLVM_PRJ_INC} -I${LLVM_PRJ_INC}/c++/v1") + set(CMAKE_ARGS_MEMCHECK_FLAGS "-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -Wno-unused-command-line-argument") + set(EXTRA_CXX_FLAGS "${CMAKE_ARGS_MEMCHECK_FLAGS} ${CMAKE_ARGS_MEMCHECK_LIB} ${CMAKE_ARGS_MEMCHECK_INC}") endif() -add_subdirectory(3rd/gtest) +# Dependencies +ExternalProject_Add( + gtest_project + SOURCE_DIR ${PROJECT_SOURCE_DIR}/3rd/gtest + GIT_REPOSITORY https://github.com/google/googletest.git + GIT_TAG a3460d1aeeaa43fdf137a6adefef10ba0b59fe4b + PREFIX ${CMAKE_CURRENT_BINARY_DIR}/3rd/gtest + INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/3rd/gtest + CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/3rd/gtest -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} -DCMAKE_CXX_FLAGS=${EXTRA_CXX_FLAGS} -DCMAKE_C_FLAGS=${EXTRA_CXX_FLAGS} -Dgtest_disable_pthreads=ON +) + +FetchContent_Declare( + gbench + SOURCE_DIR ${PROJECT_SOURCE_DIR}/3rd/gbench + GIT_REPOSITORY https://github.com/kriskwiatkowski/benchmark.git + GIT_TAG 49862ab56b6b7c3afd87b80bd5d787ed78ce3b96 + GIT_SHALLOW TRUE +) +FetchContent_Populate(gbench) + +FetchContent_Declare( + cpu_features + SOURCE_DIR ${PROJECT_SOURCE_DIR}/3rd/cpu_features + GIT_REPOSITORY https://github.com/google/cpu_features.git + GIT_TAG bc2846e78faeb26b8a46c17df369d4e5f1f9e2bb + GIT_SHALLOW TRUE +) +FetchContent_Populate(cpu_features) + + set(BUILD_PIC ON CACHE BOOL "") add_subdirectory(3rd/cpu_features) @@ -81,20 +94,6 @@ else() message(FATAL_ERROR "Unknown processor:" ${CMAKE_SYSTEM_PROCESSOR}) endif() -if(NOT CMAKE_BUILD_TYPE_LOWER STREQUAL "debug") - # settings below are required by benchmark library - set(CMAKE_BUILD_TYPE "Release" CACHE STRING "" FORCE) - # Target for benchmark - it also builds gtest library - set(BENCHMARK_ENABLE_GTEST_TESTS ON CACHE BOOL "Enable testing of the benchmark library." FORCE) - set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "Disable benchmark tests" FORCE) - set(GOOGLETEST_PATH "${CMAKE_SOURCE_DIR}/3rd/gtest" CACHE PATH "Path to the gtest sources" FORCE) - #if (NOT MACOSX) - # set(BENCHMARK_ENABLE_LTO ON CACHE BOOL "Enable link time optim" FORCE) - #endif() - set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) - add_subdirectory(${CMAKE_SOURCE_DIR}/3rd/gbench) -endif() - # Arch settings if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin") @@ -103,8 +102,7 @@ endif() if(CMAKE_C_COMPILER_ID MATCHES "Clang") # Additional flags only useful when compiling with clang - set(C_CXX_FLAGS - "-Wconditional-uninitialized -Wno-missing-variable-declarations -Wno-unused-command-line-argument") + string(APPEND C_CXX_FLAGS " -Wconditional-uninitialized -Wno-missing-variable-declarations -Wno-unused-command-line-argument") endif() if (MACOSX) @@ -121,9 +119,17 @@ set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-ignored-qualifiers \ -Wpedantic \ -Wshadow \ -Wno-variadic-macros \ - -Wundef \ -Wunused-result \ - -Wno-unused-command-line-argument") + -Wno-unused-command-line-argument \ + -Wno-undef \ + ${EXTRA_CXX_FLAGS}") + +if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_C_COMPILER_VERSION VERSION_GREATER 11.0) +set(C_CXX_FLAGS "${C_CXX_FLAGS} \ + -Wno-stringop-overread \ + -Wno-stringop-overflow \ + -Wno-array-parameter") +endif() # Build with address sanitizer if(ADDRSAN) @@ -157,8 +163,8 @@ if(${ARCH} STREQUAL "ARCH_x86_64") string(APPEND C_CXX_FLAGS " -march=haswell") endif() -set(PQC_CMAKE_C_FLAGS "${C_CXX_FLAGS}") -set(PQC_CMAKE_CXX_FLAGS "${C_CXX_FLAGS}") +set(PQC_CMAKE_C_FLAGS "${PQC_CMAKE_C_FLAGS} ${C_CXX_FLAGS}") +set(PQC_CMAKE_CXX_FLAGS "${PQC_CMAKE_CXX_FLAGS} ${C_CXX_FLAGS}") set(CMAKE_C_FLAGS ${PQC_CMAKE_C_FLAGS}) set(CMAKE_CXX_FLAGS ${PQC_CMAKE_CXX_FLAGS}) @@ -341,16 +347,41 @@ add_executable( target_link_libraries( ut + gtest gtest_main pqc_s) +ExternalProject_Get_Property(gtest_project INSTALL_DIR) target_include_directories( ut PRIVATE - ${CMAKE_SOURCE_DIR}) + + ${CMAKE_SOURCE_DIR} + ${INSTALL_DIR}/include) + +target_link_directories( + ut + PRIVATE + ${INSTALL_DIR}/lib) + +# github CI requires that +add_dependencies(ut gtest_project) if(NOT CMAKE_BUILD_TYPE_LOWER STREQUAL "debug") -add_subdirectory(test/bench) + # settings below are required by benchmark library + set(CMAKE_BUILD_TYPE "Release" CACHE STRING "" FORCE) + # Target for benchmark - it also builds gtest library + set(BENCHMARK_ENABLE_GTEST_TESTS ON CACHE BOOL "Enable testing of the benchmark library." FORCE) + set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "Disable benchmark tests" FORCE) + set(GOOGLETEST_PATH "${CMAKE_SOURCE_DIR}/3rd/gtest" CACHE PATH "Path to the gtest sources" FORCE) + #if (NOT MACOSX) + # set(BENCHMARK_ENABLE_LTO ON CACHE BOOL "Enable link time optim" FORCE) + #endif() + set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) + set(CMAKE_C_FLAGS "${C_CXX_FLAGS} ${EXTRA_CXX_FLAGS}") + set(CMAKE_CXX_FLAGS "${C_CXX_FLAGS} ${EXTRA_CXX_FLAGS}") + add_subdirectory(${CMAKE_SOURCE_DIR}/3rd/gbench) + add_subdirectory(test/bench) endif() install(TARGETS pqc pqc_s diff --git a/test/bench/CMakeLists.txt b/test/bench/CMakeLists.txt index 64fa5ec0..83494650 100644 --- a/test/bench/CMakeLists.txt +++ b/test/bench/CMakeLists.txt @@ -1,5 +1,8 @@ cmake_minimum_required(VERSION 3.13) +include_directories(${PROJECT_SOURCE_DIR}/3rd/gbench/include) +link_directories(${CMAKE_CURRENT_BINARY_DIR}/../../3rd/gbench/lib) + add_executable( bench kyber.cc diff --git a/test/bench/kyber.cc b/test/bench/kyber.cc index 6657b4fb..07fdd762 100644 --- a/test/bench/kyber.cc +++ b/test/bench/kyber.cc @@ -34,8 +34,8 @@ static void BenchKyberMatK2(benchmark::State &st) { static void BenchKyberRejSampling(benchmark::State &st) { int64_t t, total = 0; - int16_t a[256]; - uint8_t buf[168*3]; + int16_t a[256] = {0}; + uint8_t buf[168*3] = {0}; for (auto _ : st) { t = benchmark::cycleclock::Now(); PQCLEAN_KYBER512_AVX2_rej_uniform_avx(a, buf);