From: Arran Cudbard-Bell Date: Sun, 26 Jul 2026 02:00:56 +0000 (-0600) Subject: multi-server: never publish profiling results from an unclean valgrind exit X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4f59e0d8f021acb47b60ba4ac952ab16aacaecf3;p=thirdparty%2Ffreeradius-server.git 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 8d2e9039465..c017f838d1b 100755 --- 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. + 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 a4a006d68fa..92f81e757c0 100755 --- 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)"