Adds static analysis to the ci

.github/workflows/testing.yml

@@ -264,3 +264,64 @@ jobs:
           ref: ${{ github.event.inputs.tag }}
       - name: lint code
         run: ${GITHUB_WORKSPACE}/packaging/lint/lint.sh
+
+  static-analysis:
+    # Pinned to ubuntu-26.04 (rather than ubuntu-latest) because
+    # clang-tools-22 -- see below -- isn't in the apt archive for older
+    # Ubuntu releases (e.g. 24.04), only 26.04+.
+    runs-on: ubuntu-26.04
+    timeout-minutes: 20
+    steps:
+      - name: checkout CCTools from branch head
+        if: github.event_name != 'workflow_dispatch'
+        uses: actions/checkout@v4
+        with:
+          # scan-build.sh diffs against the PR's base commit / the push's
+          # previous commit to scope the gate to only the lines this
+          # change touches (see scan-build.sh); that needs real history,
+          # not the single-commit shallow clone actions/checkout defaults to.
+          fetch-depth: 0
+      - name: checkout CCTools from tag
+        if: github.event_name == 'workflow_dispatch'
+        uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.inputs.tag }}
+      - name: install clang static analyzer
+        # Pinned to LLVM 22 (rather than whatever "clang-tools" resolves to
+        # on the runner's Ubuntu release) to match the clang/clang-tools
+        # version pinned in pixi.toml -- an unpinned version drift here can
+        # rename or relocate checkers relative to what scan-build.sh's two
+        # confirmation passes expect.
+        run: sudo apt-get update && sudo apt-get install -y clang-tools-22
+      - name: configure
+        run: ./configure --strict --without-system-parrot --without-system-prune --without-system-umbrella --without-system-weaver --with-readline-path no --with-fuse-path no --with-perl-path no
+      - name: static analysis (clang scan-build) on all C packages
+        # Unset for workflow_dispatch: a release build from an
+        # already-merged tag has no "PR diff" to scope to, and every commit
+        # reaching it already passed this gate diff-scoped at merge time.
+        # scan-build.sh reports findings but doesn't fail the build when
+        # this is unset (see its diff-scoping section).
+        env:
+          SCAN_BUILD_DIFF_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event_name == 'push' && github.event.before || '' }}
+        run: ${GITHUB_WORKSPACE}/packaging/lint/scan-build.sh
+
+  valgrind:
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    steps:
+      - name: checkout CCTools from branch head
+        if: github.event_name != 'workflow_dispatch'
+        uses: actions/checkout@v4
+      - name: checkout CCTools from tag
+        if: github.event_name == 'workflow_dispatch'
+        uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.inputs.tag }}
+      - name: install valgrind
+        run: sudo apt-get update && sudo apt-get install -y valgrind
+      - name: configure
+        run: ./configure --strict --without-system-parrot --without-system-prune --without-system-umbrella --without-system-weaver --with-readline-path no --with-fuse-path no --with-perl-path no
+      - name: build packages covered by a valgrind regression test (dttools, taskvine, work_queue, makeflow, resource_monitor)
+        run: make dttools taskvine work_queue makeflow resource_monitor
+      - name: run valgrind regression tests
+        run: ${GITHUB_WORKSPACE}/packaging/valgrind/run-valgrind-tests.sh

environment.yml

@@ -21,6 +21,7 @@ dependencies:
  - gdb                  # optional debugger to match compiler
  - flake8               # optional to lint and format Python code
  - clang-format         # optional to lint and format C code
+ - clang-tools          # optional for static analysis via scan-build
  - m4                   # optional for building man pages
  - doxygen              # optional to make doxygen api docs
  - mkdocs               # optional to make readthedocs online manual

packaging/lint/scan-build-parse.py

