From b8b150eb0866023ffe551c238bdd7db593606506 Mon Sep 17 00:00:00 2001 From: Fabian Woltermann Date: Mon, 27 Jul 2026 09:21:57 +0200 Subject: [PATCH 1/4] ci: run clang-tidy on macOS and Linux clang builds Install a version-matched clang-tidy on macOS and make the shared runner portable across both CI environments. --- .github/workflows/test-pull_request.yaml | 14 ++++++++++++-- ci/linux/clang_tidy.sh | 10 +++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) 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" From 84504b27dc51df353e5a342d4ebb4088a0cef984 Mon Sep 17 00:00:00 2001 From: Fabian Woltermann Date: Mon, 27 Jul 2026 09:21:57 +0200 Subject: [PATCH 2/4] ci: keep valgrind compatible with clang debug builds Emit DWARF4 debug information and suppress SDL's one-time initialization allocation so memcheck can report actionable failures. --- ci/linux/run_tests.sh | 2 +- ci/linux/valgrind.supp | 12 ++++++++++++ cmake/toolchain-clang.cmake | 6 ++++-- 3 files changed, 17 insertions(+), 3 deletions(-) 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..c7a0a15940f 100644 --- a/ci/linux/valgrind.supp +++ b/ci/linux/valgrind.supp @@ -161,3 +161,15 @@ fun:_dlerror_run fun:dlopen@@GLIBC_2.2.5 } +{ + SDL_Init one-time internal allocation via os_init + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + obj:* + obj:* + obj:* + obj:* + fun:_Z7os_initPKcS0_S0_ + fun:_ZN4test13FSTestFixture5SetUpEv +} 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}") From 1ca9d256c920dfdb42e842f0f338e18875bf6e32 Mon Sep 17 00:00:00 2001 From: the-e Date: Mon, 27 Jul 2026 18:15:54 +0200 Subject: [PATCH 3/4] ci: suppress SDL3/libX11 video backend leaks under valgrind Now that the `[ "$CONFIGURATION" = "Debug" ]` typo is fixed, valgrind actually runs on the Linux Debug legs again, and memcheck fails the build on six "definitely lost" contexts (480 bytes) that all originate inside SDL3's X11 backend at X11_CreateDevice, reached from SDL_VideoInit. These aren't ours to fix: both call sites (os_init's SDL_Init and the temporary video init in resolution_default(), reached via gr_init) already pair their init with a matching quit, and the allocations are made and kept by SDL/libX11 across SDL_VideoQuit. Collapsing the repeated init/quit cycles would only reduce the block count, not the number of leak contexts, so the build would still fail. The suppression is truncated at X11_CreateDevice on purpose: it stays caller-agnostic (covering both the os_init and gr_init paths) and avoids matching on SDL_InitSubSystem_REAL.part.0, whose .part.0 suffix is a partial-inlining artifact of how SDL3 happened to be built. Co-Authored-By: Claude Opus 5 --- ci/linux/valgrind.supp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ci/linux/valgrind.supp b/ci/linux/valgrind.supp index c7a0a15940f..7b7999a966a 100644 --- a/ci/linux/valgrind.supp +++ b/ci/linux/valgrind.supp @@ -173,3 +173,10 @@ fun:_Z7os_initPKcS0_S0_ fun:_ZN4test13FSTestFixture5SetUpEv } +{ + SDL3 X11 video backend internal allocations, not freed by SDL_VideoQuit + Memcheck:Leak + match-leak-kinds: definite + ... + fun:X11_CreateDevice +} From b0e55a8a658b92d19642f746433a37be5fea7dab Mon Sep 17 00:00:00 2001 From: the-e Date: Mon, 27 Jul 2026 19:10:18 +0200 Subject: [PATCH 4/4] ci: suppress glibc dlopen loader leaks under valgrind The Linux clang-16 Debug leg fails memcheck on a single zero-byte block lost in glibc's `_dl_find_object_update()`, reached from `dlopen()` while SDL probes its video backends. It is loader bookkeeping that glibc keeps by design, and it only shows up on some legs because it depends on how many objects the loader has mapped by that point. The file already carried three suppressions for this class, but each pinned a full, exact glibc stack, so every new loader code path costs another CI round. Replace them with a single block matching on `_dl_open`: nothing of ours runs below that frame, only loader internals and the static constructors of shared libraries the loader brings in, so this gives up no coverage we care about while retiring the whole class. All three removed blocks contain `_dl_open` themselves and are therefore subsumed by it. Both new suppressions match `definite,possible` rather than `definite` alone, since `--errors-for-leak-kinds` defaults to `definite,possible` and the same allocations would otherwise fail the build if valgrind ever reported them as possibly lost instead. Co-Authored-By: Claude Opus 5 --- ci/linux/valgrind.supp | 62 ++++++------------------------------------ 1 file changed, 8 insertions(+), 54 deletions(-) diff --git a/ci/linux/valgrind.supp b/ci/linux/valgrind.supp index 7b7999a966a..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 @@ -126,41 +108,6 @@ fun:_ZN7testing8internal38HandleSehExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc fun:_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc } -{ - - 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 -} -{ - dlopen leak - 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 - fun:_dl_open - fun:dlopen_doit - fun:_dl_catch_exception - fun:_dl_catch_error - fun:_dlerror_run - fun:dlopen@@GLIBC_2.2.5 -} { SDL_Init one-time internal allocation via os_init Memcheck:Leak @@ -176,7 +123,14 @@ { SDL3 X11 video backend internal allocations, not freed by SDL_VideoQuit Memcheck:Leak - match-leak-kinds: definite + 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 +}