clang tidy pipeline fixes (#7657)

* 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.

* 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: 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Claude Opus 5 <[email protected]>

.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

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"

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

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
 }
 {
-   <insert_a_suppression_name_here>
+   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
 }

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}")