multi-server: never publish profiling results from an unclean valgrind exit

Run 30180044334 published results from two suites where valgrind had killed
FreeRADIUS partway through. Callgrind output from a killed run is truncated at
whatever point it died, so the numbers are not comparable with a clean run, and
publishing them poisons the per-suite history the regression gate compares
against.

start_valgrind_profiling.sh now records valgrind's exit status in
valgrind-exit-status alongside the rest of a test's results, and says loudly
when the status means a signal. The status is written for clean runs too, so an
absent file means the wrapper never got that far rather than that the run was
fine.

publish-profiling-results.sh refuses to publish when any test's status is
non-zero, naming each suite and status, and exits non-zero so the leg goes red
rather than passing having uploaded nothing. Trees with no status file at all
publish as before.

The status file is small and travels with the results, so the store keeps the
provenance of every published run.

diff --git a/scripts/ci/publish-profiling-results.sh b/scripts/ci/publish-profiling-results.sh
index 8d2e903946581da7ebecbc0ba2257c52f65219aa..c017f838d1bd5e7d6c726ce7636d2af354fddcf9 100755 (executable)
--- a/scripts/ci/publish-profiling-results.sh
+++ b/scripts/ci/publish-profiling-results.sh
@@ -22,6 +22,10 @@ Core dumps are excluded; collect-core-dumps.sh keeps those instead. Run from
 the directory holding prof-results/. A missing prof-results/ tree, or one
 holding nothing publishable, is a quiet success.
 
+Fails without publishing when any test's valgrind-exit-status is non-zero,
+because a run valgrind killed has truncated callgrind output whose numbers are
+not comparable with previous runs.
+
   <url>  Where to POST. Its origin becomes the OIDC audience.
   -h     Show this help.
 
@@ -49,6 +53,30 @@ audience="${url%%://*}://${host_path%%/*}"
 
 [ -d prof-results ] || { echo "no prof-results/ tree; skipping"; exit 0; }
 
+#  Refuse to publish a run valgrind did not finish cleanly. start_valgrind_-
+#  profiling.sh drops a valgrind-exit-status file in each test's results dir; a
+#  non-zero status means valgrind was killed, which leaves callgrind output
+#  truncated at whatever point it died. Numbers from a truncated run are not
+#  comparable with a clean one, and publishing them silently poisons the
+#  per-suite history the regression gate compares against. Exits non-zero so
+#  the leg goes red rather than passing with nothing uploaded.
+unclean=""
+for status_file in $(find prof-results -type f -name valgrind-exit-status | sort); do
+       read -r status <"$status_file" || status="unreadable"
+       case $status in
+       0)      continue ;;
+       esac
+       unclean="${unclean} ${status_file%/valgrind-exit-status}:${status}"
+done
+if [ -n "$unclean" ]; then
+       echo "ERROR: refusing to publish, valgrind exited uncleanly in:" >&2
+       for entry in $unclean; do
+               echo "         ${entry%:*} (status ${entry##*:})" >&2
+       done
+       echo "ERROR: truncated profiling data is not comparable with previous runs" >&2
+       exit 1
+fi
+
 tmpdir=$(mktemp -d)
 trap 'rm -rf "$tmpdir"' EXIT
 

diff --git a/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh b/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh
index a4a006d68fa30ed2a8e9cb8ff9ad6ddc5d0fa275..92f81e757c0f9ce75919104f7fcc7454b20b80ca 100755 (executable)
--- a/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh
+++ b/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh
@@ -116,9 +116,28 @@ echo "INFO: disabling callgrind instrumentation"
 CTRL_OUT=$(callgrind_control --instr=off 2>/dev/null || true)
 printf '%s\n' "$CTRL_OUT"
 
-# Wait for valgrind to finish writing callgrind output
+# Wait for valgrind to finish writing callgrind output. Record how it exited:
+# a run valgrind killed produces truncated callgrind output whose numbers are
+# not comparable with a clean run, so the status has to survive to the publish
+# step, which reads this file and refuses to upload an unclean run. The status
+# is recorded for clean runs too, so an absent file means "the wrapper did not
+# get this far" rather than "the run was fine".
 echo "INFO: waiting for valgrind to exit"
-wait ${VALGRIND_PID} 2>/dev/null || true
+VALGRIND_STATUS=0
+wait ${VALGRIND_PID} 2>/dev/null || VALGRIND_STATUS=$?
+echo "${VALGRIND_STATUS}" > /etc/prof-results/valgrind-exit-status
+
+if [ "${VALGRIND_STATUS}" -ne 0 ]; then
+  #  Over 128 means a signal. 139 is SIGSEGV, which is how valgrind exiting on
+  #  its 8 MB brk segment ceiling presents; valgrind.log names the real reason
+  #  on the line above its backtrace.
+  if [ "${VALGRIND_STATUS}" -gt 128 ]; then
+    echo "ERROR: valgrind was killed by signal $((VALGRIND_STATUS - 128)); profiling data is truncated" >&2
+  else
+    echo "ERROR: valgrind exited ${VALGRIND_STATUS}; profiling data may be truncated" >&2
+  fi
+  echo "ERROR: see valgrind.log for the reason; these results will not be published" >&2
+fi
 
 # Signal that valgrind has finished writing all profiling data
 echo "INFO: Profiling complete at $(date)"