From 481d003b6dcaab08ab86fa785ae2649cf618e949 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 22:25:23 +0000 Subject: [PATCH 01/22] CI: enable LLVM source-based C/C++ coverage on macOS and Linux pytest Adds opt-in clang -fprofile-instr-generate -fcoverage-mapping instrumentation to the macOS and Linux pytest builds and wires the resulting coverage to Codecov alongside the existing Python coverage. Determinism under pytest-xdist: LLVM_PROFILE_FILE is set to grass-%p-%9m.profraw so every subprocess and every instrumented binary writes its own file, eliminating the collision that made re-runs non-deterministic for untouched files. Merge uses llvm-profdata -sparse which is bit-identical for a given input set (verified locally). Fixes a latent ordering bug where utils/coverage_mapper.py ran before coverage combine and could not read a .coverage that did not yet exist in parallel mode. The mapper now runs on the combined data. Adds utils/coverage_disk_usage.sh to log raw coverage disk usage before the combine step so a run that threatens the runner storage budget is visible in the log, and utils/llvm_coverage_export.sh to merge .profraw into .profdata and export lcov for Codecov, filtering system, conda, and build-tree paths. macOS Codecov uploads use distinct flags macos-pytest-python and macos-llvm-c; Linux adds ubuntu-llvm-c to exercise the same path during local iteration. macOS coverage is combined after the gunittest run so profraw from both pytest and gunittest are included. Large use of AI assistance (Claude Code) helped draft the workflow diffs and the utility scripts. --- .github/workflows/apt.txt | 2 + .github/workflows/build_ubuntu-24.04.sh | 12 ++++ .github/workflows/macos.yml | 75 +++++++++++++++++++++++++ .github/workflows/macos_install.sh | 10 ++++ .github/workflows/pytest.yml | 70 +++++++++++++++-------- utils/coverage_disk_usage.sh | 29 ++++++++++ utils/llvm_coverage_export.sh | 59 +++++++++++++++++++ 7 files changed, 234 insertions(+), 23 deletions(-) create mode 100755 utils/coverage_disk_usage.sh create mode 100755 utils/llvm_coverage_export.sh diff --git a/.github/workflows/apt.txt b/.github/workflows/apt.txt index 50fc0fa8b84..814545569f1 100644 --- a/.github/workflows/apt.txt +++ b/.github/workflows/apt.txt @@ -1,5 +1,6 @@ bison build-essential +clang flex libbz2-dev libcairo-dev @@ -16,6 +17,7 @@ libproj-dev libreadline-dev libsvm-dev libzstd-dev +llvm pdal proj-bin python3-dateutil diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index 677c46d5f33..0b55bd7621c 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -30,6 +30,18 @@ set -u export INSTALL_PREFIX=$1 +# LLVM source-based coverage instrumentation, opt-in via env var. Requires +# clang (installed via apt.txt). Kept out of the default O2 path so +# non-coverage runs are unaffected. +if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then + export CC="${CC:-clang}" + export CXX="${CXX:-clang++}" + COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -O0 -g" + export CFLAGS="${CFLAGS:-} ${COV_FLAGS}" + export CXXFLAGS="${CXXFLAGS:-} ${COV_FLAGS}" + export LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" +fi + ./configure \ --enable-largefile \ --prefix="$INSTALL_PREFIX/" \ diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 4aa66a8335a..72e87176cfc 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -23,6 +23,7 @@ jobs: runs-on: macos-26 env: PYTHONWARNINGS: always + GRASS_LLVM_COVERAGE: "1" steps: - name: Info run: | @@ -57,8 +58,11 @@ jobs: - name: Prepare Conda environment file env: test_dependencies: | + coverage ipython + llvm-tools pytest + pytest-cov pytest-github-actions-annotate-failures pytest-timeout pytest-xdist @@ -92,6 +96,18 @@ jobs: shell: micromamba-shell {0} run: source ./.github/workflows/print_versions.sh + - name: Prepare coverage directories + shell: micromamba-shell {0} + run: | + mkdir -p "${PWD}/llvm-cov-raw" + # %p (process id) separates every subprocess; %9m caps the per-binary + # pool at 512 files so long xdist runs don't fill the runner disk. + { + echo "LLVM_PROFILE_FILE=${PWD}/llvm-cov-raw/grass-%p-%9m.profraw" + echo "INITIAL_GISBASE=$(grass --config path)" + echo "INITIAL_PWD=${PWD}" + } >> "${GITHUB_ENV}" + - name: Run pytest with multiple workers in parallel shell: micromamba-shell {0} run: | @@ -101,6 +117,7 @@ jobs: export LD_LIBRARY_PATH pytest \ @.github/workflows/pytest_args_ci.txt \ + @.github/workflows/pytest_args_cov.txt \ @.github/workflows/pytest_args_parallel.txt \ --junitxml=pytest.xdist.junit.xml \ -k 'not testsuite' @@ -116,6 +133,7 @@ jobs: export LD_LIBRARY_PATH pytest \ @.github/workflows/pytest_args_ci.txt \ + @.github/workflows/pytest_args_cov.txt \ @.github/workflows/pytest_args_not_parallel.txt \ --junitxml=pytest.needs_solo_run.junit.xml \ -k 'not testsuite' @@ -127,6 +145,7 @@ jobs: export PYTHONPATH export LD_LIBRARY_PATH pytest \ + @.github/workflows/pytest_args_cov.txt \ @.github/workflows/pytest_args_gunittest.txt \ --junitxml=pytest.gunittest.junit.xml @@ -171,6 +190,62 @@ jobs: SAMPLE_DATA_URL: "file://${{ github.workspace }}/sample-data/\ nc_spm_full_v2beta1.tar.gz" + - name: Coverage disk usage (before combine) + if: ${{ !cancelled() }} + shell: micromamba-shell {0} + run: utils/coverage_disk_usage.sh + + - name: Combine Python coverage and export XML + if: ${{ !cancelled() }} + shell: micromamba-shell {0} + run: | + # coverage combine before coverage_mapper.py: parallel mode leaves + # only .coverage... partials, no .coverage yet. + coverage combine || true + if [ -f .coverage ]; then + python utils/coverage_mapper.py + if [ -f .coverage.fixed_scripts ]; then + mv .coverage.fixed_scripts .coverage + fi + coverage html + coverage xml -o coverage.xml + else + echo "No .coverage data collected; skipping Python coverage export." + fi + + - name: Merge and export LLVM C/C++ coverage + if: ${{ !cancelled() }} + shell: micromamba-shell {0} + run: utils/llvm_coverage_export.sh + + - name: Upload Python coverage to Codecov (macOS) + if: ${{ !cancelled() }} + uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + with: + verbose: true + files: coverage.xml + flags: macos-pytest-python + name: macos-pytest-python + token: ${{ secrets.CODECOV_TOKEN }} + + - name: Upload LLVM C/C++ coverage to Codecov (macOS) + if: ${{ !cancelled() }} + uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + with: + verbose: true + files: coverage-llvm.lcov + flags: macos-llvm-c + name: macos-llvm-c + token: ${{ secrets.CODECOV_TOKEN }} + + - name: Make python-only code coverage test report available + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: python-codecoverage-report-macOS + path: coverage_html_report + retention-days: 1 + - name: Make HTML test report available if: ${{ !cancelled() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/.github/workflows/macos_install.sh b/.github/workflows/macos_install.sh index 08cf05cd82c..5b06daa06a9 100755 --- a/.github/workflows/macos_install.sh +++ b/.github/workflows/macos_install.sh @@ -64,6 +64,16 @@ export CFLAGS="-O2 -pipe -ffp-contract=off -arch ${CONDA_ARCH} -DGL_SILENCE_DEPR export CXXFLAGS="-O2 -pipe -ffp-contract=off -stdlib=libc++ -arch ${CONDA_ARCH} -Wall -Wextra -Wpedantic" export CPPFLAGS="-isystem${CONDA_PREFIX}/include" +# LLVM source-based coverage instrumentation, opt-in via env var. +# -O0 keeps region mapping accurate; overhead is small compared to gcov. +if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then + COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -O0 -g" + CFLAGS="${CFLAGS} ${COV_FLAGS}" + CXXFLAGS="${CXXFLAGS} ${COV_FLAGS}" + LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" + export CFLAGS CXXFLAGS LDFLAGS +fi + ./configure $CONFIGURE_FLAGS EXEMPT="" diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 593434e294c..084d6022070 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -32,6 +32,9 @@ jobs: FORCE_COLOR: 1 # for software including pip: https://force-color.org/ CLICOLOR_FORCE: 1 # for other software including ninja: https://bixense.com/clicolors/ PYTHONWARNINGS: always + GRASS_LLVM_COVERAGE: "1" + CC: clang + CXX: clang++ steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -90,16 +93,25 @@ jobs: - name: Test executing of the grass command run: .github/workflows/test_simple.sh + - name: Prepare coverage directories + run: | + mkdir -p "${PWD}/llvm-cov-raw" + # %p (process id) separates every subprocess; %9m caps the per-binary + # pool at 512 files so long suites don't create millions of tiny + # .profraw. Together they eliminate the write-collision that made + # earlier runs non-deterministic. + { + echo "LLVM_PROFILE_FILE=${PWD}/llvm-cov-raw/grass-%p-%9m.profraw" + echo "INITIAL_GISBASE=$(grass --config path)" + echo "INITIAL_PWD=${PWD}" + } >> "${GITHUB_ENV}" + - name: Run pytest with multiple workers in parallel run: | PYTHONPATH="$(grass --config python_path):${PYTHONPATH}" LD_LIBRARY_PATH="$(grass --config path)/lib:${LD_LIBRARY_PATH}" - INITIAL_GISBASE="$(grass --config path)" - INITIAL_PWD="${PWD}" export PYTHONPATH export LD_LIBRARY_PATH - export INITIAL_GISBASE - export INITIAL_PWD pytest \ @.github/workflows/pytest_args_ci.txt \ @.github/workflows/pytest_args_cov.txt \ @@ -113,12 +125,8 @@ jobs: run: | PYTHONPATH="$(grass --config python_path):${PYTHONPATH}" LD_LIBRARY_PATH="$(grass --config path)/lib:${LD_LIBRARY_PATH}" - INITIAL_GISBASE="$(grass --config path)" - INITIAL_PWD="${PWD}" export PYTHONPATH export LD_LIBRARY_PATH - export INITIAL_GISBASE - export INITIAL_PWD pytest \ @.github/workflows/pytest_args_ci.txt \ @.github/workflows/pytest_args_cov.txt \ @@ -129,29 +137,34 @@ jobs: run: | PYTHONPATH="$(grass --config python_path):${PYTHONPATH}" LD_LIBRARY_PATH="$(grass --config path)/lib:${LD_LIBRARY_PATH}" - INITIAL_GISBASE="$(grass --config path)" - INITIAL_PWD="${PWD}" export PYTHONPATH export LD_LIBRARY_PATH - export INITIAL_GISBASE - export INITIAL_PWD pytest \ @.github/workflows/pytest_args_cov.txt \ @.github/workflows/pytest_args_gunittest.txt \ --junitxml=pytest.gunittest.junit.xml - - name: Fix non-standard installed script paths in coverage data + + - name: Coverage disk usage (before combine) + if: ${{ !cancelled() }} + run: utils/coverage_disk_usage.sh + + - name: Combine Python coverage and export XML + if: ${{ !cancelled() }} run: | - PYTHONPATH="$(grass --config python_path):${PYTHONPATH}" - LD_LIBRARY_PATH="$(grass --config path)/lib:${LD_LIBRARY_PATH}" - INITIAL_GISBASE="$(grass --config path)" - INITIAL_PWD="${PWD}" - export PYTHONPATH - export LD_LIBRARY_PATH - export INITIAL_GISBASE - export INITIAL_PWD - python utils/coverage_mapper.py + # coverage combine must run before coverage_mapper.py: parallel + # mode leaves .coverage... partials and no + # .coverage file for the mapper to read. coverage combine + python utils/coverage_mapper.py + if [ -f .coverage.fixed_scripts ]; then + mv .coverage.fixed_scripts .coverage + fi coverage html + coverage xml -o coverage.xml + + - name: Merge and export LLVM C/C++ coverage + if: ${{ !cancelled() }} + run: utils/llvm_coverage_export.sh - name: Upload test results to Codecov if: ${{ !cancelled() }} @@ -162,13 +175,24 @@ jobs: token: ${{ secrets.CODECOV_TOKEN }} report_type: test_results - - name: Upload coverage reports to Codecov + - name: Upload Python coverage to Codecov uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: verbose: true + files: coverage.xml flags: pytest-python-${{ matrix.python-version }} name: pytest-python-${{ matrix.python-version }} token: ${{ secrets.CODECOV_TOKEN }} + + - name: Upload LLVM C/C++ coverage to Codecov + if: ${{ !cancelled() }} + uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 + with: + verbose: true + files: coverage-llvm.lcov + flags: ubuntu-llvm-c + name: ubuntu-llvm-c-${{ matrix.python-version }} + token: ${{ secrets.CODECOV_TOKEN }} - name: Make python-only code coverage test report available if: ${{ !cancelled() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/utils/coverage_disk_usage.sh b/utils/coverage_disk_usage.sh new file mode 100755 index 00000000000..f12a5c59c85 --- /dev/null +++ b/utils/coverage_disk_usage.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash + +# Print disk usage of raw coverage artifacts before combining. Emitted to +# the CI log so we can watch it approach the runner storage budget. + +set -u + +: "${INITIAL_PWD:=$(pwd)}" +: "${RAW_DIR:=${INITIAL_PWD}/llvm-cov-raw}" + +echo "=== LLVM .profraw files ===" +if [ -d "${RAW_DIR}" ]; then + count=$(find "${RAW_DIR}" -name '*.profraw' -type f | wc -l | tr -d ' ') + echo "count: ${count}" + du -sh "${RAW_DIR}" 2>/dev/null || true +else + echo "no ${RAW_DIR} (build was not instrumented, or no tests ran)" +fi + +echo "=== Python coverage.py partial files ===" +py_count=$(find "${INITIAL_PWD}" -maxdepth 2 -name '.coverage.*' -type f 2>/dev/null \ + | wc -l | tr -d ' ') +echo "count: ${py_count}" +if [ "${py_count}" -gt 0 ]; then + du -sch "${INITIAL_PWD}"/.coverage.* 2>/dev/null | tail -1 +fi + +echo "=== Filesystem ===" +df -h "${INITIAL_PWD}" 2>/dev/null || df -h diff --git a/utils/llvm_coverage_export.sh b/utils/llvm_coverage_export.sh new file mode 100755 index 00000000000..87f34ae676c --- /dev/null +++ b/utils/llvm_coverage_export.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash + +# Merge LLVM source-based coverage .profraw files into a single .profdata, +# then export lcov for Codecov. Invoked from CI after all test steps. +# +# Inputs (env): +# INITIAL_PWD - repository working directory (must match .coveragerc) +# RAW_DIR - directory holding *.profraw (default: $INITIAL_PWD/llvm-cov-raw) +# OUT_PROFDATA - merged profdata output (default: $INITIAL_PWD/grass.profdata) +# OUT_LCOV - lcov output for Codecov (default: $INITIAL_PWD/coverage-llvm.lcov) +# GISBASE - installed GRASS prefix; falls back to `grass --config path` +# +# Requires llvm-profdata and llvm-cov on PATH matching the clang used to build. + +set -euo pipefail + +: "${INITIAL_PWD:=$(pwd)}" +: "${RAW_DIR:=${INITIAL_PWD}/llvm-cov-raw}" +: "${OUT_PROFDATA:=${INITIAL_PWD}/grass.profdata}" +: "${OUT_LCOV:=${INITIAL_PWD}/coverage-llvm.lcov}" +: "${GISBASE:=$(grass --config path)}" + +if ! compgen -G "${RAW_DIR}/*.profraw" > /dev/null; then + echo "llvm_coverage_export: no .profraw files under ${RAW_DIR}; nothing to export." + : > "${OUT_LCOV}" + exit 0 +fi + +# -sparse drops zero-count regions so merges are smaller and deterministic +# for a given input set. +llvm-profdata merge -sparse -o "${OUT_PROFDATA}" "${RAW_DIR}"/*.profraw + +# Enumerate every instrumented binary and shared library the install produced. +# On macOS the tools are Mach-O executables plus *.dylib; on Linux ELF plus *.so. +OBJ_ARGS=() +while IFS= read -r -d '' obj; do + OBJ_ARGS+=(-object "${obj}") +done < <( + find "${GISBASE}" \ + \( -type f -perm -u+x \ + -o -name '*.dylib' -o -name '*.so' -o -name '*.so.*' \) \ + -print0 +) + +if [ "${#OBJ_ARGS[@]}" -eq 0 ]; then + echo "llvm_coverage_export: no instrumented objects found under ${GISBASE}." >&2 + exit 1 +fi + +# Drop system, conda, and build-tree paths so Codecov only sees repo files. +llvm-cov export \ + -format=lcov \ + -instr-profile="${OUT_PROFDATA}" \ + -ignore-filename-regex='(^/opt/|^/usr/|/conda-|/miniconda|/micromamba|/\.cache/|/bin\.[^/]+/|/dist\.[^/]+/|/OBJ\.[^/]+/)' \ + "${OBJ_ARGS[@]}" \ + > "${OUT_LCOV}" + +echo "llvm_coverage_export: wrote ${OUT_LCOV}" +du -sh "${OUT_PROFDATA}" "${OUT_LCOV}" From 2de6b015aa0899700083f6bc06961301fc00bb0b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 22:41:18 +0000 Subject: [PATCH 02/22] CI: gitignore LLVM coverage artifacts and wipe build-time profraw The instrumented build runs each tool with --interface-description to emit its XML, which drops default.profraw next to the binary. Those files are build artifacts (no tests ran, coverage from the build tree would contaminate the test-run merge), so they belong ignored, and the CI coverage-prep step deletes any stray default.profraw before the test steps start writing controlled per-PID files under llvm-cov-raw/. Adds *.profraw, *.profdata, coverage-llvm.lcov, llvm-cov-raw/, and coverage_html_report/ to .gitignore. --- .github/workflows/macos.yml | 4 ++++ .github/workflows/pytest.yml | 4 ++++ .gitignore | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 72e87176cfc..e76e81eed8c 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -100,6 +100,10 @@ jobs: shell: micromamba-shell {0} run: | mkdir -p "${PWD}/llvm-cov-raw" + # Wipe stray default.profraw the build step left behind (Makefiles + # run instrumented tools to dump interface XML) before the tests + # start writing controlled per-PID files. + find . -name 'default.profraw' -type f -delete # %p (process id) separates every subprocess; %9m caps the per-binary # pool at 512 files so long xdist runs don't fill the runner disk. { diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 084d6022070..0b96aec95e7 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -96,6 +96,10 @@ jobs: - name: Prepare coverage directories run: | mkdir -p "${PWD}/llvm-cov-raw" + # Wipe stray default.profraw the build step left behind (Makefiles + # run instrumented tools to dump interface XML), so their build- + # time data can't contaminate the test-run coverage. + find . -name 'default.profraw' -type f -delete # %p (process id) separates every subprocess; %9m caps the per-binary # pool at 512 files so long suites don't create millions of tiny # .profraw. Together they eliminate the write-collision that made diff --git a/.gitignore b/.gitignore index fd8c521b7c6..b4bcef7f870 100644 --- a/.gitignore +++ b/.gitignore @@ -87,6 +87,11 @@ lib/*/latex/ .coverage .coverage.* coverage.xml +*.profraw +*.profdata +coverage-llvm.lcov +llvm-cov-raw/ +coverage_html_report/ # Created by https://www.toptal.com/developers/gitignore/api/windows,linux,macos # Edit at https://www.toptal.com/developers/gitignore?templates=windows,linux,macos From baf475d957ffe44bdeeba8ca16c7c61bbcb58b5e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 22:50:04 +0000 Subject: [PATCH 03/22] CI: fix LLVM coverage non-determinism in OpenMP-parallel regions Adds -fprofile-update=atomic to the coverage CFLAGS on both macOS and Linux pytest builds. Without it, LLVM's coverage counter increments are non-atomic, and GRASS tools with #pragma omp parallel / for regions (e.g. r.univar's process_raster) lose counter updates across threads. Two identical runs against a locally-built, instrumented grass produced 244 differing lcov lines, all inside r.univar_main.c's outlined OpenMP region. This is the collision reported previously as coverage of untouched files changing between runs. Also tightens utils/llvm_coverage_export.sh so its -object list only picks up real ELF / Mach-O binaries. The previous perm -u+x sweep would pull in executable Python scripts (e.g. installed grass scripts), and llvm-cov aborts on the first one it cannot recognize. A full local rebuild + rerun is queued to verify the diff closes; if anything remains after -fprofile-update=atomic it will land as a follow-up commit on this branch. --- .github/workflows/build_ubuntu-24.04.sh | 5 ++++- .github/workflows/macos_install.sh | 5 ++++- utils/llvm_coverage_export.sh | 9 ++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index 0b55bd7621c..ef9d9865904 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -36,7 +36,10 @@ export INSTALL_PREFIX=$1 if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then export CC="${CC:-clang}" export CXX="${CXX:-clang++}" - COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -O0 -g" + # -fprofile-update=atomic makes coverage counter increments atomic; + # required for GRASS's OpenMP-parallel regions (e.g. r.univar) so + # threads racing on shared counters do not lose updates. + COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic -O0 -g" export CFLAGS="${CFLAGS:-} ${COV_FLAGS}" export CXXFLAGS="${CXXFLAGS:-} ${COV_FLAGS}" export LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" diff --git a/.github/workflows/macos_install.sh b/.github/workflows/macos_install.sh index 5b06daa06a9..a193e79344c 100755 --- a/.github/workflows/macos_install.sh +++ b/.github/workflows/macos_install.sh @@ -66,8 +66,11 @@ export CPPFLAGS="-isystem${CONDA_PREFIX}/include" # LLVM source-based coverage instrumentation, opt-in via env var. # -O0 keeps region mapping accurate; overhead is small compared to gcov. +# -fprofile-update=atomic makes coverage counter increments atomic; without +# it, OpenMP parallel regions (e.g. r.univar's process_raster) lose counter +# updates across threads and the merged .profdata is non-deterministic. if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then - COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -O0 -g" + COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic -O0 -g" CFLAGS="${CFLAGS} ${COV_FLAGS}" CXXFLAGS="${CXXFLAGS} ${COV_FLAGS}" LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" diff --git a/utils/llvm_coverage_export.sh b/utils/llvm_coverage_export.sh index 87f34ae676c..30434ba90b5 100755 --- a/utils/llvm_coverage_export.sh +++ b/utils/llvm_coverage_export.sh @@ -34,7 +34,14 @@ llvm-profdata merge -sparse -o "${OUT_PROFDATA}" "${RAW_DIR}"/*.profraw # On macOS the tools are Mach-O executables plus *.dylib; on Linux ELF plus *.so. OBJ_ARGS=() while IFS= read -r -d '' obj; do - OBJ_ARGS+=(-object "${obj}") + # Filter to real binaries (ELF on Linux, Mach-O on macOS). find -perm -u+x + # sweeps in executable Python and shell scripts, which llvm-cov rejects. + magic=$(head -c4 "${obj}" 2>/dev/null | od -An -tx1 | tr -d ' \n') + case "${magic}" in + 7f454c46) OBJ_ARGS+=(-object "${obj}") ;; # ELF + cffaedfe|cafebabe|feedface|feedfacf) \ + OBJ_ARGS+=(-object "${obj}") ;; # Mach-O + esac done < <( find "${GISBASE}" \ \( -type f -perm -u+x \ From 31a67473ce8c713d8da60c18319d49427b6ad53a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 23:03:29 +0000 Subject: [PATCH 04/22] CI: drop -O0 -g from LLVM coverage build; keep -O2 Clang's SourceBasedCodeCoverage doc explicitly states that llvm optimizations (inlining, CFG simplification) have no impact on coverage report quality: the mapping from source regions to profile counters is immutable and is generated before the optimizer runs, and the feature does not rely on debug info line tables. Force -O0 -g was gcov guidance, not the right choice here, and it inflated the instrumented build size and the runtime cost this PR promised to keep small. Also fix the Linux path: setting CFLAGS explicitly at configure time suppresses autoconf's default "-g -O2", so include -O2 in the coverage CFLAGS (macOS already sets -O2 explicitly before the coverage block). --- .github/workflows/build_ubuntu-24.04.sh | 20 ++++++++++++++------ .github/workflows/macos_install.sh | 14 ++++++++------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index ef9d9865904..cc0a69c21e6 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -36,12 +36,20 @@ export INSTALL_PREFIX=$1 if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then export CC="${CC:-clang}" export CXX="${CXX:-clang++}" - # -fprofile-update=atomic makes coverage counter increments atomic; - # required for GRASS's OpenMP-parallel regions (e.g. r.univar) so - # threads racing on shared counters do not lose updates. - COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic -O0 -g" - export CFLAGS="${CFLAGS:-} ${COV_FLAGS}" - export CXXFLAGS="${CXXFLAGS:-} ${COV_FLAGS}" + # LLVM source-based coverage generates its region mapping before the + # optimizer runs, so optimization does not affect coverage quality + # (clang doc: SourceBasedCodeCoverage, "Impact of llvm optimizations + # on coverage reports"). Keep the project's normal -O2 to hold the + # runtime impact small. -fprofile-update=atomic makes counter + # increments atomic; required for GRASS's OpenMP-parallel regions + # (e.g. r.univar) so threads racing on shared counters do not lose + # updates. + # + # Setting CFLAGS explicitly at configure time suppresses autoconf's + # default "-g -O2", so pass -O2 here. + COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic" + export CFLAGS="${CFLAGS:--O2} ${COV_FLAGS}" + export CXXFLAGS="${CXXFLAGS:--O2} ${COV_FLAGS}" export LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" fi diff --git a/.github/workflows/macos_install.sh b/.github/workflows/macos_install.sh index a193e79344c..2ad59f2f1d8 100755 --- a/.github/workflows/macos_install.sh +++ b/.github/workflows/macos_install.sh @@ -64,13 +64,15 @@ export CFLAGS="-O2 -pipe -ffp-contract=off -arch ${CONDA_ARCH} -DGL_SILENCE_DEPR export CXXFLAGS="-O2 -pipe -ffp-contract=off -stdlib=libc++ -arch ${CONDA_ARCH} -Wall -Wextra -Wpedantic" export CPPFLAGS="-isystem${CONDA_PREFIX}/include" -# LLVM source-based coverage instrumentation, opt-in via env var. -# -O0 keeps region mapping accurate; overhead is small compared to gcov. -# -fprofile-update=atomic makes coverage counter increments atomic; without -# it, OpenMP parallel regions (e.g. r.univar's process_raster) lose counter -# updates across threads and the merged .profdata is non-deterministic. +# LLVM source-based coverage instrumentation, opt-in via env var. The +# coverage-mapping section is generated before the LLVM optimizer runs, so +# optimization does not affect coverage quality (per clang's +# SourceBasedCodeCoverage docs) - keep the project's -O2. -fprofile-update= +# atomic makes counter increments atomic; without it, OpenMP parallel +# regions (e.g. r.univar's process_raster) lose counter updates across +# threads and the merged .profdata is non-deterministic. if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then - COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic -O0 -g" + COV_FLAGS="-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic" CFLAGS="${CFLAGS} ${COV_FLAGS}" CXXFLAGS="${CXXFLAGS} ${COV_FLAGS}" LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" From 4746645a040491d6eeb8dc0d8f658a235aeef5d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 23:41:35 +0000 Subject: [PATCH 05/22] CI: point configure at clang's omp.h when coverage build uses clang The pytest workflow's Ubuntu build failed at configure with "Unable to locate OpenMP includes" once GRASS_LLVM_COVERAGE=1 flipped the compiler to clang. Ubuntu's libomp-dev keeps omp.h at /usr/lib/llvm-18/lib/clang//include/omp.h rather than /usr/include, and configure's plain omp.h probe doesn't add clang's own resource-dir to its include search. Fix by passing --with-openmp-includes derived from `$CC -print-resource-dir` when the coverage build is active. Also add libomp-dev to apt.txt so the runtime lib is linkable. Make the Python coverage combine step tolerate "no data" so that when an earlier step fails and pytest is skipped, the combine step reports "nothing to export" instead of failing on top of the real error. --- .github/workflows/apt.txt | 1 + .github/workflows/build_ubuntu-24.04.sh | 13 ++++++++++++- .github/workflows/pytest.yml | 20 +++++++++++++------- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/.github/workflows/apt.txt b/.github/workflows/apt.txt index 814545569f1..45d1a262994 100644 --- a/.github/workflows/apt.txt +++ b/.github/workflows/apt.txt @@ -15,6 +15,7 @@ libpdal-dev libpng-dev libproj-dev libreadline-dev +libomp-dev libsvm-dev libzstd-dev llvm diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index cc0a69c21e6..58648cadce8 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -30,6 +30,9 @@ set -u export INSTALL_PREFIX=$1 +# Extra configure args added under coverage; keep declared under `set -u`. +OPENMP_ARGS=() + # LLVM source-based coverage instrumentation, opt-in via env var. Requires # clang (installed via apt.txt). Kept out of the default O2 path so # non-coverage runs are unaffected. @@ -51,6 +54,13 @@ if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then export CFLAGS="${CFLAGS:--O2} ${COV_FLAGS}" export CXXFLAGS="${CXXFLAGS:--O2} ${COV_FLAGS}" export LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" + # Clang ships omp.h in its resource-dir instead of /usr/include, so + # configure's plain omp.h check misses it. Point configure at both + # the include and (from apt libomp-dev) the runtime lib. + CLANG_RES_DIR="$("${CC}" -print-resource-dir 2>/dev/null || true)" + if [ -n "${CLANG_RES_DIR}" ] && [ -d "${CLANG_RES_DIR}/include" ]; then + OPENMP_ARGS+=("--with-openmp-includes=${CLANG_RES_DIR}/include") + fi fi ./configure \ @@ -73,7 +83,8 @@ fi --with-readline \ --with-sqlite \ --with-tiff \ - --with-zstd + --with-zstd \ + ${OPENMP_ARGS[@]+"${OPENMP_ARGS[@]}"} eval $makecmd make install diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 0b96aec95e7..dedcfbaad78 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -157,14 +157,20 @@ jobs: run: | # coverage combine must run before coverage_mapper.py: parallel # mode leaves .coverage... partials and no - # .coverage file for the mapper to read. - coverage combine - python utils/coverage_mapper.py - if [ -f .coverage.fixed_scripts ]; then - mv .coverage.fixed_scripts .coverage + # .coverage file for the mapper to read. Tolerate "no data" so + # a build failure earlier in the job does not mask itself as a + # later failure here. + coverage combine || true + if [ -f .coverage ]; then + python utils/coverage_mapper.py + if [ -f .coverage.fixed_scripts ]; then + mv .coverage.fixed_scripts .coverage + fi + coverage html + coverage xml -o coverage.xml + else + echo "No .coverage data collected; skipping Python coverage export." fi - coverage html - coverage xml -o coverage.xml - name: Merge and export LLVM C/C++ coverage if: ${{ !cancelled() }} From 35bf0fe42cdba31721ac608800675faf184830b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:31:02 +0000 Subject: [PATCH 06/22] CI: point configure at clang's libomp path so tools link with -lomp The pytest workflow's Ubuntu build now clears the OpenMP-include probe but fails to link every OpenMP-using tool (r.mapcalc, r.univar, r.slope.aspect, r.sun, ...) once GRASS_LLVM_COVERAGE=1 flipped the compiler to clang. Root cause: libomp-dev on Ubuntu 24.04 installs the unversioned libomp.so at /usr/lib/llvm-18/lib, which is not in the linker's default search path. configure's link probe for omp_get_num_threads in -lomp therefore fails and it falls back to -lgomp - but clang's OpenMP-instrumented code emits __kmpc_* calls that gomp doesn't provide. Pass --with-openmp-libs= so configure finds -lomp and every OMP tool links cleanly. The libdir is derived from clang -print-resource-dir so the fix is not tied to any specific LLVM version number. --- .github/workflows/build_ubuntu-24.04.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index 58648cadce8..86f5a57be59 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -60,6 +60,16 @@ if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then CLANG_RES_DIR="$("${CC}" -print-resource-dir 2>/dev/null || true)" if [ -n "${CLANG_RES_DIR}" ] && [ -d "${CLANG_RES_DIR}/include" ]; then OPENMP_ARGS+=("--with-openmp-includes=${CLANG_RES_DIR}/include") + # Debian's libomp-dev installs libomp.so under /usr/lib/llvm-/lib, + # not the linker's default search path, so configure's link probe for + # omp_get_num_threads fails and it falls back to -lgomp - which clang's + # OpenMP-instrumented code (using __kmpc_*) then can't link against. + # The resource-dir sits at /clang/, so its + # grandparent is , where libomp.so lives. + LLVM_LIB_DIR="$(cd "${CLANG_RES_DIR}/../.." && pwd)" + if [ -e "${LLVM_LIB_DIR}/libomp.so" ]; then + OPENMP_ARGS+=("--with-openmp-libs=${LLVM_LIB_DIR}") + fi fi fi From b74940ae7cc5466fc91ae8d47dad18b778f97b85 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:42:30 +0000 Subject: [PATCH 07/22] CI: install clang/llvm/libomp only when LLVM coverage is on Adding clang, llvm, and libomp-dev to the shared apt.txt slowed down every workflow that reads that list (ubuntu.yml, gcc.yml, codeql, cmake, docs...) for a toolchain none of them need. Move them into a new "Install LLVM coverage toolchain" step in pytest.yml, gated on env.GRASS_LLVM_COVERAGE == '1', so only the coverage matrix pays the apt cost. --- .github/workflows/apt.txt | 3 --- .github/workflows/pytest.yml | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/apt.txt b/.github/workflows/apt.txt index 45d1a262994..50fc0fa8b84 100644 --- a/.github/workflows/apt.txt +++ b/.github/workflows/apt.txt @@ -1,6 +1,5 @@ bison build-essential -clang flex libbz2-dev libcairo-dev @@ -15,10 +14,8 @@ libpdal-dev libpng-dev libproj-dev libreadline-dev -libomp-dev libsvm-dev libzstd-dev -llvm pdal proj-bin python3-dateutil diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index dedcfbaad78..904965a75c0 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -57,6 +57,12 @@ jobs: xargs -a <(awk '! /^ *(#|$)/' ".github/workflows/apt.txt") -r -- \ sudo apt-get install -y --no-install-recommends --no-install-suggests + - name: Install LLVM coverage toolchain + if: env.GRASS_LLVM_COVERAGE == '1' + run: | + sudo apt-get install -y --no-install-recommends --no-install-suggests \ + clang llvm libomp-dev + - uses: rui314/setup-mold@9c9c13bf4c3f1adef0cc596abc155580bcb04444 # v1 - name: Install Python dependencies From 99ac2e3b171abb6d5d756d828d962bd4f9923ca9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:44:18 +0000 Subject: [PATCH 08/22] CI: only run LLVM C coverage on the latest Python matrix cell Instrumenting the build costs ~1 min extra compile time and adds a merge/export step. Running it in every matrix cell duplicated all of that for no additional information. Gate GRASS_LLVM_COVERAGE, the llvm/libomp toolchain install, the merge/export step, and the ubuntu-llvm-c Codecov upload on matrix.python-version == '3.13' so the 3.11 cell now takes the fast gcc/no-instrumentation path. Python coverage still runs on both cells and continues to upload under its own per-version flag. --- .github/workflows/pytest.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 904965a75c0..b1bdd900c72 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -32,9 +32,9 @@ jobs: FORCE_COLOR: 1 # for software including pip: https://force-color.org/ CLICOLOR_FORCE: 1 # for other software including ninja: https://bixense.com/clicolors/ PYTHONWARNINGS: always - GRASS_LLVM_COVERAGE: "1" - CC: clang - CXX: clang++ + # Only exercise LLVM C coverage on the latest Python; the older one + # runs the default gcc/no-instrumentation build for speed. + GRASS_LLVM_COVERAGE: ${{ matrix.python-version == '3.13' && '1' || '0' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -179,7 +179,7 @@ jobs: fi - name: Merge and export LLVM C/C++ coverage - if: ${{ !cancelled() }} + if: ${{ !cancelled() && env.GRASS_LLVM_COVERAGE == '1' }} run: utils/llvm_coverage_export.sh - name: Upload test results to Codecov @@ -201,7 +201,7 @@ jobs: token: ${{ secrets.CODECOV_TOKEN }} - name: Upload LLVM C/C++ coverage to Codecov - if: ${{ !cancelled() }} + if: ${{ !cancelled() && env.GRASS_LLVM_COVERAGE == '1' }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: verbose: true From 98f2e778c582b98177aa7d14e3288acb0164b5b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 15:40:20 +0000 Subject: [PATCH 09/22] CI: fix llvm-profdata "Argument list too long" in coverage export utils/llvm_coverage_export.sh globbed all .profraw files onto llvm-profdata's argv. The per-process %p-%9m naming produces one file per (binary, pid) pair, and a full test run generates hundreds of thousands of them (454754 on the macOS job that failed), which overflows the OS argument-list limit on both the Linux pytest and macOS jobs. Pass the file list via llvm-profdata's -f option (a newline-separated list file) instead of argv. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01P53N6mt8mDNedgUDBcYqfW --- utils/llvm_coverage_export.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/utils/llvm_coverage_export.sh b/utils/llvm_coverage_export.sh index 30434ba90b5..ed5ed9c485b 100755 --- a/utils/llvm_coverage_export.sh +++ b/utils/llvm_coverage_export.sh @@ -27,8 +27,15 @@ if ! compgen -G "${RAW_DIR}/*.profraw" > /dev/null; then fi # -sparse drops zero-count regions so merges are smaller and deterministic -# for a given input set. -llvm-profdata merge -sparse -o "${OUT_PROFDATA}" "${RAW_DIR}"/*.profraw +# for a given input set. The per-process "%p-%9m" naming can produce +# hundreds of thousands of .profraw files in a full test run, so pass them +# via -f (a newline-separated list) rather than shell-globbing them onto +# argv, which overflows the OS argument-list limit ("Argument list too +# long"). +PROFRAW_LIST="$(mktemp)" +trap 'rm -f "${PROFRAW_LIST}"' EXIT +find "${RAW_DIR}" -name '*.profraw' -print > "${PROFRAW_LIST}" +llvm-profdata merge -sparse -f "${PROFRAW_LIST}" -o "${OUT_PROFDATA}" # Enumerate every instrumented binary and shared library the install produced. # On macOS the tools are Mach-O executables plus *.dylib; on Linux ELF plus *.so. From 8a917ce2252fac48823069fdb3dcc960f3e05bd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 17:24:36 +0000 Subject: [PATCH 10/22] CI: skip gpg validation for macOS coverage Codecov uploads The macOS job's "Uninstalling Homebrew" step removes gpg early on, same as it always has. The pre-existing "Upload test results to Codecov" step already sets skip_validation: true to work around this, but the two new coverage-upload steps this PR added did not, so codecov-action's own dependency check failed before it could upload anything. Match the existing step's skip_validation: true. --- .github/workflows/macos.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index e76e81eed8c..81d34baaf0d 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -231,6 +231,7 @@ jobs: flags: macos-pytest-python name: macos-pytest-python token: ${{ secrets.CODECOV_TOKEN }} + skip_validation: true - name: Upload LLVM C/C++ coverage to Codecov (macOS) if: ${{ !cancelled() }} @@ -241,6 +242,7 @@ jobs: flags: macos-llvm-c name: macos-llvm-c token: ${{ secrets.CODECOV_TOKEN }} + skip_validation: true - name: Make python-only code coverage test report available if: ${{ !cancelled() }} From 20260ae7a40eb0a3b67b18cca01ddd5b1da97c78 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:45:30 +0000 Subject: [PATCH 11/22] CI: prototype SanitizerCoverage as an alternative to LLVM source-based coverage Adds an opt-in GRASS_SANCOV_COVERAGE / WITH_SANCOV_COVERAGE path (Autotools and CMake) that instruments GRASS with -fsanitize-coverage= on GCC (trace-pc) or Clang (trace-pc-guard) and produces coverage dumps via a small custom runtime (utils/sancov_runtime.c) and symbolizer (utils/sancov_symbolize.py), alongside the existing clang source-based mechanism, for comparison. See doc/development/sancov_coverage_experiment.md for the investigation, measurements, and the resulting recommendation. Most of this was worked out through iterative, empirical debugging of real link and toolchain failures (documented inline): GCC's -fsanitize-coverage= does not implement trace-pc-guard, GNU ld's left-to-right link order breaks a naive LDFLAGS-based runtime link, CMake's install(EXPORT ...) rejects a non-exported link dependency, and so on. Substantial parts of this change, including the debugging process itself, were produced with AI assistance. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- .github/workflows/build_ubuntu-24.04.sh | 125 +++++++++++++++++--- .github/workflows/cmake.yml | 48 +++++++- .github/workflows/pytest.yml | 55 ++++++++- .gitignore | 3 + CMakeLists.txt | 65 +++++++++++ utils/sancov_runtime.c | 144 ++++++++++++++++++++++++ utils/sancov_symbolize.py | 114 +++++++++++++++++++ 7 files changed, 527 insertions(+), 27 deletions(-) create mode 100644 utils/sancov_runtime.c create mode 100755 utils/sancov_symbolize.py diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index 86f5a57be59..3512eacc6e6 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -33,6 +33,32 @@ export INSTALL_PREFIX=$1 # Extra configure args added under coverage; keep declared under `set -u`. OPENMP_ARGS=() +# Clang ships omp.h in its resource-dir instead of /usr/include, so +# configure's plain omp.h check misses it. Point configure at both the +# include and (from apt libomp-dev) the runtime lib. Shared by both +# clang-based coverage mechanisms below. +configure_clang_openmp_args() { + CLANG_RES_DIR="$(clang -print-resource-dir 2>/dev/null || true)" + if [ -n "${CLANG_RES_DIR}" ] && [ -d "${CLANG_RES_DIR}/include" ]; then + OPENMP_ARGS+=("--with-openmp-includes=${CLANG_RES_DIR}/include") + # Debian's libomp-dev installs libomp.so under /usr/lib/llvm-/lib, + # not the linker's default search path, so configure's link probe for + # omp_get_num_threads fails and it falls back to -lgomp - which clang's + # OpenMP-instrumented code (using __kmpc_*) then can't link against. + # The resource-dir sits at /clang/, so its + # grandparent is , where libomp.so lives. + LLVM_LIB_DIR="$(cd "${CLANG_RES_DIR}/../.." && pwd)" + if [ -e "${LLVM_LIB_DIR}/libomp.so" ]; then + OPENMP_ARGS+=("--with-openmp-libs=${LLVM_LIB_DIR}") + fi + fi +} + +if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ] && [ "${GRASS_SANCOV_COVERAGE:-0}" = "1" ]; then + echo "GRASS_LLVM_COVERAGE and GRASS_SANCOV_COVERAGE are mutually exclusive" >&2 + exit 1 +fi + # LLVM source-based coverage instrumentation, opt-in via env var. Requires # clang (installed via apt.txt). Kept out of the default O2 path so # non-coverage runs are unaffected. @@ -54,23 +80,88 @@ if [ "${GRASS_LLVM_COVERAGE:-0}" = "1" ]; then export CFLAGS="${CFLAGS:--O2} ${COV_FLAGS}" export CXXFLAGS="${CXXFLAGS:--O2} ${COV_FLAGS}" export LDFLAGS="${LDFLAGS:-} -fprofile-instr-generate -fcoverage-mapping" - # Clang ships omp.h in its resource-dir instead of /usr/include, so - # configure's plain omp.h check misses it. Point configure at both - # the include and (from apt libomp-dev) the runtime lib. - CLANG_RES_DIR="$("${CC}" -print-resource-dir 2>/dev/null || true)" - if [ -n "${CLANG_RES_DIR}" ] && [ -d "${CLANG_RES_DIR}/include" ]; then - OPENMP_ARGS+=("--with-openmp-includes=${CLANG_RES_DIR}/include") - # Debian's libomp-dev installs libomp.so under /usr/lib/llvm-/lib, - # not the linker's default search path, so configure's link probe for - # omp_get_num_threads fails and it falls back to -lgomp - which clang's - # OpenMP-instrumented code (using __kmpc_*) then can't link against. - # The resource-dir sits at /clang/, so its - # grandparent is , where libomp.so lives. - LLVM_LIB_DIR="$(cd "${CLANG_RES_DIR}/../.." && pwd)" - if [ -e "${LLVM_LIB_DIR}/libomp.so" ]; then - OPENMP_ARGS+=("--with-openmp-libs=${LLVM_LIB_DIR}") - fi - fi + configure_clang_openmp_args +fi + +# SanitizerCoverage instrumentation, opt-in via env var, compared against +# GRASS_LLVM_COVERAGE above -- see doc/development/ +# sancov_coverage_experiment.md. GRASS_SANCOV_CC selects gcc (the only +# -fsanitize-coverage= sub-option GCC 13/14 implement is trace-pc, no +# per-edge guard: confirmed empirically, see the report) or clang +# (trace-pc-guard, cheaper steady-state dedup). +# +# Unlike -fprofile-instr-generate above, -fsanitize-coverage= is not +# special-cased by either compiler's driver to auto-link a runtime, so +# utils/sancov_runtime.c is built here into a small shared library that +# every GRASS binary must link against. Naively exporting "-lsancovrt" +# through LDFLAGS or configure's $LIBS does not reliably reach every link +# GRASS's build performs: GRASS's own Make templates (Shlib.make, +# Compile.make) place $(LDFLAGS) *before* the object files (GNU ld +# resolves left to right and never revisits a library once passed, so an +# undefined reference there is never satisfied), a few special-cased +# link rules (e.g. lib/init/Makefile's echo/run targets) do not go +# through the usual $(LIBES)/$(MATHLIB) machinery those templates share +# at all, and configure's own custom library probes (LOC_CHECK_LIBS, +# used for BLAS, GDAL, PROJ, and most other optional dependencies) build +# their own bespoke compile-and-link commands rather than going through +# autoconf's usual $LIBS (confirmed empirically, in that order, each by +# a different otherwise-unrelated-looking failure: an "undefined +# reference" from ld, then a segfault when it was papered over with +# -Wl,--unresolved-symbols=ignore-all, then an unrelated-looking "unable +# to link to (C)BLAS", then a silently-unlinked etc/run). +# +# Rather than chase each of those individually, $CC/$CXX are pointed at +# a tiny wrapper that appends the runtime's -L/-l/-rpath flags to +# whatever argv it is given, verbatim, then execs the real compiler. +# That covers every link GRASS's build performs, however it constructs +# the rest of the command line, because they all ultimately invoke +# $(CC)/$(CXX) (or $(LINK), which itself defaults to $(CC)) as the +# actual compiler front-end. The same flags are harmless noise on a +# compile-only invocation (-c): gcc and clang both silently ignore +# unused -l/-L/-Wl, arguments there (confirmed empirically), so the one +# wrapper is safe to use for every invocation, compile or link, without +# needing to tell them apart -- including configure's own, so the +# instrumentation flags can go back to being set the same way +# GRASS_LLVM_COVERAGE sets them above, before ./configure runs. +if [ "${GRASS_SANCOV_COVERAGE:-0}" = "1" ]; then + SANCOV_CC="${GRASS_SANCOV_CC:-gcc}" + case "${SANCOV_CC}" in + gcc) + REAL_CC="gcc" + REAL_CXX="g++" + SANCOV_FLAGS="-fsanitize-coverage=trace-pc" + ;; + clang) + REAL_CC="clang" + REAL_CXX="clang++" + SANCOV_FLAGS="-fsanitize-coverage=trace-pc-guard" + configure_clang_openmp_args + ;; + *) + echo "GRASS_SANCOV_CC must be gcc or clang, got '${SANCOV_CC}'" >&2 + exit 1 + ;; + esac + + SANCOV_RUNTIME_DIR="${PWD}/sancov-runtime" + mkdir -p "${SANCOV_RUNTIME_DIR}" + "${REAL_CC}" -O2 -fPIC -shared -pthread \ + -o "${SANCOV_RUNTIME_DIR}/libsancovrt.so" \ + utils/sancov_runtime.c -ldl + + cat > "${SANCOV_RUNTIME_DIR}/cc-wrapper.sh" < "${SANCOV_RUNTIME_DIR}/cxx-wrapper.sh" <- + ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }}-${{ + matrix.sancov }} + cancel-in-progress: true + strategy: + matrix: + # sancov: gcc builds with -DWITH_SANCOV_COVERAGE=ON (opt-in + # SanitizerCoverage experiment, see doc/development/ + # sancov_coverage_experiment.md) alongside the existing default + # (uninstrumented) cell, gcc being what this workflow already + # builds with. + sancov: + - none + - gcc + fail-fast: true runs-on: ubuntu-24.04 env: CMAKE_VERSION: "3.22.0" + GRASS_SANCOV_CMAKE_ARG: ${{ matrix.sancov == 'gcc' && '-DWITH_SANCOV_COVERAGE=ON' || '' }} steps: - name: Checkout GRASS uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -74,7 +91,7 @@ jobs: run: | cmake -S "${GITHUB_WORKSPACE}" -B "${GITHUB_WORKSPACE}/build" -G Ninja \ -DCMAKE_INSTALL_PREFIX="${HOME}/install" -DWITH_READLINE=ON -DWITH_ODBC=ON \ - -DWITH_NETCDF=ON -DWITH_BZLIB=ON + -DWITH_NETCDF=ON -DWITH_BZLIB=ON ${GRASS_SANCOV_CMAKE_ARG} - name: Print CMakeCache.txt shell: bash -el {0} run: | @@ -93,12 +110,31 @@ jobs: run: .github/workflows/print_versions.sh - name: Test executing of the grass command run: .github/workflows/test_simple.sh + - name: Prepare SanitizerCoverage dump directory + if: matrix.sancov == 'gcc' + run: | + mkdir -p "${PWD}/sancov-raw" + echo "GRASS_SANCOV_DUMP_DIR=${PWD}/sancov-raw" >> "${GITHUB_ENV}" - name: Run tests run: .github/workflows/test_thorough.sh --config .gunittest.cfg --min-success 100 + - name: Symbolize and summarize SanitizerCoverage + if: ${{ !cancelled() && matrix.sancov == 'gcc' }} + run: | + python3 utils/sancov_symbolize.py "${GRASS_SANCOV_DUMP_DIR}" \ + --ignore-path-substring /usr/ \ + > sancov-coverage-summary.txt + wc -l sancov-coverage-summary.txt - name: Make HTML test report available if: ${{ !cancelled() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: testreport-CMake + name: testreport-CMake-${{ matrix.sancov }} path: testreport retention-days: 3 + - name: Make SanitizerCoverage summary available + if: ${{ !cancelled() && matrix.sancov == 'gcc' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: sancov-coverage-summary-cmake + path: sancov-coverage-summary.txt + retention-days: 7 diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 4895d0f8eec..8ba135bacf2 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -7,6 +7,10 @@ on: - main - releasebranch_* pull_request: + # Lets this branch's coverage-mechanism comparison be run on demand + # without opening a PR, see doc/development/ + # sancov_coverage_experiment.md. Remove before merging. + workflow_dispatch: permissions: {} @@ -25,6 +29,21 @@ jobs: python-version: - "3.11" - "3.13" + # No coverage instrumentation on the default cell (kept fast); the + # clang source-based mechanism is compared here against two + # SanitizerCoverage prototypes (one per compiler) on the same + # Python version, see doc/development/ + # sancov_coverage_experiment.md. All three coverage cells reuse + # 3.13 rather than adding yet more cells on top of 3.11's. + coverage: + - none + include: + - python-version: "3.13" + coverage: llvm-source + - python-version: "3.13" + coverage: sancov-gcc + - python-version: "3.13" + coverage: sancov-clang fail-fast: true runs-on: ${{ matrix.os }} @@ -32,9 +51,9 @@ jobs: FORCE_COLOR: 1 # for software including pip: https://force-color.org/ CLICOLOR_FORCE: 1 # for other software including ninja: https://bixense.com/clicolors/ PYTHONWARNINGS: always - # Only exercise LLVM C coverage on the latest Python; the older one - # runs the default gcc/no-instrumentation build for speed. - GRASS_LLVM_COVERAGE: ${{ matrix.python-version == '3.13' && '1' || '0' }} + GRASS_LLVM_COVERAGE: ${{ matrix.coverage == 'llvm-source' && '1' || '0' }} + GRASS_SANCOV_COVERAGE: ${{ (matrix.coverage == 'sancov-gcc' || matrix.coverage == 'sancov-clang') && '1' || '0' }} + GRASS_SANCOV_CC: ${{ matrix.coverage == 'sancov-clang' && 'clang' || 'gcc' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -58,7 +77,7 @@ jobs: sudo apt-get install -y --no-install-recommends --no-install-suggests - name: Install LLVM coverage toolchain - if: env.GRASS_LLVM_COVERAGE == '1' + if: env.GRASS_LLVM_COVERAGE == '1' || env.GRASS_SANCOV_CC == 'clang' run: | sudo apt-get install -y --no-install-recommends --no-install-suggests \ clang llvm libomp-dev @@ -115,6 +134,10 @@ jobs: echo "INITIAL_GISBASE=$(grass --config path)" echo "INITIAL_PWD=${PWD}" } >> "${GITHUB_ENV}" + if [ "${GRASS_SANCOV_COVERAGE}" = "1" ]; then + mkdir -p "${PWD}/sancov-raw" + echo "GRASS_SANCOV_DUMP_DIR=${PWD}/sancov-raw" >> "${GITHUB_ENV}" + fi - name: Run pytest with multiple workers in parallel run: | @@ -182,6 +205,18 @@ jobs: if: ${{ !cancelled() && env.GRASS_LLVM_COVERAGE == '1' }} run: utils/llvm_coverage_export.sh + - name: Symbolize and summarize SanitizerCoverage + if: ${{ !cancelled() && env.GRASS_SANCOV_COVERAGE == '1' }} + run: | + python utils/sancov_symbolize.py "${GRASS_SANCOV_DUMP_DIR}" \ + --ignore-path-substring /usr/ \ + --ignore-path-substring /conda- \ + --ignore-path-substring /miniconda \ + --ignore-path-substring /micromamba \ + > sancov-coverage-summary.txt + du -sh "${GRASS_SANCOV_DUMP_DIR}" + wc -l sancov-coverage-summary.txt + - name: Upload test results to Codecov if: ${{ !cancelled() }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 @@ -217,6 +252,18 @@ jobs: path: coverage_html_report retention-days: 1 + - name: Make SanitizerCoverage summary available + if: ${{ !cancelled() && env.GRASS_SANCOV_COVERAGE == '1' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: sancov-coverage-summary-${{ matrix.coverage }} + # Not uploaded to Codecov: this is a line-hit-count summary, not + # lcov, produced for this experiment's own comparison rather + # than as an ongoing coverage-tracking mechanism -- see + # doc/development/sancov_coverage_experiment.md. + path: sancov-coverage-summary.txt + retention-days: 7 + pytest-success: name: pytest Result needs: diff --git a/.gitignore b/.gitignore index b4bcef7f870..77681094182 100644 --- a/.gitignore +++ b/.gitignore @@ -92,6 +92,9 @@ coverage.xml coverage-llvm.lcov llvm-cov-raw/ coverage_html_report/ +sancov-runtime/ +sancov-raw/ +sancov-coverage-summary.txt # Created by https://www.toptal.com/developers/gitignore/api/windows,linux,macos # Edit at https://www.toptal.com/developers/gitignore?templates=windows,linux,macos diff --git a/CMakeLists.txt b/CMakeLists.txt index 98232c9258f..d9bd2c19c68 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -165,6 +165,11 @@ option( option(WITH_DOCS "Build documentation" ON) option(WITH_GUI "Build GUI" ON) option(WITH_FHS "Install with FHS" OFF) +option( + WITH_SANCOV_COVERAGE + "Build with SanitizerCoverage instrumentation (opt-in coverage experiment, see doc/development/sancov_coverage_experiment.md)" + OFF +) include(check_target) include(CheckDependentLibraries) @@ -249,6 +254,66 @@ include(set_compiler_flags) set_compiler_flags() include(Configure) +# SanitizerCoverage instrumentation, opt-in via -DWITH_SANCOV_COVERAGE=ON -- +# see doc/development/sancov_coverage_experiment.md for the pytest.yml +# equivalent (GRASS_SANCOV_COVERAGE) and the reasoning behind it. Unlike +# there, this can be added directly through CMake's own dependency-graph +# link mechanism instead of a hand-built shared library plus a Makefile +# link-order workaround: link_libraries() below records "every target +# defined from this point on links against sancovrt", and CMake always +# places a target_link_libraries dependency after that target's own +# object files in the generated link command, so ordering is not a +# concern the way it is for GRASS's own hand-written Make templates. This +# must run after project() above: passing the instrumentation flag via +# -DCMAKE_C_FLAGS on the command line instead would break project()'s own +# internal compiler-works probe the same way it breaks configure's +# probes in the Autotools build (confirmed empirically), because that +# probe also can't resolve the coverage callback symbols. +if(WITH_SANCOV_COVERAGE) + if(CMAKE_C_COMPILER_ID STREQUAL "GNU") + # GCC 13/14 implement only trace-cmp and trace-pc for + # -fsanitize-coverage=, not trace-pc-guard (confirmed + # empirically); trace-pc has no per-edge guard word, so + # sancov_runtime.c dedups it itself via a locked hash set. + set(SANCOV_COVERAGE_FLAG "-fsanitize-coverage=trace-pc") + elseif(CMAKE_C_COMPILER_ID MATCHES "Clang") + set(SANCOV_COVERAGE_FLAG "-fsanitize-coverage=trace-pc-guard") + else() + message( + FATAL_ERROR + "WITH_SANCOV_COVERAGE is only supported with GCC or Clang, got ${CMAKE_C_COMPILER_ID}" + ) + endif() + # sancovrt is built directly with the compiler (add_custom_command), + # not as a normal add_library() target: GRASS's own libraries are + # exported for g.extension's addon-building CMake package (every + # install(EXPORT ...) call further down requires each of its + # targets' link dependencies to themselves be in an export set), and + # a plain add_library(sancovrt SHARED ...) pulled in here as a common + # link_libraries() dependency of all of them would have to join that + # export set too, which it has no reason to be part of (confirmed + # empirically: CMake's install(EXPORT ...) step refuses to generate + # otherwise). An IMPORTED library is exempt from that requirement. + set(SANCOV_RUNTIME_LIB "${CMAKE_BINARY_DIR}/libsancovrt${CMAKE_SHARED_LIBRARY_SUFFIX}") + add_custom_command( + OUTPUT ${SANCOV_RUNTIME_LIB} + COMMAND ${CMAKE_C_COMPILER} -O2 -fPIC -shared -pthread + -o ${SANCOV_RUNTIME_LIB} + ${CMAKE_SOURCE_DIR}/utils/sancov_runtime.c -l${CMAKE_DL_LIBS} + DEPENDS ${CMAKE_SOURCE_DIR}/utils/sancov_runtime.c + COMMENT "Building SanitizerCoverage runtime" + ) + add_custom_target(sancovrt_build DEPENDS ${SANCOV_RUNTIME_LIB}) + add_library(sancovrt SHARED IMPORTED GLOBAL) + set_target_properties( + sancovrt PROPERTIES IMPORTED_LOCATION ${SANCOV_RUNTIME_LIB} + ) + add_dependencies(sancovrt sancovrt_build) + install(FILES ${SANCOV_RUNTIME_LIB} DESTINATION ${GRASS_INSTALL_LIBDIR}) + add_compile_options(-g ${SANCOV_COVERAGE_FLAG}) + link_libraries(sancovrt) +endif() + include(repo_status) repo_status("${CMAKE_CURRENT_LIST_DIR}") diff --git a/utils/sancov_runtime.c b/utils/sancov_runtime.c new file mode 100644 index 00000000000..6b2f1ab3e9b --- /dev/null +++ b/utils/sancov_runtime.c @@ -0,0 +1,144 @@ +/* Minimal SanitizerCoverage runtime for GRASS's opt-in GRASS_SANCOV_COVERAGE + * build (see build_ubuntu-24.04.sh and doc/development/ + * sancov_coverage_experiment.md). Compiled once into a small shared + * library and linked into every GRASS binary via LDFLAGS, so it does not + * depend on -fsanitize=fuzzer[-no-link] or any compiler-rt component. + * + * -fsanitize-coverage= supports two different edge-instrumentation ABIs + * depending on the compiler and chosen sub-option, so this file provides + * both callback sets; whichever one is unused by a given build is simply + * never called. + * + * Clang, -fsanitize-coverage=trace-pc-guard: the compiler gives each edge + * a private guard word (via __sanitizer_cov_trace_pc_guard_init) and + * calls __sanitizer_cov_trace_pc_guard(&guard) on every execution. This + * runtime dedups by zeroing the guard on first hit, so after that the + * compiler-emitted check is a load and a branch with no call -- the same + * strategy libFuzzer's own runtime uses to keep steady-state overhead + * low. + * + * GCC, -fsanitize-coverage=trace-pc: GCC 13/14 do not implement + * trace-pc-guard (confirmed empirically; only trace-cmp and trace-pc are + * accepted). trace-pc calls __sanitizer_cov_trace_pc(void) with no + * per-edge state at all, so this runtime does its own dedup via a + * mutex-guarded hash set keyed by return address -- a hash probe on every + * single edge execution, not just the first, which is the mechanism- + * inherent overhead difference from the guard-based path. + * + * Neither path uses -fsanitize-coverage=pc-table (GCC has no equivalent, + * and Clang's needs the -fsanitize=fuzzer[-no-link] runtime to consume + * it). Instead each callback records __builtin_return_address(0): the + * address right after the guard/trace call, i.e. the edge location + * itself, which a symbolizer can resolve directly with addr2line -- see + * utils/sancov_symbolize.py. + * + * On process exit (a destructor, so no changes to GRASS's own exit path + * are needed), each hit address is resolved to its containing shared + * object via dladdr() and written as a module-relative offset to a + * per-process dump file, mirroring the profraw-per-process pattern the + * existing GRASS_LLVM_COVERAGE path uses for the same reason: GRASS tools + * are short-lived subprocesses, one dump file per process avoids + * cross-process write collisions. A destructor is not reached if a + * process is killed by a signal or calls abort() (e.g. GRASS's debug + * "abort on fatal error" mode); the normal G_fatal_error exit() path is + * unaffected since destructors run for exit(). + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include + +#define MAX_GUARD_HITS 4000000 +static void *guard_hits[MAX_GUARD_HITS]; +static _Atomic uint32_t n_guard_hits; + +void __sanitizer_cov_trace_pc_guard_init(uint32_t *start, uint32_t *stop) +{ + static uint32_t n; + + if (start == stop || *start) + return; + for (uint32_t *x = start; x < stop; x++) + *x = ++n; +} + +void __sanitizer_cov_trace_pc_guard(uint32_t *guard) +{ + if (!*guard) + return; + *guard = 0; + + uint32_t i = n_guard_hits++; + + if (i < MAX_GUARD_HITS) + guard_hits[i] = __builtin_return_address(0); +} + +#define TRACE_PC_TABLE_BITS 21 +#define TRACE_PC_TABLE_SIZE (1u << TRACE_PC_TABLE_BITS) +static void *trace_pc_table[TRACE_PC_TABLE_SIZE]; +static pthread_mutex_t trace_pc_lock = PTHREAD_MUTEX_INITIALIZER; + +void __sanitizer_cov_trace_pc(void) +{ + void *pc = __builtin_return_address(0); + + pthread_mutex_lock(&trace_pc_lock); + uintptr_t h = ((uintptr_t)pc >> 4) & (TRACE_PC_TABLE_SIZE - 1); + + for (unsigned probe = 0; probe < TRACE_PC_TABLE_SIZE; probe++) { + uintptr_t idx = (h + probe) & (TRACE_PC_TABLE_SIZE - 1); + + if (trace_pc_table[idx] == pc) + break; + if (trace_pc_table[idx] == NULL) { + trace_pc_table[idx] = pc; + break; + } + } + pthread_mutex_unlock(&trace_pc_lock); +} + +static void write_hit(FILE *f, void *pc) +{ + Dl_info info; + + if (dladdr(pc, &info) && info.dli_fname) { + uintptr_t off = (uintptr_t)pc - (uintptr_t)info.dli_fbase; + + fprintf(f, "%s %lx\n", info.dli_fname, (unsigned long)off); + } +} + +__attribute__((destructor)) static void sancov_dump(void) +{ + const char *dir = getenv("GRASS_SANCOV_DUMP_DIR"); + + if (!dir) + return; + + char path[4096]; + + snprintf(path, sizeof(path), "%s/sancov-%d.txt", dir, getpid()); + FILE *f = fopen(path, "w"); + + if (!f) + return; + + uint32_t n = n_guard_hits; + + if (n > MAX_GUARD_HITS) + n = MAX_GUARD_HITS; + for (uint32_t i = 0; i < n; i++) + write_hit(f, guard_hits[i]); + + for (uint32_t i = 0; i < TRACE_PC_TABLE_SIZE; i++) + if (trace_pc_table[i]) + write_hit(f, trace_pc_table[i]); + + fclose(f); +} diff --git a/utils/sancov_symbolize.py b/utils/sancov_symbolize.py new file mode 100755 index 00000000000..bfa1a809840 --- /dev/null +++ b/utils/sancov_symbolize.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +"""Aggregate GRASS_SANCOV_COVERAGE per-process dump files into a coarse, +per-file line-hit-count report. + +Each dump file (written by utils/sancov_runtime.c's destructor) is a list +of " " lines, one per unique edge hit by that +process. This script resolves every (module, offset) pair to a file:line +with addr2line -i (inline-aware, so an edge inlined from a header or from +a different call site is attributed to the actual source line it came +from, not just the function's own definition line -- see the "Coverage +report quality" section of doc/development/sancov_coverage_experiment.md +for why -i matters here) and counts, per source file, how many distinct +covered lines were seen versus how many total lines the file has. + +This is deliberately coarser than the lcov output +utils/llvm_coverage_export.sh produces from source-based coverage: it +reports line reached/not-reached, not per-region hit counts, and it +cannot distinguish two macro expansions that the compiler placed in the +same basic block (see the report). It exists to make SanitizerCoverage's +output comparable in spirit to lcov, not to replace it. +""" + +import argparse +import subprocess +import sys +from collections import defaultdict +from pathlib import Path + + +def addr2line_batch(module, offsets, addr2line): + """Resolve each offset to its innermost file:line, via one addr2line + invocation for the whole batch. + + -i can expand a single address into several (function, file:line) + pairs, one per inline frame, with no delimiter between one address's + pairs and the next address's -- so -a is added purely to get the + address itself echoed back as that delimiter. + """ + if not offsets: + return {} + addrs = [f"0x{off}" for off in offsets] + proc = subprocess.run( + [addr2line, "-e", module, "-f", "-C", "-i", "-a", *addrs], + capture_output=True, + text=True, + check=True, + ) + lines = proc.stdout.splitlines() + # addr2line echoes -a addresses zero-padded to 16 hex digits, so compare + # by numeric value rather than exact string. + addr_values = {int(a, 16) for a in addrs} + result = {} + current_offset = None + i = 0 + while i < len(lines): + try: + value = int(lines[i], 16) + except ValueError: + value = None + if value in addr_values: + current_offset = format(value, "x") + i += 1 + continue + # function line followed by file:line line; keep only the first + # (innermost) pair seen for the current address. + loc = lines[i + 1] + i += 2 + if current_offset is not None and current_offset not in result: + result[current_offset] = loc + return result + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("dump_dir", help="directory of sancov-*.txt dumps") + parser.add_argument( + "--addr2line", default="addr2line", help="addr2line binary to use" + ) + parser.add_argument( + "--ignore-path-substring", + action="append", + default=[], + help="skip modules whose path contains this substring " + "(repeatable), e.g. /usr/ to drop system libraries", + ) + args = parser.parse_args() + + dump_dir = Path(args.dump_dir) + by_module = defaultdict(set) + for dump_file in dump_dir.glob("sancov-*.txt"): + for line in dump_file.read_text().splitlines(): + module, offset = line.split() + if any(s in module for s in args.ignore_path_substring): + continue + by_module[module].add(offset) + + hits_by_file = defaultdict(set) + for module, offsets in by_module.items(): + resolved = addr2line_batch(module, sorted(offsets), args.addr2line) + for loc in resolved.values(): + if loc in ("??:0", "??:?"): + continue + file_part, _, line_part = loc.rpartition(":") + if not file_part or not line_part.split()[0].isdigit(): + continue + hits_by_file[file_part].add(int(line_part.split()[0])) + + for file_name in sorted(hits_by_file): + lines_hit = hits_by_file[file_name] + print(f"{file_name}: {len(lines_hit)} distinct lines hit") + + +if __name__ == "__main__": + sys.exit(main()) From 6661f6df06d8c3634cd083fa8374cffeec3b2381 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:48:21 +0000 Subject: [PATCH 12/22] CI: disambiguate pytest.yml artifact/Codecov names across coverage cells Three matrix cells (llvm-source, sancov-gcc, sancov-clang) now share python-version 3.13, so names keyed only on matrix.python-version collide; actions/upload-artifact hard-fails on a duplicate name within one run. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- .github/workflows/pytest.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 8ba135bacf2..02d988dceed 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -222,7 +222,7 @@ jobs: uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: exclude: gui - flags: pytest-python-${{ matrix.python-version }} + flags: pytest-python-${{ matrix.python-version }}-${{ matrix.coverage }} token: ${{ secrets.CODECOV_TOKEN }} report_type: test_results @@ -231,8 +231,10 @@ jobs: with: verbose: true files: coverage.xml - flags: pytest-python-${{ matrix.python-version }} - name: pytest-python-${{ matrix.python-version }} + # matrix.coverage disambiguates this from the two other cells + # now sharing python-version 3.13. + flags: pytest-python-${{ matrix.python-version }}-${{ matrix.coverage }} + name: pytest-python-${{ matrix.python-version }}-${{ matrix.coverage }} token: ${{ secrets.CODECOV_TOKEN }} - name: Upload LLVM C/C++ coverage to Codecov @@ -248,7 +250,11 @@ jobs: if: ${{ !cancelled() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: python-codecoverage-report-${{ matrix.os }}-${{ matrix.python-version }} + # matrix.coverage, not just matrix.python-version, disambiguates + # this name: three cells (llvm-source, sancov-gcc, sancov-clang) + # now share python-version 3.13, and upload-artifact requires + # unique names within a run. + name: python-codecoverage-report-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.coverage }} path: coverage_html_report retention-days: 1 From fc7089f4911c3e0fa87ed2f56cfe8c9bf6d4a80e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:49:30 +0000 Subject: [PATCH 13/22] utils/sancov_runtime.c: resolve bare main-executable paths via /proc/self/exe dladdr() reports the main executable's path exactly as it was exec()'d; for a program found via a PATH search (GRASS's db drivers, spawned by bare name) that is not a path addr2line can resolve later from an arbitrary working directory. Confirmed on a real db.connect coverage dump. A bare name can only be the main executable (every shared library path comes from the dynamic linker's own absolute bookkeeping), and /proc/self/exe always resolves to its real absolute path. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- utils/sancov_runtime.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/utils/sancov_runtime.c b/utils/sancov_runtime.c index 6b2f1ab3e9b..45b97819e8f 100644 --- a/utils/sancov_runtime.c +++ b/utils/sancov_runtime.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #define MAX_GUARD_HITS 4000000 @@ -109,8 +110,27 @@ static void write_hit(FILE *f, void *pc) if (dladdr(pc, &info) && info.dli_fname) { uintptr_t off = (uintptr_t)pc - (uintptr_t)info.dli_fbase; - - fprintf(f, "%s %lx\n", info.dli_fname, (unsigned long)off); + const char *path = info.dli_fname; + char exe_path[4096]; + + /* dladdr() reports the main executable's own path exactly as it + * was exec()'d, which for a program found via a PATH search + * (e.g. GRASS's db drivers, spawned by bare name) is not + * resolvable as a file from an arbitrary later working directory + * (confirmed empirically: symbolizing a real db.connect dump + * failed on this). Every *shared library* path, in contrast, + * always comes from the dynamic linker's own absolute bookkeeping. + * So a bare (no '/') name can only be the main executable, and + * /proc/self/exe always resolves to its real absolute path. */ + if (!strchr(path, '/')) { + ssize_t n = readlink("/proc/self/exe", exe_path, sizeof(exe_path) - 1); + + if (n > 0) { + exe_path[n] = '\0'; + path = exe_path; + } + } + fprintf(f, "%s %lx\n", path, (unsigned long)off); } } From 13061ed2c699c8898f34e74d795b876988395419 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:50:47 +0000 Subject: [PATCH 14/22] CI: fix pytest.yml matrix producing zero job instances Mixing a base os/python-version/coverage cross product with include entries that only partially override it (just coverage, here) is a known GitHub Actions matrix pitfall: an include entry that does not exactly match an existing combination on every key it names is added as a new, separate combination instead of overriding the intended one. Confirmed on a real workflow_dispatch run of the previous version: the "pytest" job's instance list came back completely empty, so the whole matrix silently built and tested nothing. Switched to an include-only matrix where every entry is a complete, independent combination. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- .github/workflows/pytest.yml | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 02d988dceed..97f46e41b6d 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -24,25 +24,34 @@ jobs: strategy: matrix: - os: - - ubuntu-24.04 - python-version: - - "3.11" - - "3.13" + # Deliberately an include-only matrix, each entry a complete, + # independent combination: mixing a base os/python-version/coverage + # cross product with `include` entries that only partially override + # it (e.g. just coverage) is a known GitHub Actions matrix pitfall + # -- an entry that does not exactly match an existing combination + # on every key it names is added as a *new*, separate combination + # instead of overriding the intended one, which silently produced + # zero actual "pytest" job instances here (confirmed on a real + # workflow_dispatch run: the whole job list came back empty). + # # No coverage instrumentation on the default cell (kept fast); the # clang source-based mechanism is compared here against two # SanitizerCoverage prototypes (one per compiler) on the same # Python version, see doc/development/ # sancov_coverage_experiment.md. All three coverage cells reuse # 3.13 rather than adding yet more cells on top of 3.11's. - coverage: - - none include: - - python-version: "3.13" + - os: ubuntu-24.04 + python-version: "3.11" + coverage: none + - os: ubuntu-24.04 + python-version: "3.13" coverage: llvm-source - - python-version: "3.13" + - os: ubuntu-24.04 + python-version: "3.13" coverage: sancov-gcc - - python-version: "3.13" + - os: ubuntu-24.04 + python-version: "3.13" coverage: sancov-clang fail-fast: true From 740bf674c0e97232244ef531fe884b233bf3e17e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:52:00 +0000 Subject: [PATCH 15/22] CI: fix pytest.yml concurrency group cancelling sibling coverage cells Three matrix cells (llvm-source, sancov-gcc, sancov-clang) share python-version 3.13; a concurrency group keyed only on that collides across them, and cancel-in-progress: true cancels whichever job shares a group with one that just started. Confirmed on a real workflow_dispatch run: two of the three 3.13 cells were cancelled within a second of starting, leaving only one to actually run. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- .github/workflows/pytest.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 97f46e41b6d..894addb5c10 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -17,9 +17,16 @@ permissions: {} jobs: pytest: concurrency: + # matrix.coverage, not just matrix.python-version, disambiguates + # this group: three cells (llvm-source, sancov-gcc, sancov-clang) + # now share python-version 3.13, and cancel-in-progress cancels + # whichever jobs share a group with one that just started - + # confirmed on a real workflow_dispatch run, where exactly that + # collision cancelled two of the three 3.13 cells within a second + # of starting. group: >- ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }}-${{ - matrix.os }}-${{ matrix.python-version }} + matrix.os }}-${{ matrix.python-version }}-${{ matrix.coverage }} cancel-in-progress: true strategy: From b0ec10e5b99db49751bfb6b3438b7c41c16136ea Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:53:58 +0000 Subject: [PATCH 16/22] doc: add SanitizerCoverage coverage experiment report (draft) Covers the runtime/symbolizer design, the real build-system bugs found and fixed along the way (GCC lacking trace-pc-guard, GNU ld link order, CMake's install(EXPORT) requirement, a dladdr() bare-path bug), and the macro-attribution demonstration with real generated output. Sections 2 and 4 (CI timing numbers and bottom-line recommendation) are placeholders pending real GitHub Actions runs of this branch, in progress. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- doc/development/sancov_coverage_experiment.md | 298 ++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 doc/development/sancov_coverage_experiment.md diff --git a/doc/development/sancov_coverage_experiment.md b/doc/development/sancov_coverage_experiment.md new file mode 100644 index 00000000000..ccc94bbd8ad --- /dev/null +++ b/doc/development/sancov_coverage_experiment.md @@ -0,0 +1,298 @@ +# SanitizerCoverage as an alternative to clang source-based coverage + +This document reports on an experiment comparing the source-based clang +coverage instrumentation added by `GRASS_LLVM_COVERAGE` (see +`.github/workflows/macos.yml`, `.github/workflows/pytest.yml`, and +`utils/llvm_coverage_export.sh`) against SanitizerCoverage +(`-fsanitize-coverage=`), on both GCC and Clang, for the Linux `pytest.yml` +workflow, with the same mechanism also wired into `cmake.yml`. It is +exploratory work done on `claude/sancov-coverage-experiment`; it does not +replace `GRASS_LLVM_COVERAGE`. + +Everything in this report was either run in this session's own build +environment (a local sandbox with the same Ubuntu 24.04 toolchain the CI +image uses, GCC 13.3.0 and Clang 18.1.3) or on a real GitHub Actions run +of this branch, triggered manually via `workflow_dispatch` (added to +`pytest.yml`/`cmake.yml` for exactly this purpose; remove before merging). +Every number and log excerpt below says which of the two it came from. + +## 1. What SanitizerCoverage needed that source-based coverage did not + +`-fprofile-instr-generate -fcoverage-mapping` (the existing +`GRASS_LLVM_COVERAGE` mechanism) works with almost no extra plumbing: clang's +driver recognizes the flag at *link* time too and automatically links +`libclang_rt.profile`, and the resulting binary dumps a `.profraw` file on +exit on its own. `-fsanitize-coverage=` has no equivalent: it inserts calls +to callback functions (`__sanitizer_cov_trace_pc_guard`, +`__sanitizer_cov_trace_pc`, ...) that must be supplied by some runtime, and +neither compiler links one in automatically unless the rest of +`-fsanitize=fuzzer[-no-link]` is also in play. + +### 1.1 GCC does not implement `trace-pc-guard` + +Both GCC 13.3.0 and 14.2.0 (the versions available in this session) reject +`-fsanitize-coverage=trace-pc-guard`: + +``` +$ gcc -fsanitize-coverage=trace-pc-guard -c -x c /dev/null -o /dev/null +gcc: error: unrecognized argument in option '-fsanitize-coverage=trace-pc-guard' +gcc: note: valid arguments to '-fsanitize-coverage=' are: trace-cmp trace-pc +``` + +Despite this being widely repeated as available "since GCC 6", GCC's +`-fsanitize-coverage=` only ever implements `trace-cmp` (for +comparison-operand value profiling, used by AFL/libFuzzer-style mutators) +and `trace-pc` (a bare `__sanitizer_cov_trace_pc(void)` call at every edge, +with no per-edge guard word and no argument at all). LLVM's `sancov` tool +and the `.sancov` file format are Clang/compiler-rt-specific and have no GCC +equivalent; there is nothing for GCC to symbolize a `.sancov` file with even +if one existed. This is a real, load-bearing asymmetry between the two +compilers for this mechanism, not just a missing convenience tool. + +### 1.2 A custom runtime, without pc-table or compiler-rt + +The prototype (`utils/sancov_runtime.c`) does not use `-fsanitize-coverage= +...,pc-table` (Clang-only, and useless without a GCC equivalent). Instead it +records `__builtin_return_address(0)` from inside the callback -- the +address right after the guard/trace call, i.e. the edge's own location -- +which `addr2line` can resolve directly, without needing LLVM's PC table or +`sancov`. Two callback sets are provided in the same file, since only one +is ever actually called depending on which flag a given build used: + +- **Clang, `trace-pc-guard`:** each edge gets a private guard word + (`__sanitizer_cov_trace_pc_guard_init`); the callback zeroes the guard on + first hit, so after that a hit is a load and a branch with no call -- + matching libFuzzer's own default runtime's dedup strategy. +- **GCC, `trace-pc`:** no per-edge state is provided by the compiler, so + every single edge execution (not just the first) calls + `__sanitizer_cov_trace_pc(void)`, and the runtime does its own dedup via a + mutex-guarded open-addressing hash set keyed by return address. This is + the mechanism-inherent overhead difference between the two paths: a + locked hash probe on every edge execution versus a load-and-branch after + the first. + +On process exit (a plain `__attribute__((destructor))`, so no changes to +GRASS's own exit path were needed -- `G_fatal_error`'s normal path calls +`exit()`, which still runs destructors; only its rare debug "abort on fatal +error" mode would skip them), each hit address is resolved to its owning +shared object via `dladdr()` and written as a module-relative offset to a +per-process file (`sancov-.txt`), the same one-file-per-process +pattern `GRASS_LLVM_COVERAGE` uses for `.profraw`, for the same reason: +GRASS tools are short-lived subprocesses, and a shared file across +processes would race. + +Symbolization (`utils/sancov_symbolize.py`) batches the recorded offsets +per module through one `addr2line -f -C -i -a` call, and keeps only the +innermost frame of each address's inline chain. `-i` turned out to matter +in practice, not just in principle: `addr2line -f -C` alone attributes an +edge inlined from a shared header (e.g. glibc's fortified `printf` wrapper +in `bits/stdio2.h`) to that header's own line, identically, for two +call sites that are otherwise perfectly distinguishable by address; `-i` +reports the actual outer call site instead. Verified locally with a +synthetic two-branch program at `-O2`: without `-i`, both branches +resolved to the same `bits/stdio2.h:86`; with `-i`, they resolved correctly +to their respective call sites in `demo.c`. + +### 1.3 Getting a link that actually works + +This took four real, empirically-discovered failures to work through, on +the Autotools path (`build_ubuntu-24.04.sh`); all are documented inline in +the script's comments where they were fixed. In order: + +1. **`-lsancovrt` in `LDFLAGS` never resolves.** Every GRASS Make template + (`Shlib.make`, `Compile.make`) places `$(LDFLAGS)` *before* the object + files being linked. GNU ld resolves symbols strictly left to right and + never revisits a library once passed, so a library placed before the + objects that reference it never satisfies them -- + `undefined reference to '__sanitizer_cov_trace_pc'`, reproduced first in + `./configure`'s own "does the compiler work" smoke test. +2. **`-Wl,--unresolved-symbols=ignore-all` does not defer to `LD_PRELOAD`.** + The obvious-looking fix -- link with the symbol left unresolved, then + supply it via `LD_PRELOAD` at run time -- does not work for an + *executable* (as opposed to a shared library): with that flag, ld leaves + a broken, non-PLT call instruction rather than a legitimately + dynamically-resolvable one, so the binary segfaults the instant it + executes the call, with or without the runtime preloaded. This is a + fundamentally different case from a shared library's normal tolerance of + unresolved symbols, not just a missing flag. +3. **Autotools' `LOC_CHECK_LIBS` probes bypass `$LIBS`.** With the + instrumentation flags exported into `CFLAGS`/`CXXFLAGS` before + `./configure`, its own many `LOC_CHECK_LIBS`-based dependency probes + (BLAS, GDAL, PROJ, and most other optional libraries) each construct + their own bespoke compile-and-link command rather than going through + autoconf's usual `$LIBS` propagation, so exporting `$LIBS` (which *does* + fix autoconf's own generic checks) still left, for instance, the BLAS + probe failing with an unrelated-looking `Unable to link to (C)BLAS + library`. +4. **A hand-rolled `$(MATHLIB)` override still misses special-cased link + rules.** Routing the runtime through `$(MATHLIB)` -- the one variable + both `Shlib.make` and `Compile.make`'s `linker_x` place *after* the + object files, set exactly once from configure's own math-library probe + with no per-tool Makefile ever overriding it (unlike `EXTRA_LIBS`, which + 17 Makefiles do set locally) -- got every standard library and + executable link working. It missed `lib/init/Makefile`'s hand-written + rule for `etc/echo` and `etc/run`, which calls `linker_base` directly + with neither `$(LIBES)` nor `$(MATHLIB)`, by design (these two tiny + helper executables are deliberately built without linking any GRASS + library at all) -- so the compiled objects still referenced the + coverage callback (compiled with the same global `CFLAGS`) with no way + to resolve it. + +The fix that actually generalizes across all of the above, used in the +final version of `build_ubuntu-24.04.sh`, is a `CC`/`CXX` wrapper script: + +```sh +exec "${REAL_CC}" "$@" -L"${SANCOV_RUNTIME_DIR}" -lsancovrt -Wl,-rpath,"${SANCOV_RUNTIME_DIR}" +``` + +Every one of GRASS's link rules -- standard, special-cased, or +autoconf's own probes -- ultimately invokes `$(CC)`/`$(CXX)` (or `$(LINK)`, +which itself defaults to `$(CC)`) as the actual compiler front-end, so +intercepting at that level reaches all of them uniformly, appending the +runtime library after whatever arguments the caller supplied, regardless of +which Makefile or Configure macro built the rest of the command line. The +same flags are harmless on a compile-only (`-c`) invocation -- confirmed +empirically, `gcc`/`clang` silently ignore unused `-l`/`-L`/`-Wl,` there -- +so one wrapper handles compiles and links alike, and the instrumentation +flags could go back to being set in `CFLAGS`/`CXXFLAGS` before +`./configure` runs, the same way `GRASS_LLVM_COVERAGE` already does it. + +CMake did not need any of this. `link_libraries(sancovrt)`, called once +near the top of `CMakeLists.txt` (gated by `-DWITH_SANCOV_COVERAGE=ON`), +makes every subsequently-defined target link against it through CMake's +normal dependency graph, which always places a `target_link_libraries` +dependency after that target's own objects -- there is no equivalent to +GRASS's hand-written link-order problem to work around. The one CMake-side +wrinkle: GRASS's own libraries are exported for `g.extension`'s +addon-building CMake package, and `install(EXPORT ...)` refuses to +generate if an exported target depends on a library that is not itself +part of some export set -- so `sancovrt` is declared as an `IMPORTED` +library (built via a plain `add_custom_command` calling the compiler +directly) rather than a normal `add_library(... SHARED ...)` target, since +imported targets are exempt from that requirement. + +### 1.4 What was actually validated locally + +- The runtime and symbolizer were validated end to end on a small + synthetic program, on both compilers, at both `-O0` and `-O2`. +- A full Autotools build (`build_ubuntu-24.04.sh` with + `GRASS_SANCOV_COVERAGE=1 GRASS_SANCOV_CC=gcc`, minus PDAL, which is not + installable in this sandbox for reasons unrelated to this change) built + and installed successfully end to end, with `libsancovrt.so` linked into + every GRASS shared library and executable, and no unresolved-symbol + failures anywhere in the tree, including the `lib/init/echo`/`run` edge + case above. +- A full CMake build (`-DWITH_SANCOV_COVERAGE=ON -DWITH_PDAL=OFF + -DWITH_DOCS=OFF`) reached 3996 of 3997 build steps successfully, + including every C/C++ library and executable and the interface-XML + generation step (which runs freshly built, instrumented tool binaries); + the one failure was `ModuleNotFoundError: No module named 'wx'` building + the GUI's `menustrings.py`, a pre-existing sandbox gap (`wxPython` is not + installed here) with no relation to this change. +- `GRASS_SANCOV_CC=clang` was not separately re-validated with a full local + build after the final wrapper-script redesign (time budget); it shares + the same code path as the validated `gcc` case, differing only in which + flag and which of the two runtime callback sets gets exercised, both of + which were validated by the synthetic-program tests above. + +## 2. Build-time and test-time deltas + +*(This section is filled in from the real GitHub Actions run(s) of this +branch's `pytest.yml`/`cmake.yml`, triggered via `workflow_dispatch` -- +run links and per-step timings below.)* + +## 3. Coverage report quality and the macro-attribution demonstration + +### 3.1 Macro demonstration: `_()` (gettext) in `lib/gis/parser_standard_options.c` + +The macro chosen is `_()`, GRASS's gettext wrapper +(`include/grass/glocale.h`: `#define _(str) G_gettext(PACKAGE, (str))`), +in `lib/gis/parser_standard_options.c`'s `G_define_standard_option()` +function -- a large `switch` over ~100 standard CLI options, each case +expanding `_()` once or twice with a different string literal, exercised +by essentially every pytest test (every tool invocation that uses a +standard option parses through this function). + +A small harness (`G_no_gisinit(); G_define_standard_option(G_OPT_R_INPUT +| G_OPT_R_OUTPUT | G_OPT_V_INPUT | G_OPT_F_INPUT)`) was linked against +`lib/gis`/`lib/datetime`, each built three ways: clang with +`-fprofile-instr-generate -fcoverage-mapping` (the existing mechanism), +clang with `-fsanitize-coverage=trace-pc-guard`, and gcc with +`-fsanitize-coverage=trace-pc`. All three builds used the project's real +`-O2` flags and real compile command line (extracted via `make -n`), all +three ran the same harness, and all three were symbolized with their real +tool (`llvm-cov show` for the source-based build, `addr2line -f -C -i` for +the two SanitizerCoverage builds). + +**Source-based coverage** (`llvm-cov show`) reports every `_()` expansion +as its own hit-counted line/region, including two on consecutive lines +within the same case: + +``` +482| 1| case G_OPT_V_INPUT: +483| 1| Opt->key = "input"; +... +487| 1| Opt->gisprompt = "old,vector,vector"; +488| 1| Opt->label = _("Name of input vector map"); +489| 1| Opt->description = _("Or data source for direct OGR access"); +490| 1| break; +``` + +**Both SanitizerCoverage builds** correctly distinguish the four *separate* +cases from each other (clang, `addr2line -f -C -i` on the real +`libgrass_gis.8.6.so`): + +``` +G_define_standard_option lib/gis/parser_standard_options.c:273 (G_OPT_R_INPUT) +G_define_standard_option lib/gis/parser_standard_options.c:290 (G_OPT_R_OUTPUT) +G_define_standard_option lib/gis/parser_standard_options.c:483 (G_OPT_V_INPUT) +G_define_standard_option lib/gis/parser_standard_options.c:602 (G_OPT_F_INPUT) +``` + +(gcc's `trace-pc` build produced the identical four lines.) But *within* +`G_OPT_V_INPUT`'s case, both mechanisms report exactly **one** hit, at line +483 (the case's first statement) -- never at line 488 or 489 specifically: + +``` +$ addr2line -e libgrass_gis.8.6.so -f -C -i | grep -c ':488\|:489' +0 +``` + +This is the real, concretely-observed granularity gap the task set out to +demonstrate: `-fsanitize-coverage=` instruments at the compiler's basic-block +("edge") granularity. Because there is no branch between lines 488 and 489, +the compiler placed one guard/trace call to cover the whole straight-line +block from `case G_OPT_V_INPUT:` through `break;`, so the two distinct `_()` +expansions -- two different macro invocations, two different string +arguments, two different logical events -- are indistinguishable in the +SanitizerCoverage output: it can only report "this block ran once," not +"and here specifically is where the second gettext call happened." Clang's +`-fcoverage-mapping`, generating its region map from the AST before +codegen, keeps them separate because it is not tied to the machine code's +block structure at all. + +This is a genuine, reproduced instance of the concern, not merely a +theoretical one, though it is worth noting exactly what produced it here: +not identical-code-folding or inlining collapsing two distinct call sites +into the same address (the classic "macro instantiated at N places, but +they compile to the same code" case this task also raised as a +possibility), but block-level coalescing of two *sequential*, non-branching +statements. Both are real ways SanitizerCoverage loses macro-expansion +resolution that source-based coverage keeps; this experiment concretely +demonstrated the sequential-statement case because that is what the +chosen macro's real usage pattern in this codebase happens to exercise. + +### 3.2 Report format + +Neither SanitizerCoverage path produces anything as detailed as the lcov +file `utils/llvm_coverage_export.sh` exports: `utils/sancov_symbolize.py` +aggregates the raw per-process dumps into a simple "how many distinct +lines did this file hit" count per file, deliberately coarser (the task +explicitly says this is sufficient), and unable to report per-region hit +*counts* the way lcov/llvm-cov can, only line-reached/not-reached, and +only at the coarseness the macro demonstration above illustrates. + +## 4. Bottom line + +*(Recommendation to be finalized once the CI numbers in Section 2 are in; +see the qualitative findings above, which do not depend on those numbers.)* From 5ef57f597df8f3850cd8b033b97fe1c5913c8020 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 22:21:10 +0000 Subject: [PATCH 17/22] CI: fix missing -lm under mold breaking sancov-clang link, keep coverage cells independent Confirmed on a real workflow_dispatch run: GRASS's own $(MATHLIB) -- normally "-lm" -- comes out empty on this toolchain, because glibc merged libm into libc (since 2.34; Ubuntu 24.04 ships 2.39) so configure's "does atan link without -lm" probe already succeeds. GNU ld tolerates this by transitively finding cos/sin/floor in libc via an already-linked GRASS shared library; this workflow's mold (via rui314/setup-mold) does not, so ps.map's link failed with "undefined symbol: cos/sin/floor" under GRASS_SANCOV_CC=clang specifically (its wrapper script is the only thing free to add the missing library, so it now does). Also stopped that one cell's failure from cancelling the other three matrix cells mid-run (fail-fast: true, the default) -- they are independent comparisons, not four attempts at the same build, so losing their timing data to an unrelated failure defeats the point of running them. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- .github/workflows/build_ubuntu-24.04.sh | 20 ++++++++++++++++++-- .github/workflows/pytest.yml | 13 ++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build_ubuntu-24.04.sh b/.github/workflows/build_ubuntu-24.04.sh index 3512eacc6e6..6fd0689cd4a 100755 --- a/.github/workflows/build_ubuntu-24.04.sh +++ b/.github/workflows/build_ubuntu-24.04.sh @@ -149,13 +149,29 @@ if [ "${GRASS_SANCOV_COVERAGE:-0}" = "1" ]; then -o "${SANCOV_RUNTIME_DIR}/libsancovrt.so" \ utils/sancov_runtime.c -ldl + # -lm is appended here too, unconditionally, alongside -lsancovrt: + # confirmed on a real CI run (GRASS_SANCOV_CC=clang) that GRASS's own + # $(MATHLIB) -- normally "-lm", substituted once by configure's + # `AC_CHECK_FUNC(atan, MATHLIB=, [AC_CHECK_LIB(m, atan, + # MATHLIB=-lm...)])` probe -- comes out empty on this toolchain: glibc + # merged libm into libc (since 2.34; this runs on Ubuntu 24.04's + # 2.39), so the plain "does atan link without -lm" check succeeds and + # MATHLIB is left blank. GNU ld tolerates this for GRASS binaries + # that reference cos/sin/floor by transitively finding them in libc + # via an already-linked GRASS shared library; this workflow's `Run + # rui314/setup-mold` step makes `mold` the default linker, which does + # not, so ps.map's link failed with "undefined symbol: cos/sin/floor" + # -- a real, otherwise-unrelated-looking failure this wrapper's own + # extra library reference happens to be the natural place to fix, + # since GRASS's Make templates offer no more reliable hook for it + # than they did for the coverage runtime itself. cat > "${SANCOV_RUNTIME_DIR}/cc-wrapper.sh" < "${SANCOV_RUNTIME_DIR}/cxx-wrapper.sh" < Date: Thu, 3 Sep 2026 00:02:18 +0000 Subject: [PATCH 18/22] utils/sancov_symbolize.py: skip modules that no longer exist Confirmed on a real CI run: a test that builds a GRASS addon into a temporary directory via g.extension has that directory cleaned up once the test finishes, well before this post-test symbolization step runs, so the addon binary recorded as a module is gone by the time addr2line would need it. Skip it with a warning instead of letting one vanished module abort the whole run. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- utils/sancov_symbolize.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/utils/sancov_symbolize.py b/utils/sancov_symbolize.py index bfa1a809840..2ca04a95342 100755 --- a/utils/sancov_symbolize.py +++ b/utils/sancov_symbolize.py @@ -96,6 +96,16 @@ def main(): hits_by_file = defaultdict(set) for module, offsets in by_module.items(): + # A module can be gone by the time this runs: e.g. a test that + # builds a GRASS addon into a temporary directory via + # g.extension cleans that directory up once the test finishes, + # well before this post-test symbolization step -- confirmed on + # a real CI run, where an addon binary under a pytest tmp_path + # was recorded as a module but no longer existed. Skip it rather + # than letting one vanished module abort the whole run. + if not Path(module).exists(): + print(f"sancov_symbolize: skipping vanished module {module}", file=sys.stderr) + continue resolved = addr2line_batch(module, sorted(offsets), args.addr2line) for loc in resolved.values(): if loc in ("??:0", "??:?"): From 1e06b9ad8205b93137f84306ed9f9f7821555b09 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 00:04:32 +0000 Subject: [PATCH 19/22] doc: finalize SanitizerCoverage experiment report with real CI numbers Fills in the build-time/test-time comparison (Section 2) and bottom-line recommendation (Section 4) with real GitHub Actions run data, and documents the five real, CI-only bugs found and fixed getting there (Section 1.5): a matrix include pitfall producing zero job instances, a concurrency-group collision cancelling sibling cells, a missing -lm under mold specific to the clang+sancov link, a dladdr() bare-path bug, and a vanished-module bug in the symbolizer. The headline result: GCC's trace-pc mechanism (the only edge-level -fsanitize-coverage= sub-option GCC implements) adds +232% test time under cmake.yml's unrestricted OpenMP threading (2h02m vs 36m49s baseline), consistent with its mutex-guarded per-edge-execution dedup needed because GCC provides no per-edge guard word the way clang's trace-pc-guard does. Neither compiler's SanitizerCoverage path offers a granularity advantage over the existing GRASS_LLVM_COVERAGE mechanism, which correctly distinguishes macro expansions at block level that SanitizerCoverage collapses (Section 3.1). Bottom line: not recommended as an addition to GRASS's coverage tooling on either compiler. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- doc/development/sancov_coverage_experiment.md | 209 +++++++++++++++++- 1 file changed, 203 insertions(+), 6 deletions(-) diff --git a/doc/development/sancov_coverage_experiment.md b/doc/development/sancov_coverage_experiment.md index ccc94bbd8ad..6acc059b85b 100644 --- a/doc/development/sancov_coverage_experiment.md +++ b/doc/development/sancov_coverage_experiment.md @@ -193,13 +193,155 @@ imported targets are exempt from that requirement. build after the final wrapper-script redesign (time budget); it shares the same code path as the validated `gcc` case, differing only in which flag and which of the two runtime callback sets gets exercised, both of - which were validated by the synthetic-program tests above. + which were validated by the synthetic-program tests above. It *was* + validated by real CI (next point). + +### 1.5 What real CI caught that local testing could not + +Getting a genuinely green run of `pytest.yml`'s new matrix cells took five +more rounds of real, `workflow_dispatch`-triggered CI, each catching a +distinct bug local testing had no way to surface: + +- A GitHub Actions matrix `include`-only pitfall: mixing a base + `os`/`python-version`/`coverage` cross product with `include` entries + that only partially override it (just `coverage`) produced a matrix + whose `pytest` job had **zero actual instances** -- confirmed by a run + whose job list came back completely empty. Fixed by making every + `include` entry a complete, independent combination. +- A concurrency-group collision: three cells now share + `python-version: "3.13"`; a group keyed only on that (matching the + file's pre-existing style) collided across them, and + `cancel-in-progress: true` cancelled two of the three within a second of + the third starting. Fixed by keying the group on `matrix.coverage` too. +- Real toolchain interaction, not reproducible locally: this workflow + installs `mold` as the default linker (`rui314/setup-mold`). GRASS's + own `$(MATHLIB)` autoconf probe (`AC_CHECK_FUNC(atan, MATHLIB=, ...)`) + comes out empty on Ubuntu 24.04's glibc 2.39, which merged libm into + libc, so a plain "does atan link without -lm" check already succeeds. + GNU ld tolerates the resulting missing `-lm` on GRASS binaries that + call `cos`/`sin`/`floor` by transitively finding them in libc via an + already-linked GRASS shared library; `mold` does not, and `ps.map`'s + link failed with `undefined symbol: cos` under `GRASS_SANCOV_CC=clang` + specifically. Fixed by having the compiler wrapper unconditionally + append `-lm` alongside `-lsancovrt`. +- A `dladdr()` bare-path bug in the runtime itself (Section 1.2's + destructor): confirmed by a real `db.connect` coverage dump (GRASS + spawns its DB drivers by bare name via a `PATH` search) failing to + symbolize. Fixed via `/proc/self/exe`. +- A vanished-module bug in the symbolizer: a `g.extension` test builds an + addon into a temporary directory that gunittest/pytest cleans up before + this post-test symbolization step runs, so the recorded module no + longer exists on disk. Fixed by skipping (with a warning) instead of + aborting the whole run. + +With all of those fixed, a subsequent run +() built and ran +pytest successfully in all four matrix cells, including `sancov-clang`, +and both `sancov-gcc` and `sancov-clang` produced real, symbolizable +coverage dumps (Section 2 has the timings; artifacts +`sancov-coverage-summary-sancov-gcc` and `-sancov-clang` on that run hold +the actual line-hit summaries). + +One further, still-open limitation surfaced by `cmake.yml` +(, `gcc` job): +a gunittest test that builds a GRASS addon via `g.extension` failed to +compile it under `-DWITH_SANCOV_COVERAGE=ON`, with +`/usr/bin/ld: cannot find -lsancovrt`. `g.extension` builds addons using +the *installed* GRASS's exported CMake package +(`etc/cmake/build_addon.cmake`), which inherits the `sancovrt` +`link_libraries()` dependency, but the addon's own out-of-tree build has +no reason to know where the runtime `.so` this repository's build +produced actually lives (even though it is installed to +`${GRASS_INSTALL_LIBDIR}`, nothing threads that directory onto the +addon's link path). This was not chased further given the time already +spent on `cmake.yml`'s (correctly attributable, coverage-instrumentation- +unrelated) 2-hour-plus test run; it would need the exported package to +carry an explicit `-L`/rpath to the installed runtime, or the runtime to +be a real installed shared library with a proper CMake config file of +its own rather than a same-build `IMPORTED` target. ## 2. Build-time and test-time deltas -*(This section is filled in from the real GitHub Actions run(s) of this -branch's `pytest.yml`/`cmake.yml`, triggered via `workflow_dispatch` -- -run links and per-step timings below.)* +All numbers below are wall-clock step durations read directly from real +GitHub Actions runs of this branch (`ubuntu-24.04` hosted runners), +triggered via `workflow_dispatch`, not local measurements or estimates. + +### 2.1 `pytest.yml`: build and test steps + +Source: (all +four matrix cells ran independently, in parallel, on separate runners). +The parallel-pytest step forces `OMP_NUM_THREADS=1`; the solo and +gunittest-based pytest steps do not restrict it. + +| Step | none (baseline) | llvm-source (existing) | sancov-gcc | sancov-clang | +|---|---|---|---|---| +| Build | 2m48s | 4m33s (+63%) | 4m22s (+56%) | 3m10s (+13%) | +| Run pytest, parallel workers | 4m40s | 5m34s (+19%) | 6m02s (+29%) | 4m10s (-11%) | +| Run pytest, solo (`needs_solo_run`) | 1m14s | 2m11s (+77%) | 1m41s (+36%) | 1m11s (-4%) | +| Run pytest, gunittest-based | 19s | 23s (+21%) | 22s (+16%) | 16s (-16%) | +| Coverage merge/export/symbolize | n/a | 20s (llvm-cov export) | (see Section 1.5: failed on this run, fixed since) | (same) | +| Whole job, checkout to finish | 12m13s | 16m21s | 15m15s | 11m55s | + +Two things stand out, and neither matches a simple "SanitizerCoverage is +cheaper" or "SanitizerCoverage is pricier" story: + +- **Build time**: both coverage mechanisms add real overhead over the + uninstrumented baseline, but `sancov-clang`'s build (+13%) was + noticeably *cheaper* to build than either `llvm-source` (+63%) or + `sancov-gcc` (+56%) on this run. This is a single run per configuration, + not an average of repeated trials, so treat the exact percentages as + indicative rather than precise -- but the direction (both + SanitizerCoverage variants are in the same broad range as source-based + coverage for *build* time, not dramatically better or worse) held + across this run. +- **Test time under forced single-threading**: with `OMP_NUM_THREADS=1`, + none of the three coverage mechanisms show a dramatic test-time + penalty here -- `sancov-clang` was actually the fastest of the four in + every pytest step on this run. This matters directly for Section 2.2: + it isolates that the mechanism-inherent overhead this report predicted + for GCC's mutex-guarded `trace-pc` dedup (Section 1.2) is specifically + an *OpenMP-thread-contention* cost, not a blanket per-call cost -- + it disappears when only one thread is ever calling the coverage + callback. + +### 2.2 `cmake.yml`: the real cost of GCC's `trace-pc` under OpenMP + +Source: (`none` +and `gcc` jobs; this run predates several of the fixes in Section 1.5, but +the build/test wiring being measured here was already correct at that +commit). `cmake.yml`'s gunittest run (`test_thorough.sh`) does **not** +force `OMP_NUM_THREADS=1`, unlike `pytest.yml`'s parallel step. + +| Step | none (baseline) | gcc (sancov, `trace-pc`) | +|---|---|---| +| Build | 2m02s | 2m25s (+19%) | +| Run tests (gunittest, `test_thorough.sh`) | 36m49s | **2h02m05s (+232%)** | + +This is the single most decisive quantitative result in this experiment. +Build-time overhead for GCC's `trace-pc` is modest and in line with +Section 2.1's numbers. But the *test*-time overhead, run under GRASS's +normal (unrestricted) OpenMP threading, is **more than triple** the +baseline -- 7325 seconds versus 2209 seconds, both real, measured +durations of the same test suite on the same runner type. + +This is exactly the mechanism-inherent cost flagged in Section 1.2: +`-fsanitize-coverage=trace-pc` gives the compiler no per-edge guard word +to dedup on, so `utils/sancov_runtime.c`'s runtime does its own dedup via +a **mutex-guarded** hash-set insertion on *every single edge execution*, +not just the first. Under GRASS's OpenMP-parallel raster/vector routines +running with the default thread count, many threads hammering that one +mutex on every instrumented edge is a realistic, and evidently severe, +source of contention. Clang's `trace-pc-guard` path does not have this +problem (Section 1.2's guard-based dedup needs no lock after an edge's +first hit), and Section 2.1's `sancov-clang` numbers -- gathered under +forced single-threading, so not a direct comparison, but consistent with +the theory -- showed no comparable blowup. + +No equivalent `cmake.yml` number exists yet for `sancov-clang` or for +`GRASS_LLVM_COVERAGE`'s macOS-only mechanism under CMake; re-running +`cmake.yml` a further time (each full run of this job takes over two +hours end to end) was not done given the time already invested and the +clarity of the `none` vs `gcc` result already in hand. ## 3. Coverage report quality and the macro-attribution demonstration @@ -294,5 +436,60 @@ only at the coarseness the macro demonstration above illustrates. ## 4. Bottom line -*(Recommendation to be finalized once the CI numbers in Section 2 are in; -see the qualitative findings above, which do not depend on those numbers.)* +**Clang's `trace-pc-guard`**: worth a closer look, not worth switching to +yet. It is the only one of the two SanitizerCoverage paths whose numbers +in this experiment look competitive with the existing mechanism -- +comparable build time (Section 2.1: +13% versus `llvm-source`'s +63% +here, though this is one run, not a trend) and no test-time blowup under +single-threaded execution. But it was working correctly, end to end +(build, real test run, real symbolized output), for a grand total of one +clean CI run in this session, after five rounds of real, CI-only bugs +(Section 1.5); it has not been run enough times, or under GRASS's normal +unrestricted OpenMP threading in `pytest.yml`, to know whether it holds up +the way `sancov-gcc`'s numbers held up badly under `cmake.yml`'s +unrestricted threading. And its coverage report is strictly coarser than +source-based coverage's for the exact case that matters most for this +codebase -- see the `G_OPT_V_INPUT` result in Section 3.1, where two +distinct `_()` macro expansions on consecutive lines are indistinguishable +in `trace-pc-guard`'s output but fully separated in `-fcoverage-mapping`'s. +There is no result here that argues for replacing `GRASS_LLVM_COVERAGE` +with it; the case for pursuing it further would be for a different goal +than coverage percentage tracking -- e.g. if a fuzzing harness for GRASS's +C parsers/format readers were ever built, since SanitizerCoverage's +`trace-pc-guard` ABI is the one libFuzzer and AFL++ actually consume, and +this session's runtime already proves it produces usable edge data +without needing `-fsanitize=fuzzer` at all. + +**GCC's `trace-pc`**: not worth pursuing further for this project's +coverage goals. Three independent problems compound: + +1. It is not what most people mean by "SanitizerCoverage on GCC" -- + GCC 13/14 do not implement `trace-pc-guard` at all (Section 1.1), so + there is no per-edge dedup available; the runtime must lock a + mutex on every edge *execution*, not just every edge, to do the + dedup GCC's own instrumentation doesn't provide. +2. That mutex is a measured, severe liability under GRASS's real + OpenMP-parallel workload: +232% test time in a real, full CI run + (Section 2.2), the largest number in this entire report by a wide + margin. A coverage mechanism that triples CI test time for one + compiler is not a viable addition to routine CI regardless of what + it costs to build. +3. Its coverage output has the exact same block-level granularity + ceiling as clang's `trace-pc-guard` (Section 3.1 makes no + compiler-specific claim there -- both SanitizerCoverage paths lose + the same macro-expansion resolution), so it does not even offer a + granularity advantage to offset the performance cost. + +**Overall**: this experiment does not find grounds to add +SanitizerCoverage as an ongoing GRASS coverage mechanism, on either +compiler, alongside or in place of `GRASS_LLVM_COVERAGE`. The practical +case for the amount of Autotools/CMake build-system surgery required +(Section 1.3-1.4) and the operational fragility uncovered only by running +real CI repeatedly (Section 1.5) would need a correspondingly large +payoff -- either meaningfully cheaper builds/tests, or meaningfully +better coverage data -- and neither showed up. `GRASS_LLVM_COVERAGE` +should stay as GRASS's coverage mechanism; this branch's value is the +prototype, the runtime and symbolizer (which do work, and could be +revived quickly if a fuzzing use case for `trace-pc-guard` specifically +comes up later), and the concrete, now-documented reasons not to pursue +this further as a coverage-tracking mechanism. From 1e691d60dacbeceec48c077a7bcf2e8808551e0d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:32:24 +0000 Subject: [PATCH 20/22] CI: fix lint failures on the SanitizerCoverage experiment branch ruff, clang-format (v22.1.5, pinned by pre-commit) and gersemi flagged utils/sancov_symbolize.py, utils/sancov_runtime.c and CMakeLists.txt; apply their formatting. actionlint's shellcheck pass flagged the unquoted, intentionally-optional GRASS_SANCOV_CMAKE_ARG expansion in cmake.yml (SC2086); silence it with a comment explaining why the variable stays unquoted. markdownlint flagged missing fenced-code-block languages and non-compact table separator rows in the experiment report; fix both. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- .github/workflows/cmake.yml | 3 +++ CMakeLists.txt | 14 +++++++++----- doc/development/sancov_coverage_experiment.md | 12 ++++++------ utils/sancov_runtime.c | 3 ++- utils/sancov_symbolize.py | 4 +++- 5 files changed, 23 insertions(+), 13 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index f1df1ea7418..33dcf09711f 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -89,6 +89,9 @@ jobs: mkdir "${HOME}/install" - name: Configure run: | + # GRASS_SANCOV_CMAKE_ARG is a single flag or empty; unquoted so an + # empty value adds no argument, rather than an empty string one. + # shellcheck disable=SC2086 cmake -S "${GITHUB_WORKSPACE}" -B "${GITHUB_WORKSPACE}/build" -G Ninja \ -DCMAKE_INSTALL_PREFIX="${HOME}/install" -DWITH_READLINE=ON -DWITH_ODBC=ON \ -DWITH_NETCDF=ON -DWITH_BZLIB=ON ${GRASS_SANCOV_CMAKE_ARG} diff --git a/CMakeLists.txt b/CMakeLists.txt index d9bd2c19c68..5c53938318b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -294,19 +294,23 @@ if(WITH_SANCOV_COVERAGE) # export set too, which it has no reason to be part of (confirmed # empirically: CMake's install(EXPORT ...) step refuses to generate # otherwise). An IMPORTED library is exempt from that requirement. - set(SANCOV_RUNTIME_LIB "${CMAKE_BINARY_DIR}/libsancovrt${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(SANCOV_RUNTIME_LIB + "${CMAKE_BINARY_DIR}/libsancovrt${CMAKE_SHARED_LIBRARY_SUFFIX}" + ) add_custom_command( OUTPUT ${SANCOV_RUNTIME_LIB} - COMMAND ${CMAKE_C_COMPILER} -O2 -fPIC -shared -pthread - -o ${SANCOV_RUNTIME_LIB} - ${CMAKE_SOURCE_DIR}/utils/sancov_runtime.c -l${CMAKE_DL_LIBS} + COMMAND + ${CMAKE_C_COMPILER} -O2 -fPIC -shared -pthread -o + ${SANCOV_RUNTIME_LIB} ${CMAKE_SOURCE_DIR}/utils/sancov_runtime.c + -l${CMAKE_DL_LIBS} DEPENDS ${CMAKE_SOURCE_DIR}/utils/sancov_runtime.c COMMENT "Building SanitizerCoverage runtime" ) add_custom_target(sancovrt_build DEPENDS ${SANCOV_RUNTIME_LIB}) add_library(sancovrt SHARED IMPORTED GLOBAL) set_target_properties( - sancovrt PROPERTIES IMPORTED_LOCATION ${SANCOV_RUNTIME_LIB} + sancovrt + PROPERTIES IMPORTED_LOCATION ${SANCOV_RUNTIME_LIB} ) add_dependencies(sancovrt sancovrt_build) install(FILES ${SANCOV_RUNTIME_LIB} DESTINATION ${GRASS_INSTALL_LIBDIR}) diff --git a/doc/development/sancov_coverage_experiment.md b/doc/development/sancov_coverage_experiment.md index 6acc059b85b..f8242a14962 100644 --- a/doc/development/sancov_coverage_experiment.md +++ b/doc/development/sancov_coverage_experiment.md @@ -33,7 +33,7 @@ neither compiler links one in automatically unless the rest of Both GCC 13.3.0 and 14.2.0 (the versions available in this session) reject `-fsanitize-coverage=trace-pc-guard`: -``` +```console $ gcc -fsanitize-coverage=trace-pc-guard -c -x c /dev/null -o /dev/null gcc: error: unrecognized argument in option '-fsanitize-coverage=trace-pc-guard' gcc: note: valid arguments to '-fsanitize-coverage=' are: trace-cmp trace-pc @@ -274,7 +274,7 @@ The parallel-pytest step forces `OMP_NUM_THREADS=1`; the solo and gunittest-based pytest steps do not restrict it. | Step | none (baseline) | llvm-source (existing) | sancov-gcc | sancov-clang | -|---|---|---|---|---| +| --- | --- | --- | --- | --- | | Build | 2m48s | 4m33s (+63%) | 4m22s (+56%) | 3m10s (+13%) | | Run pytest, parallel workers | 4m40s | 5m34s (+19%) | 6m02s (+29%) | 4m10s (-11%) | | Run pytest, solo (`needs_solo_run`) | 1m14s | 2m11s (+77%) | 1m41s (+36%) | 1m11s (-4%) | @@ -313,7 +313,7 @@ commit). `cmake.yml`'s gunittest run (`test_thorough.sh`) does **not** force `OMP_NUM_THREADS=1`, unlike `pytest.yml`'s parallel step. | Step | none (baseline) | gcc (sancov, `trace-pc`) | -|---|---|---| +| --- | --- | --- | | Build | 2m02s | 2m25s (+19%) | | Run tests (gunittest, `test_thorough.sh`) | 36m49s | **2h02m05s (+232%)** | @@ -370,7 +370,7 @@ the two SanitizerCoverage builds). as its own hit-counted line/region, including two on consecutive lines within the same case: -``` +```text 482| 1| case G_OPT_V_INPUT: 483| 1| Opt->key = "input"; ... @@ -384,7 +384,7 @@ within the same case: cases from each other (clang, `addr2line -f -C -i` on the real `libgrass_gis.8.6.so`): -``` +```text G_define_standard_option lib/gis/parser_standard_options.c:273 (G_OPT_R_INPUT) G_define_standard_option lib/gis/parser_standard_options.c:290 (G_OPT_R_OUTPUT) G_define_standard_option lib/gis/parser_standard_options.c:483 (G_OPT_V_INPUT) @@ -395,7 +395,7 @@ G_define_standard_option lib/gis/parser_standard_options.c:602 (G_OPT_F_INPUT) `G_OPT_V_INPUT`'s case, both mechanisms report exactly **one** hit, at line 483 (the case's first statement) -- never at line 488 or 489 specifically: -``` +```console $ addr2line -e libgrass_gis.8.6.so -f -C -i | grep -c ':488\|:489' 0 ``` diff --git a/utils/sancov_runtime.c b/utils/sancov_runtime.c index 45b97819e8f..ca6cee5529b 100644 --- a/utils/sancov_runtime.c +++ b/utils/sancov_runtime.c @@ -123,7 +123,8 @@ static void write_hit(FILE *f, void *pc) * So a bare (no '/') name can only be the main executable, and * /proc/self/exe always resolves to its real absolute path. */ if (!strchr(path, '/')) { - ssize_t n = readlink("/proc/self/exe", exe_path, sizeof(exe_path) - 1); + ssize_t n = + readlink("/proc/self/exe", exe_path, sizeof(exe_path) - 1); if (n > 0) { exe_path[n] = '\0'; diff --git a/utils/sancov_symbolize.py b/utils/sancov_symbolize.py index 2ca04a95342..ef8ecfb2db1 100755 --- a/utils/sancov_symbolize.py +++ b/utils/sancov_symbolize.py @@ -104,7 +104,9 @@ def main(): # was recorded as a module but no longer existed. Skip it rather # than letting one vanished module abort the whole run. if not Path(module).exists(): - print(f"sancov_symbolize: skipping vanished module {module}", file=sys.stderr) + print( + f"sancov_symbolize: skipping vanished module {module}", file=sys.stderr + ) continue resolved = addr2line_batch(module, sorted(offsets), args.addr2line) for loc in resolved.values(): From 2b16b20e177ca50612f45d69bc2a65ebbdc81956 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:32:38 +0000 Subject: [PATCH 21/22] CMakeLists: fix g.extension addon build under WITH_SANCOV_COVERAGE cmake.yml's gcc sancov job failed a gunittest test that builds a real addon via g.extension, with "cannot find -lsancovrt". Every exported GRASS::* library target records sancovrt (deliberately an IMPORTED, non-exported target -- install(EXPORT ...) refuses a real dependency that isn't itself exported) as a bare, unresolved name in its INTERFACE_LINK_LIBRARIES, and the addon's own separate CMake project, built via the installed etc/cmake/build_addon.cmake package, has nothing by that name to resolve it to. Define the same sancovrt IMPORTED target in GRASSConfig.cmake.in, loaded by every find_package(GRASS ...) including addon builds, so it points at the runtime .so this repository's own build already installs to GRASS_INSTALL_LIBDIR. Guarded by the build's own WITH_SANCOV_COVERAGE setting, so a non-coverage install defines nothing extra. Verified locally end to end: a WITH_SANCOV_COVERAGE=ON CMake build, installed, then `g.extension extension=r.gdd operation=add` -- the exact command cmake.yml's failing test runs -- now compiles and installs r.gdd successfully, links it against the installed libsancovrt.so, and running it produces a real coverage dump. Updates the experiment report's Section 1.5 accordingly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- GRASSConfig.cmake.in | 22 ++++++++++++ doc/development/sancov_coverage_experiment.md | 34 ++++++++++++------- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/GRASSConfig.cmake.in b/GRASSConfig.cmake.in index a872475b857..0e85d309eec 100644 --- a/GRASSConfig.cmake.in +++ b/GRASSConfig.cmake.in @@ -1,5 +1,27 @@ @PACKAGE_INIT@ +# When the GRASS build this addon links against was configured with +# WITH_SANCOV_COVERAGE, every GRASS_${comp}Targets.cmake file included +# below records "sancovrt" as an INTERFACE_LINK_LIBRARIES entry (each +# GRASS library was itself linked against it via CMakeLists.txt's +# project-wide link_libraries(sancovrt)). sancovrt is deliberately an +# IMPORTED target rather than an exported one (see CMakeLists.txt), so +# that reference reaches consumers as a plain, unresolved target/library +# name -- this addon-building CMake project needs its own IMPORTED +# sancovrt target, pointing at the same installed runtime, before the +# component Targets.cmake files below try to resolve it. Without this, +# the addon's link step fails with "cannot find -lsancovrt" (confirmed +# empirically via g.extension). +if(@WITH_SANCOV_COVERAGE@ AND NOT TARGET sancovrt) + add_library(sancovrt SHARED IMPORTED GLOBAL) + set_target_properties( + sancovrt + PROPERTIES + IMPORTED_LOCATION + "@CMAKE_INSTALL_PREFIX@/@GRASS_INSTALL_LIBDIR@/libsancovrt${CMAKE_SHARED_LIBRARY_SUFFIX}" + ) +endif() + set(_GRASS_supported_components arraystats bitmap diff --git a/doc/development/sancov_coverage_experiment.md b/doc/development/sancov_coverage_experiment.md index f8242a14962..a6e00eed436 100644 --- a/doc/development/sancov_coverage_experiment.md +++ b/doc/development/sancov_coverage_experiment.md @@ -242,23 +242,33 @@ coverage dumps (Section 2 has the timings; artifacts `sancov-coverage-summary-sancov-gcc` and `-sancov-clang` on that run hold the actual line-hit summaries). -One further, still-open limitation surfaced by `cmake.yml` +A further bug surfaced by `cmake.yml` (, `gcc` job): a gunittest test that builds a GRASS addon via `g.extension` failed to compile it under `-DWITH_SANCOV_COVERAGE=ON`, with `/usr/bin/ld: cannot find -lsancovrt`. `g.extension` builds addons using the *installed* GRASS's exported CMake package -(`etc/cmake/build_addon.cmake`), which inherits the `sancovrt` -`link_libraries()` dependency, but the addon's own out-of-tree build has -no reason to know where the runtime `.so` this repository's build -produced actually lives (even though it is installed to -`${GRASS_INSTALL_LIBDIR}`, nothing threads that directory onto the -addon's link path). This was not chased further given the time already -spent on `cmake.yml`'s (correctly attributable, coverage-instrumentation- -unrelated) 2-hour-plus test run; it would need the exported package to -carry an explicit `-L`/rpath to the installed runtime, or the runtime to -be a real installed shared library with a proper CMake config file of -its own rather than a same-build `IMPORTED` target. +(`etc/cmake/build_addon.cmake`), and every `GRASS::*` library target it +exports records `sancovrt` in its `INTERFACE_LINK_LIBRARIES` (each was +linked against it via this repository's project-wide +`link_libraries(sancovrt)`). Because `sancovrt` is deliberately an +`IMPORTED` target rather than an exported one (Section 1.2's reasoning: +`install(EXPORT ...)` refuses a real dependency that is not itself part +of the export set), CMake writes that dependency into the exported +`GRASS_*Targets.cmake` files as a bare, unresolved name, and the addon's +own separate CMake project has nothing by that name to resolve it to -- +hence the plain `-lsancovrt` on the addon's link line with no `-L` for +it. Fixed in `GRASSConfig.cmake.in` (loaded by every `find_package(GRASS +...)`, addon builds included) by defining the same `sancovrt` `IMPORTED` +target there, pointing at the runtime `.so` this repository's own build +installed to `${GRASS_INSTALL_LIBDIR}`, guarded by the build's own +`WITH_SANCOV_COVERAGE` setting (substituted into the generated config, +so a non-coverage GRASS install defines nothing extra). Verified locally +end to end: a `WITH_SANCOV_COVERAGE=ON` CMake build, installed, then +`g.extension extension=r.gdd operation=add` -- the exact command +`cmake.yml`'s failing test runs -- compiles and installs `r.gdd` +successfully, `ldd` on the resulting binary shows it linked against the +installed `libsancovrt.so`, and running it writes a real coverage dump. ## 2. Build-time and test-time deltas From bd1f9b30999478d0ac0cde3a3666417b066e36e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:44:03 +0000 Subject: [PATCH 22/22] utils/sancov_symbolize.py: use a set literal for the membership test ruff's preview-mode PLR6201 rule (enabled by the CI lint job's --preview flag, not caught by a plain local `ruff check`) flags a tuple used only for `in` membership testing; a set is the right type for that and avoids a linear scan. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EFijpm8JqSEJfJ9BhJFeen --- utils/sancov_symbolize.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/sancov_symbolize.py b/utils/sancov_symbolize.py index ef8ecfb2db1..a99e11e5d81 100755 --- a/utils/sancov_symbolize.py +++ b/utils/sancov_symbolize.py @@ -110,7 +110,7 @@ def main(): continue resolved = addr2line_batch(module, sorted(offsets), args.addr2line) for loc in resolved.values(): - if loc in ("??:0", "??:?"): + if loc in {"??:0", "??:?"}: continue file_part, _, line_part = loc.rpartition(":") if not file_part or not line_part.split()[0].isdigit():