From 7d38281b069c61c26f8f0c7ddc2deea5a3766581 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Sat, 8 Aug 2026 19:13:27 -0600 Subject: [PATCH] test: ask run_san.sh for its subset, with the override cleared (#521 follow-up) run_san.sh gains --list-suites, answered before its prerequisite check so it works on a box with no sanitizer build, and harness_selftest asks it instead of parsing the SUITES= block with sed. **The reason I gave for this in review of #521 was wrong, and the correction is the useful part.** I claimed the dangerous direction was a suite named in a COMMENT inside the SUITES= block: satisfying the grep and reporting coverage that does not exist. That cannot happen. SUITES= is a double-quoted string spanning backslash continuations, not a bash array, so a `#` inside it is literal text and not a comment. I tested it rather than asserting it, and the simulated hazard failed to reproduce -- the runner and the parser agreed, because the "comment" was still string content. So the source parse was not unsafe in the way I said. What it was, was a second reading of a definition that only run_san.sh should own, which is #473's rule and CONTEXT.md's: ask the runner, never parse the source. That reason stands on its own and does not need the hazard I invented. **And a plain --list-suites here is a REGRESSION, which is why the override is cleared.** The claim under test is about the shipped DEFAULT. Asking the runner inherits PGC_SAN_SUITES, so a developer with an override exported gets a red from a check that is not about their override. Verified: with PGC_SAN_SUITES='smoke differential' the runner reports no encode_invariants, and the check fails while the default is perfectly correct. `env -u` fixes it. Three directions verified rather than one: default 54 checks, PASSED with PGC_SAN_SUITES exported 54 checks, PASSED (would have red) encode_invariants cut from default FAIL ... got [encode_invariants] want [] Refs #521 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FeNm2Gw6h16Z123We3F1vJ --- test/harness_selftest.sh | 13 ++++++++++++- test/run_san.sh | 30 ++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/test/harness_selftest.sh b/test/harness_selftest.sh index 191a7a48..12ea1b51 100755 --- a/test/harness_selftest.sh +++ b/test/harness_selftest.sh @@ -634,7 +634,18 @@ check "a failing suite names the first fatal event in its log" \ # # Asked as "every suite that drives the selftest" rather than by name, so moving # the selftest to another suite cannot quietly narrow this. -_san_suites="$(sed -n '/^SUITES=/,/}"/p' "$PGC_SRCDIR/test/run_san.sh")" +# Ask the runner rather than parsing the source (CONTEXT.md, #473) -- but with +# PGC_SAN_SUITES cleared, because the claim under test is about the SHIPPED +# DEFAULT, not about whatever an operator overrode it with for one run. +# +# Both halves are load-bearing and each fixes a different defect. Asking the +# runner means this cannot drift from what run_san.sh actually iterates. Clearing +# the variable means a developer with an override exported does not get a red +# from a check that is not about their override -- which is what a plain +# --list-suites here produces, verified: with PGC_SAN_SUITES='smoke differential' +# the runner reports no encode_invariants and this check would fail while the +# shipped default is perfectly correct. +_san_suites="$(env -u PGC_SAN_SUITES bash "$PGC_SRCDIR/test/run_san.sh" --list-suites 2>/dev/null)" check "premise: run_san.sh's default subset was found and is non-empty" \ "$([ -n "$_san_suites" ] && echo yes || echo no)" "yes" diff --git a/test/run_san.sh b/test/run_san.sh index bc7939c3..3edbaa7f 100644 --- a/test/run_san.sh +++ b/test/run_san.sh @@ -34,6 +34,30 @@ # set -uo pipefail +# The subset, and the one place it is defined. +# +# Declared before anything else runs so --list-suites can answer without building +# a sanitizer PostgreSQL first, and so a caller asking what would be sanitized +# gets the same string the loop below iterates -- not a second copy of it, and +# not a text parser's reading of this file. #473 replaced exactly that pattern in +# run_all_versions.sh, and CONTEXT.md records why: a parser over the array +# disagrees with the shell on the mistake this invites, and the disagreement is +# silent. +SUITES="${PGC_SAN_SUITES:-smoke native_writer native_roundtrip native_encoding \ + native_zonemap write_fsst_compressed write_minmax_fastpath encode_effort \ + encode_invariants \ + native_dml native_skip native_fetch_position native_fetch_cache \ + arrow_import arrow_export parquet_import parquet_export native_read_parquet \ + native_parquet_schema hardening corruption differential fuzz_arrow fuzz_parquet}" + +# --list-suites: print the subset, one name per line, and exit. Answered before +# the prerequisite check below, so it works on a box with no sanitizer build at +# all -- a caller asking "what does this cover" should not need one. +if [ "${1:-}" = "--list-suites" ]; then + printf '%s\n' $SUITES + exit 0 +fi + SAN="${1:-/usr/local/pg18_san}" PGCONF="$SAN/bin/pg_config" if [ ! -x "$PGCONF" ]; then @@ -75,12 +99,6 @@ if ! make -s PG_CONFIG="$PGCONF" install > /tmp/run-san-ext.log 2>&1; then fi FUZZ_ITERS="${PGC_SAN_FUZZ_ITERS:-200}" -SUITES="${PGC_SAN_SUITES:-smoke native_writer native_roundtrip native_encoding \ - native_zonemap write_fsst_compressed write_minmax_fastpath encode_effort \ - encode_invariants \ - native_dml native_skip native_fetch_position native_fetch_cache \ - arrow_import arrow_export parquet_import parquet_export native_read_parquet \ - native_parquet_schema hardening corruption differential fuzz_arrow fuzz_parquet}" echo "-- running the subset under ASAN+UBSAN (fatal)" fail=0