test: refuse to measure a binary that was not built from this source

.gitignore

@@ -7,5 +7,6 @@ results/
 regression.diffs
 regression.out
 .pgc_built_for_major
+.pgc_source_stamp.*
 __pycache__/
 *.pyc

CHANGELOG.md

@@ -18,6 +18,36 @@ true until the next version shipped.
 
 ### Added
 
+- The test harness refuses to measure a binary that was not built from the source
+  under test.
+
+  `test/run_all_versions.sh` builds and installs once per major and then runs every
+  suite with `PGC_SKIP_BUILD=1`, so no suite could tell whether the binary it
+  measured came from this tree. The controller now records a fingerprint of the
+  build inputs after a successful install, and `pgc_setup` checks it in every
+  suite, whether that suite built or skipped. A mismatch is fatal and names both
+  fingerprints.
+
+  A second check covers the other half. `make install` does not reload anything:
+  `shared_preload_libraries` maps the library at postmaster start, so a reinstall
+  under a running server leaves the backends executing older code than the file on
+  disk. The `.so` mtime is compared against `pg_postmaster_start_time()`, and the
+  message says to restart rather than only that something is wrong.
+
+  Neither check fails when it cannot answer. Someone who ran `make install` by hand
+  has no stamp, so that case prints `freshness UNVERIFIED` and says which question
+  went unanswered, rather than printing nothing and letting a reader assume it
+  passed.
+
+  `pgc_freshness_verdict` and `pgc_running_binary_verdict` are pure functions of
+  strings, so `test/selftest/340-the-binary-must-be-built-from.sh` drives them
+  directly: 27 arms including both empty inputs, a non-numeric epoch, equal
+  timestamps on the boundary, sensitivity to each fingerprint input class, and
+  the three the review added -- the derived build-directory set, the running
+  binary check driven against a live cluster, and devloop writing the stamp.
+  `harness_selftest` goes from 261 checks to 288, both measured.
+
+
 - Hilbert clustering: `pgcolumnar.cluster_hilbert` and
   `pgcolumnar.recluster_hilbert` (#889).
 

test/devloop.sh

@@ -77,6 +77,33 @@ if ! bash test/rebuild.sh "$PGC"; then
 	exit 1
 fi
 
+# THIS SCRIPT IS A CONTROLLER, so it records what it just installed.
+#
+# It builds and installs, then runs every suite with PGC_SKIP_BUILD=1. Without a
+# stamp those suites print "freshness UNVERIFIED" and continue, so the class this
+# check exists to catch -- edit a file, forget to rebuild, measure the old binary
+# -- was unprotected in exactly the loop where a human does it (@jdatcmd, #898
+# review). run_all_versions.sh was the only writer, and nobody edits C inside the
+# matrix.
+#
+# In a subshell sourcing lib.sh rather than recomputing the hash here: two
+# implementations of one fingerprint drift, and the suites compare against
+# whatever this writes.
+#
+# NOT `|| true`. If the stamp cannot be written, every suite below reports
+# UNVERIFIED and this stops being a controller with nothing saying so.
+if (
+	. "$BUILD/test/lib.sh"
+	pgc_write_source_stamp \
+		"$(pgc_source_stamp_path "$BUILD" "$(pgc_major_of "$PGC")")" \
+		"$(pgc_source_fingerprint "$BUILD")"
+); then
+	:
+else
+	echo "devloop: could not record the source stamp; the suites below will" >&2
+	echo "         report freshness UNVERIFIED rather than checking it" >&2
+fi
+
 rc=0
 for s in "$@"; do
 	echo "===================================================================="

test/lib.sh

@@ -141,7 +141,7 @@ pgc_setup() {
 	# A behavior that exists only from some major is core's, not this extension's,
 	# and a check written against the newer one fails on the older ones for a
 	# reason that is not a defect. Branch on this rather than deriving it again.
-	PGC_MAJOR="$("$PGC_PG_CONFIG" --version | sed -E 's/^[^0-9]*([0-9]+).*/\1/')"
+	PGC_MAJOR="$(pgc_major_of "$PGC_PG_CONFIG")"
 	# Derived from this process rather than a fixed 54329: two suites run at
 	# once on one box otherwise start on the same port, and the loser reports a
 	# wall of ERROR: database "regress" already exists with no named check
@@ -222,6 +222,17 @@ pgc_setup() {
 			echo "       (refusing to report checks against the previously installed .so)" >&2
 			exit 1
 		fi
+
+		# THIS RUN IS THE CONTROLLER for whatever follows in this batch: it built
+		# and installed, so record what the binary was built from. The stamp is
+		# written HERE and nowhere else. An earlier revision wrote it in the
+		# skip-build branch instead, which made the check tautological -- every run
+		# recorded the source it was about to compare against, so a suite measuring
+		# an edited tree reported "matches the binary under test". My own red arm
+		# caught it, which is the only reason this comment exists.
+		pgc_write_source_stamp \
+			"$(pgc_source_stamp_path "$PGC_SRCDIR" "$PGC_MAJOR")" \
+			"$(pgc_source_fingerprint "$PGC_SRCDIR")"
 	else
 		# Named because the variable is not what it says. It reads as "skip the
 		# build" and means "skip the build AND the install, and test whatever is
@@ -234,6 +245,29 @@ pgc_setup() {
 
 	pgc_so_line
 
+	# And verify it, whether this run built or skipped. A skipped build is exactly
+	# when the binary can be older than the source.
+	_pgc_fresh_recorded="$(pgc_read_source_stamp \
+		"$(pgc_source_stamp_path "$PGC_SRCDIR" "$PGC_MAJOR")")"
+	_pgc_fresh_current="$(pgc_source_fingerprint "$PGC_SRCDIR")"
+	case "$(pgc_freshness_verdict "$_pgc_fresh_recorded" "$_pgc_fresh_current")" in
+		fresh)
+			echo "-- source: $_pgc_fresh_current matches the binary under test"
+			;;
+		stale)
+			echo "FATAL: the binary under test was not built from this source" >&2
+			echo "       source now $_pgc_fresh_current, binary built from $_pgc_fresh_recorded" >&2
+			echo "       (refusing to report checks about code that is not installed)" >&2
+			exit 1
+			;;
+		unknown)
+			# Not a failure: a person who ran make install by hand has no stamp, and
+			# refusing would break a documented workflow. Said plainly so the reader
+			# knows which question was not answered.
+			echo "-- source: $_pgc_fresh_current, freshness UNVERIFIED (no stamp for major $PGC_MAJOR)"
+			;;
+	esac
+
 	echo "-- initdb"
 	pgc_pg "initdb -D '$PGC_PGDATA' -A trust" >/dev/null 2>&1
 	{
@@ -342,6 +376,10 @@ pgc_setup() {
 	# "already exists" looks exactly like a suite that landed on someone else's
 	# cluster. Removing the noise is most of the value here; failing loudly on
 	# the impossible case is the rest.
+	# The server is up, so now ask whether it is RUNNING the binary we verified on
+	# disk. A cp is not enough: shared_preload_libraries maps the .so at start.
+	pgc_check_running_binary "$("$PGC_PG_CONFIG" --pkglibdir)/pgcolumnar.so" || exit 1
+
 	{
 		local _exists
 
@@ -524,6 +562,174 @@ pgc_build_needs_clean() {
 # backslash inside single quotes -- which emits the four bytes `1 9 \ n`. That
 # passed unnoticed because the reader does tr -dc '0-9' and strips the junk; a
 # direct comparison against the major failed. Found in review, not by a check.
+# ---- is the binary under test built from the source in this tree? -----------
+#
+# THE GAP THIS CLOSES, AND WHAT ALREADY COVERED THE REST.
+#
+# selftest 110 compares the INSTALLED .so against the one built in this tree, so a
+# missed install and a foreign overwrite are already caught. Neither that check nor
+# pgc_so_line can see the case where BOTH copies agree with each other and both are
+# stale against edited source: nothing in the harness derives anything from the
+# source text. That is the hole, and it is the one PGC_SKIP_BUILD opens widest,
+# because its whole purpose is not to rebuild.
+#
+# Measured cost of the miss: a probe run under PGC_SKIP_BUILD=1 that asserted its fix
+# was "present" by grepping the SOURCE while measuring a .so another worktree had
+# installed. The control failed and the failure read as a product defect.
+#
+# THE CONTROLLER SHAPE. Whoever builds records a fingerprint of the build inputs
+# beside the install. Every suite in that batch recomputes the fingerprint and
+# compares. One hash per suite, one build per batch, and a stale binary can no longer
+# report a plausible list of checks.
+#
+# pgc_freshness_verdict is a pure function of two strings so it can be tested without
+# a build, the same reason pgc_build_needs_clean is.
+
+# The build inputs, hashed. Sources, headers, the Makefile, the control file and the
+# SQL that ships: anything whose change should invalidate a binary. Sorted, because a
+# directory listing is not ordered and an unordered input makes the hash unstable.
+# pgc_source_build_dirs DIR -> one source directory per line
+#
+# DERIVED, NOT LISTED. The first version read $dir/src only, and objstore/ is a
+# SEPARATE shared library that the top-level Makefile builds and installs by
+# recursion. Editing objstore/columnar_objstore_module.c left the fingerprint
+# unchanged, so objstore_module, objstore_sink_write and objstore_stash_recovery
+# could measure a stale module while the suite printed "matches the binary under
+# test" (@jdatcmd, #898 review). A stale binary under an explicit assurance is
+# worse than one under no assurance, because the line is what stops the next
+# person checking.
+#
+# A remembered list would have the same defect again the next time a module is
+# added, so this returns every directory that has its own Makefile. That is the
+# same rule the build itself follows.
+pgc_source_build_dirs() {	# pgc_source_build_dirs DIR -> dirs
+	local dir="${1:-.}"
+	printf '%s\n' "$dir/src"
+	find "$dir" -mindepth 2 -maxdepth 2 -type f -name Makefile \
+		-printf '%h\n' 2>/dev/null | grep -v "^$dir/src$" || true
+}
+
+pgc_source_fingerprint() {	# pgc_source_fingerprint DIR -> hash
+	local dir="${1:-.}"
+	local d
+	{
+		while IFS= read -r d; do
+			[ -n "$d" ] || continue
+			find "$d" -maxdepth 1 -type f \( -name '*.c' -o -name '*.h' \
+				-o -name 'Makefile' \) -print0 2>/dev/null
+		done < <(pgc_source_build_dirs "$dir")
+		find "$dir" -maxdepth 1 -type f \( -name 'Makefile' -o -name '*.control' \
+			-o -name '*.sql' \) -print0 2>/dev/null
+	} | sort -z | xargs -0 cat 2>/dev/null | md5sum | cut -c1-12
+}
+
+# fresh   the binary was built from this source
+# stale   it was not, and every check that follows would be about the wrong code
+# unknown nobody in this batch recorded a fingerprint, so this cannot be answered
+pgc_freshness_verdict() {	# pgc_freshness_verdict RECORDED CURRENT -> verdict
+	local recorded="${1:-}" current="${2:-}"
+	[ -z "$recorded" ] && { echo unknown; return; }
+	[ -z "$current" ] && { echo unknown; return; }
+	[ "$recorded" = "$current" ] && echo fresh || echo stale
+}
+
+# AND A cp IS NOT ENOUGH: THE POSTMASTER MAPS THE .so AT START.
+#
+# shared_preload_libraries='pgcolumnar' means the library is loaded once, when the
+# postmaster starts. `make install` over a running instance changes the file and
+# nothing else: every backend keeps executing the code it already mapped. So a
+# binary can match the source exactly and the server can still be running something
+# older, which the source check above cannot see.
+#
+# The harness normally escapes this because each suite initdb's and starts its own
+# cluster after the install. It stops escaping it the moment a cluster outlives an
+# install: a persistent cluster reused between runs, a bench rig left up, or a second
+# batch installing into a prefix whose server is already serving.
+#
+# So compare when the binary was installed against when the server started. A
+# postmaster older than the binary has the old code mapped, whatever the file says.
+#
+# fresh     the server started after the binary was installed
+# predates  the binary is newer than the server, so the server has older code
+# unknown   one of the two timestamps could not be read
+# pgc_check_running_binary SO_PATH
+#
+# The runtime half of the freshness check: does the RUNNING server postdate the
+# library on disk? Extracted from pgc_setup so it can be driven, because the
+# verdict function alone could not be.
+#
+# WHY THAT MATTERS. Every suite initdb's a fresh cluster and starts it after the
+# install, so through any shipped path the postmaster is always newer than the
+# .so and `predates` is UNREACHABLE. selftest 340 fed the verdict function
+# fixture values and proved its arithmetic; the CALL SITE could have been deleted
+# with every check still passing (@jdatcmd, #898 review). That is this project's
+# own rule about a helper a suite merely sources.
+#
+# Taking the path as an argument is what makes it reachable: the selftest points
+# it at a file it has just touched, so the stat, the pg_postmaster_start_time()
+# query, the verdict and the refusal all run for real and only the path is
+# redirected. Nothing has to touch the installed library to prove the guard
+# fires.
+#
+# Returns non-zero rather than calling exit, so a caller that is not a suite can
+# turn it into its own kind of failure. pgc_setup passes the status through.
+pgc_check_running_binary() {	# pgc_check_running_binary SO_PATH -> 0|1
+	local _so_path="$1" _so_epoch _pm_epoch
+	_so_epoch="$(stat -c %Y "$_so_path" 2>/dev/null || echo '')"
+	_pm_epoch="$(psql_admin_scalar \
+		"SELECT floor(extract(epoch from pg_postmaster_start_time()))::bigint;" \
+		2>/dev/null | tr -dc '0-9')"
+	case "$(pgc_running_binary_verdict "$_so_epoch" "$_pm_epoch")" in
+		fresh)
+			echo "-- server: started after the binary was installed"
+			;;
+		predates)
+			echo "FATAL: this server was already running when the binary changed" >&2
+			echo "       .so installed at epoch $_so_epoch, postmaster started $_pm_epoch" >&2
+			echo "       shared_preload_libraries maps the library at start, so the" >&2
+			echo "       backends are executing older code than the file on disk." >&2
+			echo "       Restart the cluster; a reinstall alone does not reload it." >&2
+			return 1
+			;;
+		unknown)
+			echo "-- server: could not compare binary and postmaster timestamps"
+			;;
+	esac
+	return 0
+}
+
+pgc_running_binary_verdict() {	# pgc_running_binary_verdict SO_EPOCH PM_EPOCH
+	local so="${1:-}" pm="${2:-}"
+	case "$so" in '' | *[!0-9]*) echo unknown; return ;; esac
+	case "$pm" in '' | *[!0-9]*) echo unknown; return ;; esac
+	[ "$pm" -ge "$so" ] && echo fresh || echo predates
+}
+
+pgc_write_source_stamp() {	# pgc_write_source_stamp FILE HASH
+	printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null || true
+}
+
+pgc_read_source_stamp() {	# pgc_read_source_stamp FILE -> hash or empty
+	[ -r "${1:-}" ] || { echo ""; return; }
+	tr -dc 'a-f0-9' < "$1" | head -c 12
+}
+
+# The stamp lives beside the tree that built the binary, keyed by major, because one
+# tree installs into several prefixes and each has its own binary.
+# pgc_major_of PG_CONFIG -> major version number
+#
+# One definition, because devloop.sh now writes a stamp whose PATH is keyed on
+# the major and pgc_setup reads it back. Two copies of this sed would be two
+# answers to "which major", and the failure would be a stamp written where
+# nothing looks for it -- a silent UNVERIFIED rather than an error.
+pgc_major_of() {	# pgc_major_of PG_CONFIG -> major
+	"$1" --version | sed -E 's/^[^0-9]*([0-9]+).*/\1/'
+}
+
+pgc_source_stamp_path() {	# pgc_source_stamp_path DIR MAJOR
+	printf '%s/.pgc_source_stamp.%s\n' "${1:-.}" "${2:-0}"
+}
+
 pgc_write_build_stamp() {
 	printf '%s\n' "${2:-}" > "${1:-/dev/null}" 2>/dev/null || true
 }

