diff --git a/.github/workflows/test-pull_request.yaml b/.github/workflows/test-pull_request.yaml index ecf853dc6cf..3f8ce38fe93 100644 --- a/.github/workflows/test-pull_request.yaml +++ b/.github/workflows/test-pull_request.yaml @@ -177,12 +177,22 @@ jobs: run: $GITHUB_WORKSPACE/ci/linux/run_tests.sh - name: Show CCache statistics run: ccache --show-stats + - name: Install clang-tidy + # macOS runners don't ship clang-tidy; pin to the same version as the Linux + # container's clang-tidy-16 for consistent diagnostics. pyyaml is required + # too: clang-tidy-diff.py only accepts -export-fixes when it can import yaml. + # --break-system-packages is required since the runner's Homebrew Python + # is externally managed (PEP 668); the runner is ephemeral so this is safe. + if: startsWith(matrix.compiler, 'clang') + run: pip3 install --break-system-packages clang-tidy==16.0.4 pyyaml - name: Run Clang Tidy # Clang-tidy reuses the precompiled headers so this only makes sense for the clang compilers - if: startsWith(matrix.compiler, 'clang-') + if: startsWith(matrix.compiler, 'clang') + env: + CLANG_TIDY_BINARY: clang-tidy run: $GITHUB_WORKSPACE/ci/linux/clang_tidy.sh ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} - name: Process clang-tidy warnings - if: startsWith(matrix.compiler, 'clang-') + if: startsWith(matrix.compiler, 'clang') uses: asarium/clang-tidy-action@v1 with: fixesFile: clang-fixes.yaml diff --git a/ci/linux/clang_tidy.sh b/ci/linux/clang_tidy.sh index 2a0eec709cb..f10981c9770 100755 --- a/ci/linux/clang_tidy.sh +++ b/ci/linux/clang_tidy.sh @@ -18,8 +18,16 @@ fi # branch BASE_COMMIT=$(git merge-base $1 $2) +# Allow the caller to override the clang-tidy binary and job count, since neither +# /usr/bin/clang-tidy-16 nor nproc are available outside the Linux CI container +# (e.g. macOS runners install clang-tidy via pip and use sysctl for core count). +CLANG_TIDY_BINARY="${CLANG_TIDY_BINARY:-/usr/bin/clang-tidy-16}" +NUM_JOBS="${NUM_JOBS:-$( (command -v nproc >/dev/null 2>&1 && nproc) || sysctl -n hw.ncpu )}" + +# Note: Manually passing in the Vulkan flags that are normally provided by cmake (but are not so, here), to ensure +# that the source files are checked with the actual configuration used. echo "Running clang-tidy on changed files" git diff -U0 --no-color "$BASE_COMMIT..$2" | \ $HERE/clang-tidy-diff.py -path "$(pwd)/build" -p1 \ -regex '(code(?!((\/graphics\/shaders\/compiled)|(\/globalincs\/windebug)|(\/def_files\/data)))|freespace2|qtfred|test\/src|build|tools)\/.*\.(cpp|h)' \ - -clang-tidy-binary /usr/bin/clang-tidy-16 -j$(nproc) -export-fixes "$(pwd)/clang-fixes.yaml" + -clang-tidy-binary "$CLANG_TIDY_BINARY" -j"$NUM_JOBS" -export-fixes "$(pwd)/clang-fixes.yaml" diff --git a/ci/linux/run_tests.sh b/ci/linux/run_tests.sh index 92c31053ece..4a89127c41e 100755 --- a/ci/linux/run_tests.sh +++ b/ci/linux/run_tests.sh @@ -12,7 +12,7 @@ if [ "$RUNNER_OS" = "macOS" ] && [ "$ARCH" != "$(uname -m)" ]; then exit 0 fi -if [ "$CONFIGURATION" = "Debug"] && [[ "$RUNNER_OS" != "macOS" ]] ; then +if [ "$CONFIGURATION" = "Debug" ] && [[ "$RUNNER_OS" != "macOS" ]] ; then valgrind --leak-check=full --error-exitcode=1 --gen-suppressions=all \ --suppressions="$HERE/valgrind.supp" ./bin/unittests --gtest_shuffle else diff --git a/ci/linux/valgrind.supp b/ci/linux/valgrind.supp index 535ad5c7b18..41699f569a8 100644 --- a/ci/linux/valgrind.supp +++ b/ci/linux/valgrind.supp @@ -8,24 +8,6 @@ obj:* obj:* } -{ - Travis memleak - Memcheck:Leak - match-leak-kinds: definite - fun:malloc - fun:_dl_signal_error - fun:_dl_open - fun:dlopen_doit - fun:_dl_catch_error - fun:_dlerror_run - fun:dlopen@@GLIBC_2.2.5 - obj:/usr/lib/x86_64-linux-gnu/libmirclient.so.7 - fun:call_init.part.0 - fun:call_init - fun:_dl_init - fun:dl_open_worker - fun:_dl_catch_error -} { CLang Lua unitialized value error Memcheck:Cond @@ -127,37 +109,28 @@ fun:_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc } { - + SDL_Init one-time internal allocation via os_init Memcheck:Leak match-leak-kinds: definite fun:malloc - fun:_dl_signal_error - fun:_dl_open - fun:dlopen_doit - fun:_dl_catch_error - fun:_dlerror_run - fun:dlopen@@GLIBC_2.2.5 obj:* obj:* - fun:call_init.part.0 - fun:call_init - fun:_dl_init - fun:dl_open_worker + obj:* + obj:* + fun:_Z7os_initPKcS0_S0_ + fun:_ZN4test13FSTestFixture5SetUpEv } { - dlopen leak + SDL3 X11 video backend internal allocations, not freed by SDL_VideoQuit Memcheck:Leak - match-leak-kinds: definite - fun:malloc - fun:_dl_exception_create - fun:_dl_signal_error - fun:_dl_map_object - fun:dl_open_worker - fun:_dl_catch_exception + match-leak-kinds: definite,possible + ... + fun:X11_CreateDevice +} +{ + glibc loader bookkeeping and library constructor allocations retained across dlopen + Memcheck:Leak + match-leak-kinds: definite,possible + ... fun:_dl_open - fun:dlopen_doit - fun:_dl_catch_exception - fun:_dl_catch_error - fun:_dlerror_run - fun:dlopen@@GLIBC_2.2.5 } diff --git a/cmake/toolchain-clang.cmake b/cmake/toolchain-clang.cmake index 63fe41836ba..182d17f8176 100644 --- a/cmake/toolchain-clang.cmake +++ b/cmake/toolchain-clang.cmake @@ -134,8 +134,10 @@ endif() set(CXX_FLAGS_RELEASE "${CXX_OPT_FLAG_RELEASE} -Wno-unused-variable -Wno-unused-parameter") set(C_FLAGS_RELEASE "${C_OPT_FLAG_RELEASE} -Wno-unused-variable -Wno-unused-parameter") -set(CXX_FLAGS_DEBUG "${CXX_OPT_FLAG_DEBUG} -g -Wshadow") -set(C_FLAGS_DEBUG "${C_OPT_FLAG_DEBUG} -g -Wshadow") +# Valgrind (as used by CI) does not understand the DWARF5 debug info that clang +# emits by default, so force DWARF4 to keep memcheck able to symbolize the binary. +set(CXX_FLAGS_DEBUG "${CXX_OPT_FLAG_DEBUG} -g -gdwarf-4 -Wshadow") +set(C_FLAGS_DEBUG "${C_OPT_FLAG_DEBUG} -g -gdwarf-4 -Wshadow") # Always use the base flags and add our compiler flags at the back set(CMAKE_CXX_FLAGS "${CXX_BASE_FLAGS} ${COMPILER_FLAGS}")