@@ -0,0 +1,108 @@
+#!/usr/bin/env python3
+"""
+Parse a clang scan-build run's index.html into a stable, sorted list of
+findings, one per line, formatted as `checker:file:line`.
+
+This is used by packaging/lint/scan-build.sh to turn each of its two
+analysis passes into a comparable list of findings (a finding only gates
+the build if it shows up in both passes -- see scan-build.sh for why).
+
+Findings under vendored/third-party paths (lua, sqlite) are dropped here
+regardless of caller, so they never get flagged -- see
+EXCLUDED_PATH_SUBSTRINGS below.
+
+Usage:
+    scan-build-parse.py <scan-build-output-dir>
+
+<scan-build-output-dir> is the directory passed to `scan-build -o`; this
+script finds the single timestamped run subdirectory inside it (scan-build
+creates a new one per run) and parses its index.html.
+"""
+
+import html
+import os
+import re
+import sys
+
+ROW_RE = re.compile(
+    r'<tr class="([^"]+)">'
+    r'<td class="DESC">[^<]*</td>'
+    r'<td class="DESC">[^<]*</td>'
+    r'<td>([^<]*)</td>'
+    r'<td class="DESC">[^<]*</td>'
+    r'<td class="Q">(\d+)</td>',
+    re.S,
+)
+
+# Vendored/third-party code that scan-build should never report findings
+# for, regardless of which package pulls it in. Matched case-insensitively
+# against the finding's file path.
+EXCLUDED_PATH_SUBSTRINGS = ("lua", "sqlite")
+
+
+def is_excluded(file_path):
+    lowered = file_path.lower()
+    return any(substr in lowered for substr in EXCLUDED_PATH_SUBSTRINGS)
+
+
+def find_run_dir(output_dir):
+    candidates = [
+        os.path.join(output_dir, name)
+        for name in sorted(os.listdir(output_dir))
+        if os.path.isdir(os.path.join(output_dir, name))
+    ]
+    if not candidates:
+        return None
+    # scan-build names each run's directory with a timestamp; the most
+    # recent one (last alphabetically, since the format is YYYY-MM-DD-...)
+    # is the one we just produced.
+    return candidates[-1]
+
+
+def parse_index(index_path):
+    with open(index_path, "r", encoding="utf-8", errors="replace") as f:
+        text = f.read()
+
+    findings = set()
+    for checker, file_path, line in ROW_RE.findall(text):
+        file_path = html.unescape(file_path).strip()
+        if not file_path:
+            continue
+        if is_excluded(file_path):
+            continue
+        findings.add(f"{checker}:{file_path}:{line}")
+
+    return findings
+
+
+def main():
+    if len(sys.argv) != 2:
+        print(f"usage: {sys.argv[0]} <scan-build-output-dir>", file=sys.stderr)
+        return 2
+
+    output_dir = sys.argv[1]
+
+    if not os.path.isdir(output_dir):
+        print(f"no such directory: {output_dir}", file=sys.stderr)
+        return 2
+
+    run_dir = find_run_dir(output_dir)
+    if run_dir is None:
+        # scan-build found nothing to report and didn't create a run
+        # subdirectory at all -- zero findings, not an error.
+        return 0
+
+    index_path = os.path.join(run_dir, "index.html")
+    if not os.path.isfile(index_path):
+        print(f"no index.html in {run_dir}", file=sys.stderr)
+        return 2
+
+    findings = parse_index(index_path)
+    for finding in sorted(findings):
+        print(finding)
+
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

packaging/lint/scan-build.sh