test/run_all_versions.sh

@@ -710,6 +710,31 @@ for pgc in "${CONFIGS[@]}"; do
 		continue
 	fi
 
+	# THIS RUNNER IS THE CONTROLLER FOR THIS MAJOR'S BATCH. It built and installed
+	# once; the suites below all run with PGC_SKIP_BUILD=1 and would otherwise have
+	# no way to tell whether the binary they measure came from this tree. Record the
+	# fingerprint of the build inputs so each of them can check it.
+	#
+	# In a subshell sourcing lib.sh rather than recomputing the hash here: two
+	# implementations of one fingerprint would drift, and the suites compare against
+	# whatever this writes. lib.sh's top level is assignments and function
+	# definitions only, so sourcing it costs nothing and starts nothing.
+	# NOT `|| true`. If the stamp cannot be written, every suite in this batch
+	# reports "freshness UNVERIFIED" and the controller arm silently stops being a
+	# controller arm -- the whole batch degrades to the state this exists to
+	# prevent, and nothing says so. Say so.
+	if (
+		. "$builddir/test/lib.sh"
+		pgc_write_source_stamp \
+			"$(pgc_source_stamp_path "$builddir" "$major")" \
+			"$(pgc_source_fingerprint "$builddir")"
+	); then
+		:
+	else
+		echo "WARNING: could not record the source stamp for major $major." >&2
+		echo "         Every suite below will report freshness UNVERIFIED." >&2
+	fi
+
 	verfail=0
 	results=""
 	maxjobs="${PGC_JOBS:-6}"