From 7e3f16162f4121c90e50f4ba9f63009c755ad349 Mon Sep 17 00:00:00 2001 From: iphydf Date: Tue, 3 Dec 2024 23:09:12 +0000 Subject: [PATCH] chore: Add UBSAN option for cmake. To enable undefined behavior sanitizer. --- .ci-scripts/build-qtox-linux.sh | 11 ++-- .ci-scripts/build-qtox-macos.sh | 3 +- CMakeLists.txt | 81 +++++++++++++++++++-------- CMakePresets.json | 14 +++++ cmake/CheckAtomic.cmake | 97 --------------------------------- src/nexus.cpp | 3 +- test/net/bsu_test.cpp | 2 +- 7 files changed, 83 insertions(+), 128 deletions(-) delete mode 100644 cmake/CheckAtomic.cmake diff --git a/.ci-scripts/build-qtox-linux.sh b/.ci-scripts/build-qtox-linux.sh index 8001eb4062..168a8e8981 100755 --- a/.ci-scripts/build-qtox-linux.sh +++ b/.ci-scripts/build-qtox-linux.sh @@ -10,6 +10,7 @@ usage() { echo "$0 [--minimal|--full] --build-type [Debug|Release] [--with-gui-tests] [--sanitize] [--tidy]" echo "Build script to build/test qtox from a CI environment." echo "--minimal or --full are required, --build-type is required." + echo "UndefinedBehaviorSanitizer is always enabled. In Release builds, it is used without additional runtime dependencies." } while (($# > 0)); do @@ -84,19 +85,21 @@ export QT_QPA_PLATFORM=offscreen if [ "$MINIMAL" -eq 1 ]; then cmake "$SRCDIR" \ -DCMAKE_BUILD_TYPE="$BUILD_TYPE" \ + -DSTRICT_OPTIONS=ON \ + -DUBSAN=ON \ + -GNinja \ -DSMILEYS=DISABLED \ -DUPDATE_CHECK=OFF \ - -DSTRICT_OPTIONS=ON \ -DSPELL_CHECK=OFF \ - -GNinja \ "${CMAKE_ARGS[@]}" else cmake "$SRCDIR" \ -DCMAKE_BUILD_TYPE="$BUILD_TYPE" \ - -DUPDATE_CHECK=ON \ -DSTRICT_OPTIONS=ON \ - -DCODE_COVERAGE=ON \ + -DUBSAN=ON \ -GNinja \ + -DCODE_COVERAGE=ON \ + -DUPDATE_CHECK=ON \ "${CMAKE_ARGS[@]}" fi diff --git a/.ci-scripts/build-qtox-macos.sh b/.ci-scripts/build-qtox-macos.sh index c2eec54f48..8627d62072 100755 --- a/.ci-scripts/build-qtox-macos.sh +++ b/.ci-scripts/build-qtox-macos.sh @@ -35,6 +35,7 @@ build_qtox() { # CMake will use -I instead of -isystem, so we need to set it manually. cmake \ -DCMAKE_CXX_FLAGS="-isystem/usr/local/include" \ + -DUBSAN=ON \ -DUPDATE_CHECK=ON \ -DSPELL_CHECK=OFF \ -DSTRICT_OPTIONS=ON \ @@ -45,7 +46,7 @@ build_qtox() { . cmake --build . ctest --output-on-failure --parallel "$(sysctl -n hw.ncpu)" - cmake --build . --target install + cmake --install . cp qTox.dmg "$BIN_NAME" } diff --git a/CMakeLists.txt b/CMakeLists.txt index 5eaa18dbb5..3f28584645 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,9 @@ # # ############################################################################## -cmake_minimum_required(VERSION 3.10) +cmake_minimum_required(VERSION 3.16) +cmake_policy(VERSION 3.16) + set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake) option(PLATFORM_EXTENSIONS @@ -21,15 +23,11 @@ option(SPELL_CHECK "Enable spellcheck support" ON) option(SVGZ_ICON "Compress the SVG icon of qTox" ON) option(ASAN "Compile with AddressSanitizer" OFF) option(TSAN "Compile with ThreadSanitizer" OFF) +option(UBSAN "Compile with UndefinedBehaviorSanitizer" OFF) option(STRICT_OPTIONS "Error on compile warning, used by CI" OFF) option(GUI_TESTS "Enable GUI rendering tests, likely tied to a specific Qt version" OFF) -# process generated files if cmake >= 3.10 -if(POLICY CMP0071) - cmake_policy(SET CMP0071 NEW) -endif() - if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Debug @@ -38,20 +36,6 @@ if(NOT CMAKE_BUILD_TYPE) FORCE) endif() -if(ASAN) - set(CMAKE_CXX_FLAGS_DEBUG - "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address") - set(CMAKE_LINKER_FLAGS_DEBUG - "${CMAKE_LINKER_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address") -endif() - -if(TSAN) - set(CMAKE_CXX_FLAGS_DEBUG - "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=thread") - set(CMAKE_LINKER_FLAGS_DEBUG - "${CMAKE_LINKER_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=thread") -endif() - set(ENV{PKG_CONFIG_PATH} ${CMAKE_SOURCE_DIR}/libs/lib/pkgconfig:/opt/ffmpeg/lib/pkgconfig:$ENV{PKG_CONFIG_PATH} ) @@ -72,6 +56,58 @@ execute_process( project(qtox) +# C++ and C standards. +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_C_STANDARD 11) + +if(ASAN + OR TSAN + OR UBSAN) + set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer") +endif() + +if(ASAN) + set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address") + set(CMAKE_LINKER_FLAGS_DEBUG "${CMAKE_LINKER_FLAGS_DEBUG} -fsanitize=address") +endif() + +if(TSAN) + if(ASAN) + message(FATAL_ERROR "ASAN and TSAN cannot be enabled at the same time") + endif() + set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=thread") + set(CMAKE_LINKER_FLAGS_DEBUG "${CMAKE_LINKER_FLAGS_DEBUG} -fsanitize=thread") +endif() + +if(UBSAN) + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + # Add these to common flags (release and debug), because we can use the + # runtime-less version via -fsanitize-trap=all below. + set(CMAKE_CXX_FLAGS + "${CMAKE_CXX_FLAGS} -fsanitize=undefined,nullability,local-bounds,float-divide-by-zero" + ) + set(CMAKE_LINKER_FLAGS + "${CMAKE_LINKER_FLAGS} -fsanitize=undefined,nullability,local-bounds,float-divide-by-zero" + ) + # In release, reduce attack surface by never calling into a runtime library, + # and instead just aborting the program on UB. + set(CMAKE_CXX_FLAGS_RELEASE + "${CMAKE_CXX_FLAGS_RELEASE} -fsanitize-trap=all") + set(CMAKE_LINKER_FLAGS_RELEASE + "${CMAKE_LINKER_FLAGS_RELEASE} -fsanitize-trap=all") + elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined") + set(CMAKE_LINKER_FLAGS "${CMAKE_LINKER_FLAGS} -fsanitize=undefined") + # This is the GCC version of -fsanitize-trap=all. + set(CMAKE_CXX_FLAGS_RELEASE + "${CMAKE_CXX_FLAGS_RELEASE} -fno-sanitize-recover=all") + else() + message( + FATAL_ERROR + "UBSAN is not supported by the compiler (${CMAKE_CXX_COMPILER_ID})") + endif() +endif() + # Instruct CMake to run moc automatically when needed. set(CMAKE_AUTOMOC ON) # Run resource compilation automatically @@ -84,8 +120,7 @@ set(AUTORCC_OPTIONS -compress 9 -threshold 0) # unwanted side effects. set(AUTORCC_OPTIONS ${AUTORCC_OPTIONS} -format-version 1) -# Use C++20. -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20") +# Disable exceptions (Qt doesn't use them, we don't need them). set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions") # Hardening flags (ASLR, warnings, etc) @@ -104,8 +139,6 @@ add_definitions(-DQT_NO_CAST_FROM_BYTEARRAY) add_definitions(-DQT_NO_CAST_TO_ASCII) add_definitions(-DQT_RESTRICTED_CAST_FROM_ASCII) -include(CheckAtomic) - # Use ccache when available to speed up builds. if(USE_CCACHE) find_program(CCACHE_FOUND ccache) diff --git a/CMakePresets.json b/CMakePresets.json index ffb3e9f688..ad0d72132a 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -7,6 +7,20 @@ "generator": "Ninja", "cacheVariables": { "ASAN": true, + "UBSAN": true, + "SPELL_CHECK": true, + "STRICT_OPTIONS": true, + "CMAKE_BUILD_TYPE": "Debug", + "CMAKE_COMPILE_WARNING_AS_ERROR": true, + "CMAKE_EXPORT_COMPILE_COMMANDS": true + } + }, + { + "name": "debug-tsan", + "binaryDir": "${sourceDir}/_build-tsan", + "generator": "Ninja", + "cacheVariables": { + "TSAN": true, "SPELL_CHECK": true, "STRICT_OPTIONS": true, "CMAKE_BUILD_TYPE": "Debug", diff --git a/cmake/CheckAtomic.cmake b/cmake/CheckAtomic.cmake deleted file mode 100644 index 4feba8352a..0000000000 --- a/cmake/CheckAtomic.cmake +++ /dev/null @@ -1,97 +0,0 @@ -# SPDX-License-Identifier: GPL-3.0-or-later -# Copyright © 2019 by The qTox Project Contributors -# Copyright © 2024 The TokTok team. - -# atomic builtins are required for threading support. - -INCLUDE(CheckCXXSourceCompiles) -INCLUDE(CheckLibraryExists) - -# Sometimes linking against libatomic is required for atomic ops, if -# the platform doesn't support lock-free atomics. - -function(check_working_cxx_atomics varname) - set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) - set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++11") - CHECK_CXX_SOURCE_COMPILES(" -#include -std::atomic x; -int main() { - return x; -} -" ${varname}) - set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS}) -endfunction(check_working_cxx_atomics) - -function(check_working_cxx_atomics64 varname) - set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) - set(CMAKE_REQUIRED_FLAGS "-std=c++11 ${CMAKE_REQUIRED_FLAGS}") - CHECK_CXX_SOURCE_COMPILES(" -#include -#include -std::atomic x (0); -int main() { - uint64_t i = x.load(std::memory_order_relaxed); - return 0; -} -" ${varname}) - set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS}) -endfunction(check_working_cxx_atomics64) - -# Determine if the compiler has GCC-compatible command-line syntax. - -if(NOT DEFINED COMPILER_IS_GCC_COMPATIBLE) - if(CMAKE_COMPILER_IS_GNUCXX) - set(COMPILER_IS_GCC_COMPATIBLE ON) - elseif( MSVC ) - set(COMPILER_IS_GCC_COMPATIBLE OFF) - elseif( "${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" ) - set(COMPILER_IS_GCC_COMPATIBLE ON) - elseif( "${CMAKE_CXX_COMPILER_ID}" MATCHES "Intel" ) - set(COMPILER_IS_GCC_COMPATIBLE ON) - else() - message(STATUS "Warning: Unknown compiler, assume GCC compatible") - set(COMPILER_IS_GCC_COMPATIBLE ON) - endif() -endif() - -# This isn't necessary on MSVC, so avoid command-line switch annoyance -# by only running on GCC-like hosts. -if (COMPILER_IS_GCC_COMPATIBLE) - # First check if atomics work without the library. - check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITHOUT_LIB) - # If not, check if the library exists, and atomics work with it. - if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB) - check_library_exists(atomic __atomic_fetch_add_4 "" HAVE_LIBATOMIC) - if( HAVE_LIBATOMIC ) - list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic") - check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITH_LIB) - if (NOT HAVE_CXX_ATOMICS_WITH_LIB) - message(FATAL_ERROR "Host compiler must support std::atomic!") - endif() - else() - message(FATAL_ERROR "Host compiler appears to require libatomic, but cannot find it.") - endif() - endif() -endif() - -# Check for 64 bit atomic operations. -if(MSVC) - set(HAVE_CXX_ATOMICS64_WITHOUT_LIB True) -else() - check_working_cxx_atomics64(HAVE_CXX_ATOMICS64_WITHOUT_LIB) -endif() - -# If not, check if the library exists, and atomics work with it. -if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB) - check_library_exists(atomic __atomic_load_8 "" HAVE_CXX_LIBATOMICS64) - if(HAVE_CXX_LIBATOMICS64) - list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic") - check_working_cxx_atomics64(HAVE_CXX_ATOMICS64_WITH_LIB) - if (NOT HAVE_CXX_ATOMICS64_WITH_LIB) - message(FATAL_ERROR "Host compiler must support std::atomic!") - endif() - else() - message(FATAL_ERROR "Host compiler appears to require libatomic, but cannot find it.") - endif() -endif() diff --git a/src/nexus.cpp b/src/nexus.cpp index a5f889dba3..b87a457756 100644 --- a/src/nexus.cpp +++ b/src/nexus.cpp @@ -358,7 +358,8 @@ void Nexus::updateWindowsArg(QWindow* closedWindow) QAction* action = windowActions->addAction(windowList[i]->title()); action->setCheckable(true); action->setChecked(windowList[i] == activeWindow); - connect(action, &QAction::triggered, [=] { onOpenWindow(windowList[i]); }); + connect(action, &QAction::triggered, this, + [this, window = windowList[i]] { onOpenWindow(window); }); windowMenu->addAction(action); dockMenu->insertAction(dockLast, action); } diff --git a/test/net/bsu_test.cpp b/test/net/bsu_test.cpp index 3dcc90f5cd..72010e51c8 100644 --- a/test/net/bsu_test.cpp +++ b/test/net/bsu_test.cpp @@ -57,5 +57,5 @@ void TestBootstrapNodesUpdater::testLocal() QVERIFY(defaultNodes.size() > 0); } -QTEST_GUILESS_MAIN(TestBootstrapNodesUpdater) +QTEST_MAIN(TestBootstrapNodesUpdater) #include "bsu_test.moc"