@@ -0,0 +1,199 @@
+#! /bin/bash
+set -e
+
+# sort and comm must agree on collation, or comm spuriously reports
+# "not in sorted order" on perfectly sorted input (locale-dependent
+# collation can disagree with itself across the two calls) and aborts the
+# script via set -e before it can report its outcome.
+export LC_ALL=C
+
+# Find cctools src directory
+CCTOOLS_SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")"/../.. && pwd)"
+cd "${CCTOOLS_SRC}"
+
+# Static analysis via clang's static analyzer (scan-build); see the
+# taskvine tech-debt audit's "static analysis" item. Covers every C package
+# under CCTOOLS_PACKAGES instead of taskvine only. doc and poncho are
+# excluded: they aren't C.
+C_PACKAGES="dttools batch_job taskvine grow makeflow work_queue ftp_lite resource_monitor chirp deltadb"
+
+# --- locate scan-build and the ccc-analyzer/c++-analyzer wrapper scripts it needs ---
+
+SCAN_BUILD=""
+if command -v scan-build > /dev/null 2>&1
+then
+	SCAN_BUILD="scan-build"
+else
+	# Debian/Ubuntu package versioned scan-build as scan-build-<N> with no
+	# unversioned symlink guaranteed; search newest-first down to the
+	# oldest LLVM release still plausibly present on a runner.
+	for version in $(seq 30 -1 15)
+	do
+		if command -v "scan-build-${version}" > /dev/null 2>&1
+		then
+			SCAN_BUILD="scan-build-${version}"
+			break
+		fi
+	done
+fi
+
+if [ -z "${SCAN_BUILD}" ]
+then
+	echo "scan-build (clang static analyzer) is not installed -- see packaging/lint/scan-build.sh for what this job checks."
+	exit 1
+fi
+
+CCC_ANALYZER=""
+CXX_ANALYZER=""
+for dir in "${CONDA_PREFIX:-}/libexec" /usr/lib/llvm-*/libexec /usr/share/clang/scan-build*/libexec /usr/libexec
+do
+	if [ -x "${dir}/ccc-analyzer" ] && [ -x "${dir}/c++-analyzer" ]
+	then
+		CCC_ANALYZER="${dir}/ccc-analyzer"
+		CXX_ANALYZER="${dir}/c++-analyzer"
+		break
+	fi
+done
+
+if [ -z "${CCC_ANALYZER}" ]
+then
+	echo "found ${SCAN_BUILD} but couldn't locate its ccc-analyzer/c++-analyzer wrapper scripts (looked under /usr/lib/llvm-*/libexec, /usr/share/clang/scan-build*/libexec, /usr/libexec) -- see packaging/lint/scan-build.sh."
+	exit 1
+fi
+
+echo "=== using ${SCAN_BUILD}, analyzers at $(dirname "${CCC_ANALYZER}") ==="
+
+# --- match the analyzer's "real" compiler to the one this tree is configured with ---
+#
+# ccc-analyzer/c++-analyzer run the static analyzer AND do the actual
+# compile, but if CCC_CC/CCC_CXX aren't set they default to plain gcc/g++
+# off PATH rather than whatever this tree was configured with (see
+# CCTOOLS_CC/CCTOOLS_CXX in config.mk). In an environment where those
+# differ -- e.g. a conda dev shell, where PATH's gcc is the distro's
+# system compiler/glibc but ./configure picked conda's -- that mismatch
+# can trip real compiler warnings-as-errors under --strict that don't
+# occur with the compiler this tree actually builds with, and don't occur
+# in CI either (a plain Ubuntu runner only ever has the one gcc). Pulling
+# the real compiler out of config.mk keeps the analyzer's compile step
+# consistent with the rest of the build.
+CONFIG_MK="${CCTOOLS_SRC}/config.mk"
+if [ -f "${CONFIG_MK}" ]
+then
+	CONFIGURED_CC="$(sed -n 's/^CCTOOLS_CC=.*;//p' "${CONFIG_MK}")"
+	CONFIGURED_CXX="$(sed -n 's/^CCTOOLS_CXX=.*;//p' "${CONFIG_MK}")"
+	if [ -n "${CONFIGURED_CC}" ] && [ -n "${CONFIGURED_CXX}" ]
+	then
+		export CCC_CC="${CONFIGURED_CC}"
+		export CCC_CXX="${CONFIGURED_CXX}"
+		echo "=== analyzer's real compiler set to configured CCTOOLS_CC/CCTOOLS_CXX: ${CCC_CC} / ${CCC_CXX} ==="
+	fi
+fi
+
+# --- run the analysis, twice ---
+#
+# cctools' build uses its own CCTOOLS_CC/CCTOOLS_CXX make variables rather
+# than the standard CC/CXX that scan-build auto-intercepts, so the analyzer
+# wrappers are injected directly via those variables instead (see rules.mk).
+# A clean build is required first each time: scan-build only sees compiles
+# that actually happen, so any already-built .o files from a previous step
+# would silently go unanalyzed.
+#
+# The clang static analyzer's path exploration is not fully deterministic
+# run-to-run for the *same* unchanged source (observed directly while
+# building this job: ~2% of findings shifted location or appeared/
+# disappeared between two consecutive clean runs of identical code). To
+# keep the gate from failing CI on that inherent tool noise, a finding only
+# counts as "new" if it shows up in BOTH of two independent runs; anything
+# that only appears once is treated as analyzer flakiness, not a real new
+# issue. This roughly doubles the job's runtime (a few minutes each way)
+# in exchange for not being a flaky gate.
+
+run_scan_build()
+{
+	out_dir="$1"
+	make clean > /dev/null
+	# shellcheck disable=SC2086
+	"${SCAN_BUILD}" -o "${out_dir}" --keep-empty make -j"$(nproc)" \
+		CCTOOLS_CC="@echo COMPILE \$@;${CCC_ANALYZER}" \
+		CCTOOLS_CXX="@echo COMPILE \$@;${CXX_ANALYZER}" \
+		${C_PACKAGES}
+}
+
+OUT_DIR_1="$(mktemp -d)"
+OUT_DIR_2="$(mktemp -d)"
+FOUND_1="$(mktemp)"
+FOUND_2="$(mktemp)"
+CONFIRMED="$(mktemp)"
+cleanup()
+{
+	rm -rf "${OUT_DIR_1}" "${OUT_DIR_2}" "${FOUND_1}" "${FOUND_2}" "${CONFIRMED}"
+}
+trap cleanup EXIT
+
+echo "=== pass 1/2 ==="
+run_scan_build "${OUT_DIR_1}"
+python3 "${CCTOOLS_SRC}/packaging/lint/scan-build-parse.py" "${OUT_DIR_1}" | sort -u > "${FOUND_1}"
+echo "=== pass 1/2: $(wc -l < "${FOUND_1}") finding(s) ==="
+
+echo "=== pass 2/2 ==="
+run_scan_build "${OUT_DIR_2}"
+python3 "${CCTOOLS_SRC}/packaging/lint/scan-build-parse.py" "${OUT_DIR_2}" | sort -u > "${FOUND_2}"
+echo "=== pass 2/2: $(wc -l < "${FOUND_2}") finding(s) ==="
+
+comm -12 "${FOUND_1}" "${FOUND_2}" > "${CONFIRMED}"
+echo "=== $(wc -l < "${CONFIRMED}") finding(s) confirmed by both passes ==="
+
+# --- gate on findings that land on a line this change touches ---
+#
+# There is no baseline/suppression file: a confirmed finding on a line the
+# change added or modified fails the build outright, including a repeat
+# false positive -- a reviewer evaluates those as they come up rather than
+# them being pre-suppressed. What keeps this from also failing on the
+# repo's large pre-existing backlog of findings is scope, not a baseline:
+# only findings on a touched line are ever compared at all, so a
+# pre-existing finding elsewhere in a file this change happens to touch is
+# never even looked at, no matter where it sits.
+#
+# SCAN_BUILD_DIFF_BASE (set by CI to the PR's base commit or the push's
+# previous commit) is what provides that scope. Without it -- a local run,
+# or a context with no meaningful prior commit to diff against (e.g. a
+# release build with no PR) -- there is nothing to scope to, so findings
+# are reported for visibility only and the run does not fail the build.
+if [ -n "${SCAN_BUILD_DIFF_BASE:-}" ] && [ "${SCAN_BUILD_DIFF_BASE}" != "0000000000000000000000000000000000000000" ] && git cat-file -e "${SCAN_BUILD_DIFF_BASE}^{commit}" 2> /dev/null
+then
+	MERGE_BASE="$(git merge-base "${SCAN_BUILD_DIFF_BASE}" HEAD)"
+	echo "=== diff mode: scoping gate to changes since ${MERGE_BASE} ==="
+	TOUCHED_LINES="$(mktemp)"
+	CONFIRMED_SCOPED="$(mktemp)"
+	trap 'rm -f "${TOUCHED_LINES}" "${CONFIRMED_SCOPED}"; cleanup' EXIT
+	# shellcheck disable=SC2086
+	git diff --unified=0 "${MERGE_BASE}" HEAD -- ${C_PACKAGES} \
+		| python3 "${CCTOOLS_SRC}/packaging/lint/diff-touched-lines.py" \
+		| sort -u > "${TOUCHED_LINES}"
+	echo "=== $(wc -l < "${TOUCHED_LINES}") line(s) touched by this change ==="
+	# CONFIRMED entries are checker:file:line; TOUCHED_LINES entries are
+	# file:line. Paths are assumed colon-free.
+	awk -F: 'NR==FNR{touched[$0]=1; next} { if (($2 ":" $3) in touched) print }' "${TOUCHED_LINES}" "${CONFIRMED}" > "${CONFIRMED_SCOPED}"
+
+	if [ -s "${CONFIRMED_SCOPED}" ]
+	then
+		echo "=== scan-build findings on lines this change touches ==="
+		cat "${CONFIRMED_SCOPED}"
+		echo
+		echo "Fix these, or if one is a false positive, leave it for a reviewer to evaluate on the PR."
+		exit 1
+	fi
+
+	echo "=== no scan-build findings on lines this change touches ==="
+else
+	echo "=== SCAN_BUILD_DIFF_BASE not set (or not resolvable): nothing to scope to, not gating ==="
+	if [ -s "${CONFIRMED}" ]
+	then
+		echo "=== scan-build findings (repo-wide, informational only) ==="
+		cat "${CONFIRMED}"
+	else
+		echo "=== no scan-build findings ==="
+	fi
+fi
+
+# vim: set noexpandtab tabstop=4: