From a0606fbf9753231051d3b94031cdbcf85889b784 Mon Sep 17 00:00:00 2001 From: Alexey Rochev Date: Sat, 7 Sep 2024 01:21:11 +0300 Subject: [PATCH 1/3] Use consistent target_link_libraries signature --- src/CMakeLists.txt | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 83d54eec..c7cee48d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -402,15 +402,15 @@ if (BUILD_TESTING) add_executable(itemlistupdater_test itemlistupdater_test.cpp) add_test(NAME itemlistupdater_test COMMAND itemlistupdater_test) - target_link_libraries(itemlistupdater_test tremotesf_objects Qt::Test) + target_link_libraries(itemlistupdater_test PRIVATE tremotesf_objects Qt::Test) add_executable(log_test log/log_test.cpp) add_test(NAME log_test COMMAND log_test) - target_link_libraries(log_test tremotesf_objects Qt::Test) + target_link_libraries(log_test PRIVATE tremotesf_objects Qt::Test) add_executable(demangle_test log/demangle_test.cpp) add_test(NAME demangle_test COMMAND demangle_test) - target_link_libraries(demangle_test tremotesf_objects Qt::Test) + target_link_libraries(demangle_test PRIVATE tremotesf_objects Qt::Test) if (NOT TREMOTESF_WITH_HTTPLIB STREQUAL "none") include("${CMAKE_SOURCE_DIR}/cmake/FindCppHttplib.cmake") @@ -418,37 +418,37 @@ if (BUILD_TESTING) add_executable(requestrouter_test rpc/requestrouter_test.cpp) target_compile_definitions(requestrouter_test PRIVATE TEST_DATA_PATH="${CMAKE_CURRENT_SOURCE_DIR}/rpc/test-data" CPPHTTPLIB_OPENSSL_SUPPORT) add_test(NAME requestrouter_test COMMAND requestrouter_test) - target_link_libraries(requestrouter_test tremotesf_objects Qt::Test) + target_link_libraries(requestrouter_test PRIVATE tremotesf_objects Qt::Test) if (TARGET PkgConfig::httplib) - target_link_libraries(requestrouter_test PkgConfig::httplib) + target_link_libraries(requestrouter_test PRIVATE PkgConfig::httplib) else () - target_link_libraries(requestrouter_test httplib::httplib) + target_link_libraries(requestrouter_test PRIVATE httplib::httplib) endif () endif() add_executable(pathutils_test rpc/pathutils_test.cpp) add_test(NAME pathutils_test COMMAND pathutils_test) - target_link_libraries(pathutils_test tremotesf_objects Qt::Test) + target_link_libraries(pathutils_test PRIVATE tremotesf_objects Qt::Test) add_executable(tracker_test rpc/tracker_test.cpp) add_test(NAME tracker_test COMMAND tracker_test) - target_link_libraries(tracker_test tremotesf_objects Qt::Test) + target_link_libraries(tracker_test PRIVATE tremotesf_objects Qt::Test) add_executable(servers_test rpc/servers_test.cpp) add_test(NAME servers_test COMMAND servers_test) - target_link_libraries(servers_test tremotesf_objects Qt::Test) + target_link_libraries(servers_test PRIVATE tremotesf_objects Qt::Test) add_executable(coroutines_test coroutines/coroutines_test.cpp) add_test(NAME coroutines_test COMMAND coroutines_test) - target_link_libraries(coroutines_test tremotesf_objects Qt::Test) + target_link_libraries(coroutines_test PRIVATE tremotesf_objects Qt::Test) add_executable(magnetlinkparser_test magnetlinkparser_test.cpp) add_test(NAME magnetlinkparser_test COMMAND magnetlinkparser_test) - target_link_libraries(magnetlinkparser_test tremotesf_objects Qt::Test) + target_link_libraries(magnetlinkparser_test PRIVATE tremotesf_objects Qt::Test) add_executable(torrentfileparser_test torrentfileparser_test.cpp) add_test(NAME torrentfileparser_test COMMAND torrentfileparser_test WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/test-torrents") - target_link_libraries(torrentfileparser_test tremotesf_objects Qt::Test) + target_link_libraries(torrentfileparser_test PRIVATE tremotesf_objects Qt::Test) endif () set_common_options_on_targets() From 21ae6df1337eef14827f72c69d1c792cbbb046ef Mon Sep 17 00:00:00 2001 From: Alexey Rochev Date: Sat, 7 Sep 2024 01:19:57 +0300 Subject: [PATCH 2/3] Add TREMOTESF_ASAN option instead of using CXXFLAGS env variable CXXFLAGS environment variable interferes with vcpkg building dependencies, which we don't want. Add fsanitize options directly in CMake instead. --- .cirrus.yml | 2 +- .github/workflows/build-macos.yml | 3 +-- .github/workflows/build-windows-msvc.yml | 4 +--- .github/workflows/main.yml | 4 ++-- CHANGELOG.md | 1 + CMakeLists.txt | 1 + cmake/CommonOptions.cmake | 11 +++++++++++ org.equeim.Tremotesf.json | 8 ++++---- 8 files changed, 22 insertions(+), 12 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 3145e9af..46281e7a 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -29,7 +29,7 @@ build_freebsd_task: else httplib=none fi - CXXFLAGS=-fsanitize=address cmake -S . --preset base -D TREMOTESF_QT6=ON -D TREMOTESF_WITH_HTTPLIB="$httplib" + cmake -S . --preset base -D TREMOTESF_QT6=ON -D TREMOTESF_WITH_HTTPLIB="$httplib" -D TREMOTESF_ASAN=ON echo 'Building Debug' cmake --build --preset base-debug echo 'Testing Debug' diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index 3603db14..52ce44d4 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -47,10 +47,9 @@ jobs: id: build uses: equeim/action-cmake-build@v10 with: - cmake-arguments: --preset macos-${{ matrix.architecture }}-vcpkg + cmake-arguments: --preset macos-${{ matrix.architecture }}-vcpkg -D TREMOTESF_ASAN=${{ inputs.release-tag == '' && 'ON' || 'OFF' }} package: true env: - CXXFLAGS: ${{ inputs.release-tag == '' && '-fsanitize=address' || '' }} ASAN_OPTIONS: detect_leaks=0 - name: Archive packages diff --git a/.github/workflows/build-windows-msvc.yml b/.github/workflows/build-windows-msvc.yml index 0d891dc4..d04ed680 100644 --- a/.github/workflows/build-windows-msvc.yml +++ b/.github/workflows/build-windows-msvc.yml @@ -47,10 +47,8 @@ jobs: id: build uses: equeim/action-cmake-build@v10 with: - cmake-arguments: --preset ${{ inputs.build-with-msvc-clang-toolchain == 'true' && 'windows-msvc-clang-vcpkg' || 'windows-msvc-vcpkg' }} -D VCPKG_INSTALLED_DIR=${{ github.workspace }}/vcpkg_installed + cmake-arguments: --preset ${{ inputs.build-with-msvc-clang-toolchain == 'true' && 'windows-msvc-clang-vcpkg' || 'windows-msvc-vcpkg' }} -D VCPKG_INSTALLED_DIR=${{ github.workspace }}/vcpkg_installed -D TREMOTESF_ASAN=${{ inputs.release-tag == '' && inputs.build-with-msvc-clang-toolchain == 'false' && 'ON' || 'OFF' }} package: true - env: - CXXFLAGS: ${{ inputs.release-tag == '' && inputs.build-with-msvc-clang-toolchain == 'false' && '/fsanitize=address /D_DISABLE_VECTOR_ANNOTATION /D_DISABLE_STRING_ANNOTATION' || '' }} - name: Archive packages if: inputs.release-tag == '' diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 265abfb1..d3ecf743 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -208,8 +208,8 @@ jobs: run: | set -e -o pipefail echo '::group::Configuring CMake' - # Not enabling ASAN here since it causes linker crash - cmake -S . --preset base -D TREMOTESF_QT6=ON -D TREMOTESF_WITH_HTTPLIB=bundled + # ASAN + LTO causes linker crash + cmake -S . --preset base -D TREMOTESF_QT6=ON -D TREMOTESF_WITH_HTTPLIB=bundled -D TREMOTESF_ASAN=ON -D CMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE=OFF echo '::endgroup' echo '::group::Building Debug' cmake --build --preset base-debug diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b8aa198..735917eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Added - Dialog is shown when fatal error occurs on Windows +- TREMOTESF_ASAN CMake option to build with AddressSanitizer (off by default) ### Fixed - Performance regression on Windows (and potential performance improvements on other platforms) diff --git a/CMakeLists.txt b/CMakeLists.txt index ee1f897a..bd22614d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,7 @@ endif () project(tremotesf VERSION 2.7.0 LANGUAGES ${languages}) option(TREMOTESF_QT6 "Build with Qt 6" ON) +option(TREMOTESF_ASAN "Build with AddressSanitizer" OFF) set(TREMOTESF_WITH_HTTPLIB "auto" CACHE STRING "Where to find cpp-httplib dependency for unit tests. Possible values are: auto, system, bundled, none") if (NOT TREMOTESF_WITH_HTTPLIB MATCHES "^(auto|system|bundled|none)$") diff --git a/cmake/CommonOptions.cmake b/cmake/CommonOptions.cmake index a9018227..33e0cbf6 100644 --- a/cmake/CommonOptions.cmake +++ b/cmake/CommonOptions.cmake @@ -95,15 +95,25 @@ function(set_common_options_on_targets) /we4906 /w45204 ) + if (TREMOTESF_ASAN) + list(APPEND common_compile_options /fsanitize=address /D_DISABLE_VECTOR_ANNOTATION /D_DISABLE_STRING_ANNOTATION) + endif() elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") list(TRANSFORM gcc_style_warnings PREPEND "/clang:") list(APPEND common_compile_options ${gcc_style_warnings}) + if (TREMOTESF_ASAN) + message(WARNING "Ignoring TREMOTESF_ASAN=ON with clang-cl, it doesn't work out of the box") + endif() endif() else() set( common_compile_options ${gcc_style_warnings} ) + if (TREMOTESF_ASAN) + list(APPEND common_compile_options -fsanitize=address) + list(APPEND common_link_options -fsanitize=address) + endif() endif() if (DEFINED TREMOTESF_COMMON_COMPILE_OPTIONS) @@ -154,6 +164,7 @@ function(set_common_options_on_targets) target_compile_options(${target} PRIVATE ${common_compile_options}) target_compile_definitions(${target} PRIVATE ${common_compile_definitions}) target_compile_features(${target} PUBLIC ${common_public_compile_features}) + target_link_options(${target} PRIVATE ${common_link_options}) set_target_properties(${target} PROPERTIES ${common_target_properties}) endif() endforeach() diff --git a/org.equeim.Tremotesf.json b/org.equeim.Tremotesf.json index 78c49fbb..ef9f29d4 100644 --- a/org.equeim.Tremotesf.json +++ b/org.equeim.Tremotesf.json @@ -82,13 +82,13 @@ "env": { "CTEST_OUTPUT_ON_FAILURE": "1", "ASAN_OPTIONS": "detect_leaks=0" - }, - "cxxflags": "-fsanitize=address" + } }, "config-opts": [ "-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON", - "-DTREMOTESF_WITH_HTTPLIB=system" + "-DTREMOTESF_WITH_HTTPLIB=system", + "-DTREMOTESF_ASAN=ON" ], "run-tests": true, "sources": [ @@ -99,4 +99,4 @@ ] } ] -} +} \ No newline at end of file From 55c7f485a5dd14bcdf0eebec64958b38584363a0 Mon Sep 17 00:00:00 2001 From: Alexey Rochev Date: Sat, 7 Sep 2024 01:45:52 +0300 Subject: [PATCH 3/3] Enable ASAN in RPM CI builds --- .github/workflows/main.yml | 4 ++-- packaging/rpm/tremotesf.spec | 23 ++++++++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d3ecf743..f89bf6ea 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -62,7 +62,7 @@ jobs: - name: Install build dependencies run: | - readarray -t dependencies < <(rpmspec ${{steps.fedora-compiler.outputs.rpm-macro}} -q --srpm --qf '[%{REQUIRES}\n]' packaging/rpm/tremotesf.spec) + readarray -t dependencies < <(rpmspec ${{steps.fedora-compiler.outputs.rpm-macro}} --define 'with_asan 1' -q --srpm --qf '[%{REQUIRES}\n]' packaging/rpm/tremotesf.spec) ${{env.INSTALL_PACKAGES}} "${dependencies[@]}" - name: Make source archive @@ -76,7 +76,7 @@ jobs: - name: Build RPM run: | - rpmbuild ${{steps.fedora-compiler.outputs.rpm-macro}} -bb packaging/rpm/tremotesf.spec + rpmbuild ${{steps.fedora-compiler.outputs.rpm-macro}} -bb --with asan packaging/rpm/tremotesf.spec - name: Install RPM run: | diff --git a/packaging/rpm/tremotesf.spec b/packaging/rpm/tremotesf.spec index 294cf08c..6848ca1a 100644 --- a/packaging/rpm/tremotesf.spec +++ b/packaging/rpm/tremotesf.spec @@ -2,6 +2,8 @@ # # SPDX-License-Identifier: CC0-1.0 +%bcond asan 0 + %global app_id org.equeim.Tremotesf %if %{defined suse_version} || 0%{?fedora} >= 40 @@ -47,17 +49,23 @@ BuildRequires: openssl-devel %if %{defined fedora} BuildRequires: cmake(httplib) BuildRequires: libappstream-glib -%if "%{toolchain}" == "clang" + %if "%{toolchain}" == "clang" BuildRequires: clang -%else + %if %{with asan} +BuildRequires: compiler-rt + %endif + %else BuildRequires: gcc-c++ -%endif + %if %{with asan} +BuildRequires: libasan + %endif + %endif Requires: qt%{qt_version}-qtsvg Requires: breeze-icon-theme -%if %{qt_version} == 5 + %if %{qt_version} == 5 Requires: kwayland-integration -%endif -%global tremotesf_with_httplib system + %endif + %global tremotesf_with_httplib system %endif %if %{defined suse_version} @@ -90,10 +98,11 @@ Remote GUI for Transmission BitTorrent client. %build -%cmake -D TREMOTESF_QT6=%[%{qt_version} == 6 ? "ON" : "OFF"] -D TREMOTESF_WITH_HTTPLIB=%{tremotesf_with_httplib} +%cmake -D TREMOTESF_QT6=%[%{qt_version} == 6 ? "ON" : "OFF"] -D TREMOTESF_WITH_HTTPLIB=%{tremotesf_with_httplib} -D TREMOTESF_ASAN=%{with asan} %cmake_build %check +export ASAN_OPTIONS=detect_leaks=0 %ctest %install