diff --git a/.github/actions/test-margin-and-environment/action.yml b/.github/actions/test-margin-and-environment/action.yml new file mode 100644 index 000000000..e27af4ee1 --- /dev/null +++ b/.github/actions/test-margin-and-environment/action.yml @@ -0,0 +1,43 @@ +name: Record the test margin and the DDS environment +description: >- + Says plainly whether a test step ran out of time, and records what /dev/shm and + the installed middleware looked like afterwards. Both are invisible once a run + has finished, and a step killed by its cap is reported by the runner as an + ordinary failure naming whichever test was in flight. + +inputs: + start: + description: Epoch seconds recorded immediately before the test step. + required: true + cap-minutes: + description: >- + The cap the guard measures against. Pass the step's timeout-minutes where + the step has one; where only the job is capped, pass the job's - the + overrun branch cannot fire there, because a job-level kill stops every + remaining step, but the margin warning still arrives in time to act on. + required: true + +runs: + using: composite + steps: + - shell: bash + run: | + # The start is missing only when an earlier step failed before it ran. + # Say so and carry on to the census, rather than dying on a usage error + # and taking the environment evidence down with it. + if [ -n "${{ inputs.start }}" ]; then + ./scripts/ci_test_completeness.sh "${{ inputs.start }}" "${{ inputs.cap-minutes }}" || guard_rc=$? + else + echo "::notice::no test step start was recorded - the run failed before the tests" + fi + # A DDS participant killed rather than shut down leaves its segments + # behind for the rest of the job, and the installed middleware decides + # which transport those segments belong to. + df -h /dev/shm + echo "shm segments: $(ls /dev/shm | wc -l)" + # No `head` in this pipeline: it closes the pipe early, SIGPIPEs `ls`, and + # the step's `pipefail` would then kill the census exactly when there are + # enough segments to be worth listing. + ls -la /dev/shm | tail -n +1 + dpkg -l | grep -E 'rmw-fastrtps|fastdds|cyclonedds' || true + exit "${guard_rc:-0}" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a791b10d..0c7888b0e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,10 +19,21 @@ jobs: os_image: ubuntu:resolute container: image: ${{ matrix.os_image }} + # A container gets 64 MB of /dev/shm by default, which is Docker's number and + # not a considered one. Fast DDS puts a 512 KB segment plus port files there + # per participant - measured at ~0.65 MB - and a participant killed rather + # than shut down never gets its segments back. Measured: 15 live + # participants take 9.6 MB, and 125 killed ones fill 63 MB. 1g is tmpfs, so + # it costs only what is used, against 16 GB of runner memory. + options: --shm-size=1g # Must stay above the build plus the test step's own budget below, or the job cap kills the # run before that step's cap can - which loses the "which step ran long" answer. The build # side of this job is around 18 minutes. timeout-minutes: 90 + env: + # One source for the test step's cap. The step's timeout-minutes and the + # guard that reports the margin both read this. + TEST_STEP_CAP_MINUTES: 45 defaults: run: shell: bash @@ -88,6 +99,9 @@ jobs: ccache -s ./scripts/ccache_report.sh "${{ matrix.ros_distro }}" + - name: Record the test step start + run: echo "TEST_STEP_START=$(date +%s)" >> "$GITHUB_ENV" + - name: Run unit and integration tests # The suite keeps outgrowing this budget as packages land, and every overrun so far has # been real work finishing rather than a hang: 15 minutes was raised to 25 when lyrical @@ -95,7 +109,12 @@ jobs: # arrived (lyrical 22m50s before them, jazzy 28m15s after). The jazzy job below caps the # whole job instead of this step, so only these two distros can be killed mid-package - # which reads as a test failure and hides every test the kill cut off. - timeout-minutes: 45 + # The cap lives in the job's env, referenced here and by the guard below, + # so raising one raises the other and they cannot drift apart. + # fromJSON, because an expression yields a string and this key demands a + # number: `${{ env.X }}` alone fails the workflow with "Unexpected value + # '45'" before any step runs. + timeout-minutes: ${{ fromJSON(env.TEST_STEP_CAP_MINUTES) }} env: # FastRTPS 2.6 on Humble has a known use-after-free in the # discovery-teardown path (EDP::unpairWriterProxy) that segfaults @@ -116,6 +135,13 @@ jobs: --ctest-args -LE linter \ --event-handlers console_direct+ + - name: Check the run completed, and record the environment + if: always() + uses: ./.github/actions/test-margin-and-environment + with: + start: ${{ env.TEST_STEP_START }} + cap-minutes: ${{ env.TEST_STEP_CAP_MINUTES }} + - name: Show test results if: always() run: colcon test-result --verbose @@ -159,6 +185,13 @@ jobs: ccache_prefix: ccache-jazzy-test- container: image: ${{ matrix.os_image }} + # A container gets 64 MB of /dev/shm by default, which is Docker's number and + # not a considered one. Fast DDS puts a 512 KB segment plus port files there + # per participant - measured at ~0.65 MB - and a participant killed rather + # than shut down never gets its segments back. Measured: 15 live + # participants take 9.6 MB, and 125 killed ones fill 63 MB. 1g is tmpfs, so + # it costs only what is used, against 16 GB of runner memory. + options: --shm-size=1g timeout-minutes: 90 defaults: run: @@ -229,6 +262,9 @@ jobs: ccache -s ./scripts/ccache_report.sh "${{ matrix.ros_distro }}-graph-watchdog" + - name: Record the test step start + run: echo "TEST_STEP_START=$(date +%s)" >> "$GITHUB_ENV" + - name: Run graph_watchdog tests timeout-minutes: 45 env: @@ -244,6 +280,13 @@ jobs: --ctest-args -LE linter \ --event-handlers console_direct+ + - name: Check the run completed, and record the environment + if: always() + uses: ./.github/actions/test-margin-and-environment + with: + start: ${{ env.TEST_STEP_START }} + cap-minutes: '45' + - name: Show test results if: always() run: colcon test-result --verbose @@ -270,6 +313,13 @@ jobs: runs-on: ubuntu-latest container: image: ubuntu:noble + # A container gets 64 MB of /dev/shm by default, which is Docker's number and + # not a considered one. Fast DDS puts a 512 KB segment plus port files there + # per participant - measured at ~0.65 MB - and a participant killed rather + # than shut down never gets its segments back. Measured: 15 live + # participants take 9.6 MB, and 125 killed ones fill 63 MB. 1g is tmpfs, so + # it costs only what is used, against 16 GB of runner memory. + options: --shm-size=1g # 90, like build-and-test and graph-watchdog. This job covers both the build # and the test phase, a cold ccache makes the build phase the variable one, # and at 60 it had been finishing in 54 to 56 minutes on main - close enough @@ -339,6 +389,9 @@ jobs: ccache -s ./scripts/ccache_report.sh jazzy-test + - name: Record the test step start + run: echo "TEST_STEP_START=$(date +%s)" >> "$GITHUB_ENV" + - name: Run unit and integration tests env: # FastRTPS has a known use-after-free in discovery-teardown that @@ -355,6 +408,13 @@ jobs: --ctest-args -LE linter \ --event-handlers console_direct+ + - name: Check the run completed, and record the environment + if: always() + uses: ./.github/actions/test-margin-and-environment + with: + start: ${{ env.TEST_STEP_START }} + cap-minutes: '90' + - name: Show test results if: always() run: colcon test-result --verbose @@ -385,10 +445,53 @@ jobs: # ctest -LE linter per package, a superset of the selection here - they do # not skip ros2_medkit_opcua - so the asserts are still exercised against # unit and integration tests on every PR. + # + # Sharded because this job reached 89m48s against its 90 minute cap - twelve + # seconds - and its test step grew 166% in a month. It is the only test-bearing + # job that does not skip ros2_medkit_graph_watchdog, so it alone absorbed the + # 24 minute suite the others carved out. Measured test time per package on one + # run: integration_tests 1050s, graph_watchdog ~1440s, gateway + fault_manager + # 389s, everything else 85s. The shards below follow those numbers. + # + # Each shard builds the whole workspace, because every package needs its + # dependencies built; only the test selection differs. Minutes are free on a + # public repository and wall clock is not, so paying for the build four times + # to quarter the wall clock is the right trade here. + name: coverage (${{ matrix.shard }}) if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest + strategy: + # Mandatory. With fail-fast a single failing shard cancels the others, and a + # cancelled job is indistinguishable from one killed by its cap. + fail-fast: false + matrix: + include: + - shard: integration + select: --packages-select ros2_medkit_integration_tests + - shard: graph-watchdog + select: --packages-select ros2_medkit_graph_watchdog + - shard: gateway + select: --packages-select ros2_medkit_gateway ros2_medkit_fault_manager + - shard: rest + select: >- + --packages-skip ros2_medkit_opcua ros2_medkit_integration_tests + ros2_medkit_graph_watchdog ros2_medkit_gateway ros2_medkit_fault_manager + # Left at 90 deliberately. Measured on the first sharded run: the worst shard + # (graph-watchdog) took 3046s, then integration 2663s, gateway 2041s and rest + # 1462s, against 86-90 minutes serial before. Lower the cap once a few runs + # agree, not from one cold-cache sample. Note the margin guard in + # build-and-test does NOT run here: this job caps the job rather than a step, + # and a job-level kill stops every remaining step, `if: always()` included, so + # there is nothing a guard could report. container: image: ubuntu:noble + # A container gets 64 MB of /dev/shm by default, which is Docker's number and + # not a considered one. Fast DDS puts a 512 KB segment plus port files there + # per participant - measured at ~0.65 MB - and a participant killed rather + # than shut down never gets its segments back. Measured: 15 live + # participants take 9.6 MB, and 125 killed ones fill 63 MB. 1g is tmpfs, so + # it costs only what is used, against 16 GB of runner memory. + options: --shm-size=1g timeout-minutes: 90 defaults: run: @@ -418,6 +521,11 @@ jobs: uses: actions/cache@v4 with: path: /root/.cache/ccache + # One entry for the whole matrix, not one per shard: every shard builds + # the same workspace with the same flags, so per-shard entries would + # quadruple this cache against a 10 GB repository limit and evict the + # very entries that keep the builds warm. Concurrent shards racing to + # save the same key is benign - the first wins, the rest log it. key: ccache-coverage-${{ github.sha }} restore-keys: | ccache-coverage- @@ -455,10 +563,30 @@ jobs: # build and could not fail CI on a failure - it read as a gate # without being one. colcon test --return-code-on-test-failure \ - --packages-skip ros2_medkit_opcua \ + ${{ matrix.select }} \ --ctest-args -LE linter \ --event-handlers console_direct+ + # colcon WARNS and exits 0 on a --packages-select name that does not + # exist, so a rename would leave this shard testing nothing while the + # merged report still looked complete - another shard's tests execute + # the same installed code and put it back in the report. + # + # Only for --packages-select. The catch-all shard uses --packages-skip, + # where the names that follow are the packages it must NOT test - the + # first of them is ros2_medkit_opcua, which this job never builds, so + # checking them would fail that shard on every run. + set -- ${{ matrix.select }} + if [ "$1" = "--packages-select" ]; then + shift + for pkg in "$@"; do + if [ -z "$(find "build/${pkg}/test_results" -type f -name '*.xml' 2>/dev/null | head -n 1)" ]; then + echo "::error::shard '${{ matrix.shard }}' selected ${pkg} but it produced no test results - the name may have rotted" + exit 1 + fi + done + fi + - name: Generate coverage report run: | # --parallel: this single lcov --capture was measured at >99% of the @@ -496,25 +624,118 @@ jobs: fi lcov --list coverage.info + + # No completeness gate here: a shard only ever exercises its own packages, so + # every shard's tracefile is legitimately partial. The gate runs on the + # merged report in coverage-merge below, which is the first place the + # question "did every package reach the report" can be asked. + - name: Upload this shard's tracefile + uses: actions/upload-artifact@v4 + with: + name: coverage-tracefile-${{ matrix.shard }} + path: coverage.info + # Long enough to answer a coverage question raised after the weekend: + # merging again needs these, and at one day a Monday question could not be + # answered from artifacts at all. + retention-days: 7 + + # The shards each measure their own packages, so no single one of them can + # answer "does the report still describe the whole workspace". This job is the + # first place that question exists, so the completeness gate, the HTML report + # and the Codecov upload all live here rather than being repeated four times + # against four partial answers. + coverage-merge: + name: coverage (merge) + # always(), so a failed shard does not take the merged report, the + # completeness gate and the Codecov upload down with it. Losing the + # diagnostic exactly when a shard broke is the wrong way round; the job + # reports what it merged and then fails on the shard's behalf. + if: always() && github.event_name == 'push' && github.ref == 'refs/heads/main' + needs: coverage + runs-on: ubuntu-latest + container: + image: ubuntu:noble + timeout-minutes: 30 + defaults: + run: + shell: bash + + steps: + - name: Install tools + run: | + apt-get update + # gpg and curl are both required by the codecov action's dependency + # check, which fails the step rather than skipping the upload. + apt-get install -y git lcov gpg curl + + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Download every shard's tracefile + uses: actions/download-artifact@v4 + with: + pattern: coverage-tracefile-* + path: tracefiles + # A shard that failed uploaded nothing; merge what exists rather than + # failing here, so the report still describes the shards that ran. + merge-multiple: false + + - name: Merge the tracefiles + run: | + # lcov -a sums execution counts for matching file and test names, which + # is exactly right here: the shards measure disjoint packages, and any + # file two of them both touched should carry the sum of both. + mapfile -t files < <(find tracefiles -name coverage.info | sort) + if [ "${#files[@]}" -eq 0 ]; then + echo "::error::no shard tracefiles were downloaded - the shards produced nothing to merge" + exit 1 + fi + echo "merging ${#files[@]} tracefiles" + args=() + for f in "${files[@]}"; do args+=(-a "$f"); done + # `empty` only: a shard may legitimately measure little. `corrupt` was also + # here and had to go - a truncated tracefile would have been merged with + # its bad records skipped and no non-zero exit, so every package would + # still be present, the gate would pass, and Codecov would quietly get a + # lower number. + lcov "${args[@]}" --output-file coverage.info --ignore-errors empty + lcov --list coverage.info genhtml coverage.info --output-directory coverage_html --ignore-errors source # A package that never gets --coverage emits no .gcda and drops out of the # report entirely - out of the numerator and the denominator both, so the - # percentage silently stops describing the workspace. Assert the report - # still covers every package the source tree says holds production C++. + # percentage silently stops describing the workspace. Assert the merged + # report still covers every package the source tree says holds production + # C++. Sharding makes this stricter, not weaker: a shard that silently + # tested nothing now shows up as a package missing from the merge. + # always(), like the merge above: when a shard fails this gate reports a + # missing package, and without this the HTML upload and the step that names + # the real cause would both be skipped - leaving "package X missing" as the + # only visible message for "shard Y failed". - name: Verify every C++ package reached the coverage report + if: always() run: | ./scripts/check_coverage_packages.sh coverage.info \ --skip ros2_medkit_opcua - name: Upload coverage HTML report as artifact + if: always() uses: actions/upload-artifact@v4 with: name: coverage-report path: coverage_html/ + # Last, so everything above is recorded first. The merged report is still + # worth having when a shard broke - it just does not describe the whole + # workspace, and saying so is this job's business. + - name: Fail if any shard did not succeed + if: always() && needs.coverage.result != 'success' + run: | + echo "::error::a coverage shard did not succeed (matrix result: ${{ needs.coverage.result }}). The merged report above covers only the shards that finished, so its percentage is a floor, not the workspace figure." + exit 1 + - name: Upload coverage to Codecov - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + if: needs.coverage.result == 'success' uses: codecov/codecov-action@v7 with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e52773219..3c098fa78 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,6 +89,14 @@ is the tally of the run you just started and not of everything tested since the rather than matching a name. Run it on its own with `./scripts/drop_stale_results.sh` if you invoke `colcon test` directly. +Every run also reclaims shared memory left behind by DDS participants that were killed +rather than shut down. The aggregation suites kill peers on purpose, and nothing gives +those segments back: about 0.65 MB each, and a container's `/dev/shm` is 64 MB by default, +so they accumulate across runs until something unrelated fails for lack of space. The +reclaiming is `fastdds shm clean`, the vendor's own tool, which removes a segment only when +no process holds its lock. Run it on its own with `./scripts/sweep_shm.sh`; it refuses +while ROS processes are alive, and `--force` overrides that. + #### Pre-commit and Pre-push Hooks ```bash @@ -174,6 +182,11 @@ genhtml coverage.info --output-directory coverage_html --ignore-errors source ./scripts/check_coverage_packages.sh coverage.info --skip ros2_medkit_opcua ``` +In CI this gate runs in the `coverage (merge)` job rather than alongside the tests: the +coverage run is sharded by package, so each shard's tracefile is legitimately partial and +the merged report is the first place the question "did every package reach the report" +can be asked. + Open `coverage_html/index.html` in your browser. `ros2_medkit_opcua` is skipped because it pulls `open62541pp` over the network diff --git a/scripts/ci_test_completeness.sh b/scripts/ci_test_completeness.sh new file mode 100755 index 000000000..b4e46fa03 --- /dev/null +++ b/scripts/ci_test_completeness.sh @@ -0,0 +1,105 @@ +#!/usr/bin/env bash +# Say plainly when a test step ran out of time, instead of letting it read as a +# test failure. +# +# A step killed by its timeout is marked FAILED by the runner, not cancelled, and +# the in-flight test leaves no result file. `colcon test-result` then reports that +# as one erroring test named `.xunit.missing_result`: a test that never +# failed, among thousands that never ran. On its own that is indistinguishable +# from a single flake, which is how a budget overrun gets filed as a test problem +# while every package queued behind it disappears without trace. +# +# The step's own log is not readable from inside the job, so the overrun is +# detected by the clock instead: the caller records a start timestamp before the +# test step and passes the step's cap here. Elapsed at or above the cap means the +# runner killed it. +# +# What this deliberately does NOT do is count registered tests against result +# files. Some tests legitimately write none - `test_dds_domain_allocation` is +# registered by ros2_medkit_cmake in every package and produces no xunit - so +# that comparison reports missing results on a run where everything passed. +# +# Usage: ci_test_completeness.sh +set -euo pipefail + +START_EPOCH="${1:-}" +CAP_MINUTES="${2:-}" + +usage() { + echo "usage: ci_test_completeness.sh " >&2 + echo " both are positive integers; the start is seconds since the epoch, as" >&2 + echo " written by date +%s before the step being measured" >&2 +} + +# Validated rather than trusted: a start value from a different clock, or a cap +# of zero, otherwise turns this from a diagnostic into a source of false +# verdicts - a monotonic start reads as an enormous overrun, and a zero cap +# divides by zero while still printing an overrun. +if [[ ! "${START_EPOCH}" =~ ^[0-9]+$ || ! "${CAP_MINUTES}" =~ ^[0-9]+$ ]]; then + usage + exit 2 +fi +if [[ "${START_EPOCH}" -eq 0 || "${CAP_MINUTES}" -eq 0 ]]; then + usage + exit 2 +fi + +now=$(date +%s) +if [[ "${START_EPOCH}" -gt "${now}" ]]; then + echo "::error::the recorded start (${START_EPOCH}) is in the future - the clock moved, so no margin can be computed" + exit 2 +fi + +elapsed=$(( now - START_EPOCH )) +cap_seconds=$(( CAP_MINUTES * 60 )) + +empty=0 +found=0 +while IFS= read -r xml; do + found=$((found + 1)) + if [[ ! -s "${xml}" ]]; then + echo "::error::empty result file: ${xml}" + empty=$((empty + 1)) + fi +done < <(find build -path '*/test_results/*' -name '*.xml' 2>/dev/null) + +# `missing_result` is not a filename. ament writes it as the name attribute of a +# INSIDE the ordinary xunit file, before running the command, so the +# file exists and is not empty. Measured: across five build trees, zero files are +# named that, while one tree carried two genuine missing results in its XML. +# `|| true` inside the braces, not after the pipe: grep exits 1 when it matches +# nothing, which is the healthy case, and under `set -e` with `pipefail` that +# killed this script before it printed anything at all - so a clean run reported +# nothing and failed the step. +missing=$( { grep -l 'missing_result' -r build --include='*.xml' 2>/dev/null || true; } | wc -l) + +printf 'test step: %ds elapsed of a %ds cap (%d%%); results: %d written, %d empty, %d missing_result\n' \ + "${elapsed}" "${cap_seconds}" "$(( elapsed * 100 / cap_seconds ))" "${found}" "${empty}" "${missing}" + +status=0 + +if [[ "${found}" -eq 0 ]]; then + echo "::error::no test result files at all - the run never reached the tests" + status=1 +fi + +if [[ "${empty}" -gt 0 ]]; then + status=1 +fi + +# The runner kills the step AT its cap and this check runs afterwards, so an +# overrun is simply elapsed at or past the cap. An earlier version subtracted a +# fixed minute, which made every cap below five minutes report an overrun on a +# run that had barely started. +if [[ "${elapsed}" -ge "${cap_seconds}" ]]; then + echo "::error::the test step reached its ${CAP_MINUTES} minute cap. Any missing_result above belongs to the test that was interrupted, and the packages queued behind it did not run at all. This is a budget overrun, not a test verdict." + status=1 +elif [[ "${elapsed}" -ge $(( cap_seconds * 8 / 10 )) ]]; then + echo "::warning::the test step used $(( elapsed * 100 / cap_seconds ))% of its ${CAP_MINUTES} minute cap - it will start being killed before anyone decides to raise it" +fi + +if [[ "${missing}" -gt 0 && "${elapsed}" -lt "${cap_seconds}" ]]; then + echo "::warning::${missing} test(s) produced no result file without the step running out of time - investigate the test, not the budget" +fi + +exit "${status}" diff --git a/scripts/sweep_shm.sh b/scripts/sweep_shm.sh new file mode 100755 index 000000000..7a8800be7 --- /dev/null +++ b/scripts/sweep_shm.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# Reclaim shared-memory segments left behind by DDS participants that were killed +# rather than shut down. +# +# A participant that exits cleanly frees its segments; one that dies on SIGKILL +# does not, and nothing reclaims them afterwards. Measured on Fast DDS: about +# 0.65 MB per participant, and 125 killed participants filled 63 MB of a 64 MB +# /dev/shm. Test runs that kill nodes on purpose - the aggregation suites do - +# accumulate this across every run on a developer machine, and the failure it +# eventually produces looks like anything but a full tmpfs. +# +# The reclaiming is done by `fastdds shm clean`, which is the vendor's own tool +# and ships with every distro we support. It decides a segment is stale by taking +# an exclusive non-blocking flock on the segment's lock file; the kernel drops +# that lock when a process dies, SIGKILL included, so a segment is removed only +# when nothing holds it. Each distro's copy matches its own file naming +# (fastrtps_* on 2.x, fastdds_* on 3.x), so the tool from the sourced +# distribution is the one to call - there is nothing for us to match on. +# +# Refuses to run while ROS processes are alive: the flock test protects live +# segments, but a sweep in the middle of a test run is still a lie about what the +# run measured. +set -euo pipefail + +usage() { + echo "usage: sweep_shm.sh [--force]" >&2 + echo " --force sweep even if ROS processes are running" >&2 +} + +FORCE=0 +case "${1:-}" in + --force) FORCE=1 ;; + "") ;; + *) usage; exit 2 ;; +esac + +shm_bytes() { + # One value or nothing. An earlier form piped du into cut with `|| echo 0`, + # which under pipefail could emit cut's partial output AND the fallback, so the + # arithmetic below got a two-line operand and died with a bad math expression. + local out + if ! out=$(du -sb /dev/shm 2>/dev/null); then + echo unknown + return + fi + printf '%s' "${out}" | head -n 1 | cut -f1 +} + +if [[ "${FORCE}" -eq 0 ]]; then + # Match on the command line rather than the process name: colcon runs as + # python3, and a pattern that also matches this script would match itself. + # demo_ covers the launch fixtures the documented demo workflow starts, which + # an earlier pattern missed entirely - a developer running only demo nodes got + # a sweep in the middle of live work. + # + # Our own process and our caller are excluded, because the caller's command + # line is often a match by itself: `./scripts/test.sh test_demo_lifecycle` + # contains demo_, so every single-test preset refused to sweep and the only + # symptom was a puzzling message. + if pgrep -f '[g]ateway_node|[f]ault_manager_node|[d]emo_|[c]test|[l]aunch_test|[c]olcon test' 2>/dev/null | + grep -qvE "^($$|${PPID})$"; then + echo "sweep_shm: ROS processes are running - refusing to sweep (use --force if you mean it)" >&2 + exit 1 + fi +fi + +before="$(shm_bytes)" + +if command -v fastdds >/dev/null 2>&1; then + fastdds shm clean || echo "sweep_shm: 'fastdds shm clean' reported a problem, continuing" >&2 +else + echo "sweep_shm: no 'fastdds' on PATH - source a ROS distribution first" >&2 + exit 1 +fi + +after="$(shm_bytes)" + +if [[ "${before}" == unknown || "${after}" == unknown ]]; then + echo "sweep_shm: cleaned, but /dev/shm could not be measured (du declined)" >&2 + exit 0 +fi + +printf 'sweep_shm: /dev/shm %s -> %s bytes (reclaimed %s)\n' "${before}" "${after}" "$((before - after))" diff --git a/scripts/test.sh b/scripts/test.sh index 7e75f93ff..488a428ec 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -57,6 +57,12 @@ COMMON_ARGS=(--event-handlers console_direct+ --parallel-workers "$(nproc)" --re # being able to ask for on its own. The reasoning lives there. "$HERE/drop_stale_results.sh" build +# Reclaim shared memory stranded by participants that were killed rather than shut +# down - the aggregation suites kill peers on purpose, and nothing gives those +# segments back. Tolerated when it declines: it refuses to sweep while ROS +# processes are alive, and that refusal must not stop a test run. +"$HERE/sweep_shm.sh" || true + # Run tests, capture exit code so we always show results even on failure. set +e case "$PRESET" in diff --git a/src/ros2_medkit_fault_reporter/test/test_domain_launch.test.py b/src/ros2_medkit_fault_reporter/test/test_domain_launch.test.py index 6ba9fb642..1cef4214a 100644 --- a/src/ros2_medkit_fault_reporter/test/test_domain_launch.test.py +++ b/src/ros2_medkit_fault_reporter/test/test_domain_launch.test.py @@ -43,8 +43,17 @@ TOPIC = '/medkit_launch_domain_probe' +# The child answers the shutdown signal instead of dying from it, so its exit +# status is a fact the test can pin rather than a range to tolerate. +# +# What this does NOT buy, despite the obvious guess: it strands no shared memory +# either way. Measured on jazzy, /dev/shm bytes before and after - a bare +# `rclpy.spin` killed by SIGINT returns to baseline exactly, because CPython +# finalises the interpreter before re-raising, and the participant is destroyed +# on the way out. Only SIGKILL strands segments (+0.65 MB, measured), and no +# handler in this child can affect that. CHILD = ( - 'import os, sys, rclpy;' + 'import os, signal, sys, rclpy;' 'from rclpy.node import Node;' 'from std_msgs.msg import String;' "print('CHILD_ROS_DOMAIN_ID=' + str(os.environ.get('ROS_DOMAIN_ID')), flush=True);" @@ -53,7 +62,18 @@ f"pub = node.create_publisher(String, '{TOPIC}', 10);" "msg = String(data='from-the-child');" 'timer = node.create_timer(0.1, lambda: pub.publish(msg));' - 'rclpy.spin(node)' + # launch escalates to SIGTERM when SIGINT is not answered in time, and + # Python's default action for that one is to die on the spot. Routing it + # through the same handler SIGINT uses keeps both paths ending in the + # shutdown below. + 'signal.signal(signal.SIGTERM, signal.default_int_handler)\n' + 'try:\n' + ' rclpy.spin(node)\n' + 'except KeyboardInterrupt:\n' + ' pass\n' + 'finally:\n' + ' node.destroy_node()\n' + ' rclpy.try_shutdown()\n' ) @@ -111,5 +131,11 @@ def test_the_child_node_is_reachable_on_that_domain(self): class TestChildShutdown(unittest.TestCase): def test_the_child_was_stopped(self, proc_info, child): - # SIGINT/SIGTERM during launch shutdown, not a crash. - self.assertIn(proc_info[child].returncode, (0, -2, -15, 130, 143)) + # Zero, not merely "not a crash". A fixture that dies from its shutdown + # signal reports the same status whether it shut down or was cut off + # mid-flight, so the old range accepted both. Zero distinguishes them. + self.assertEqual( + proc_info[child].returncode, + 0, + 'the child did not shut down cleanly, so its DDS participant leaked', + ) diff --git a/src/ros2_medkit_integration_tests/CMakeLists.txt b/src/ros2_medkit_integration_tests/CMakeLists.txt index 168664b7d..dab635448 100644 --- a/src/ros2_medkit_integration_tests/CMakeLists.txt +++ b/src/ros2_medkit_integration_tests/CMakeLists.txt @@ -104,6 +104,7 @@ target_include_directories(demo_param_beacon_node PRIVATE ${_demo_include_dir}) medkit_target_dependencies(demo_param_beacon_node rclcpp) add_executable(managed_lifecycle demo_nodes/managed_lifecycle_node.cpp) +target_include_directories(managed_lifecycle PRIVATE ${_demo_include_dir}) medkit_target_dependencies(managed_lifecycle rclcpp rclcpp_lifecycle) # Proof fixture for GRAPH_NODE_UNREADABLE (see the file doc): a plain rclcpp::Node that @@ -206,6 +207,15 @@ if(BUILD_TESTING) set_tests_properties(test_test_utils_constants PROPERTIES LABELS "unit") medkit_test_needs_no_domain(test_test_utils_constants) + # The crash reporter every demo binary installs. Death tests, because the + # behaviour under test only exists in a process that is being killed - a + # handler that is merely installed proves nothing about what a crash leaves + # behind. ROS-free: it creates no node, only signals. + find_package(ament_cmake_gmock REQUIRED) + medkit_add_gmock(test_crash_backtrace test/test_crash_backtrace.cpp) + target_include_directories(test_crash_backtrace PRIVATE ${_demo_include_dir}) + medkit_test_needs_no_domain(test_crash_backtrace) + # Configures every launch-test-registering package this file can reach for # real, twice each (ordinary workspace conditions and system-package # conditions), and compares what each run actually registered. Lives here @@ -439,6 +449,10 @@ if(BUILD_TESTING) endif() endforeach() endif() + + # gtest/gmock vendor sources are built as subdirectory targets and inherit + # this package's promoted warnings, which they do not compile clean under. + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_integration_tests/README.md b/src/ros2_medkit_integration_tests/README.md index 65cd04722..104326cf2 100644 --- a/src/ros2_medkit_integration_tests/README.md +++ b/src/ros2_medkit_integration_tests/README.md @@ -78,6 +78,32 @@ ros2 launch ros2_medkit_integration_tests demo_nodes.launch.py | `long_calibration` | `/powertrain/engine` | Action | Fibonacci-based long-running action | | `dual_calibration` | `/testrig/dual` | Services + Actions | `left/calibrate` and `right/calibrate`, `left/sweep` and `right/sweep` - one provider carrying each operation short name twice, so the ROS path is the only id that separates the copies | +### Crash reports + +Every demo binary installs a fatal-signal handler before anything else runs. +Most get it from `run_demo_node()` in +`include/ros2_medkit_integration_tests/demo_node_main.hpp`; the two that cannot +use that helper - `unresponsive_param_node` and `managed_lifecycle_node`, which +own their shutdown sequence - call `install_crash_backtrace()` themselves. A node killed by `SIGSEGV`, +`SIGBUS` or `SIGABRT` writes its stack to stderr, so launch_testing captures it +with the rest of the process output: + +``` +MEDKIT-CRASH signal=SIGSEGV +/path/to/demo_brake_pressure_sensor(+0x129cb) [0x5603199349cb] +/opt/ros//lib/libfastdds.so.3(...) [0x...] +MEDKIT-CRASH end +``` + +Grep a CI log for `MEDKIT-CRASH` to find one. Frames from our own binaries come +back as `binary(+0xOFFSET)` because the release build has no exported symbols; +resolve them with `addr2line -e `. The process still dies from +the original signal, so the exit status a test asserts on is unchanged. + +This exists because a node that dies during startup otherwise leaves nothing at +all: no output, and no core file, since a container cannot set the host's +`core_pattern`. + ## Writing New Tests ### Feature Test Template diff --git a/src/ros2_medkit_integration_tests/demo_nodes/managed_lifecycle_node.cpp b/src/ros2_medkit_integration_tests/demo_nodes/managed_lifecycle_node.cpp index 41b35d705..c27aa8069 100644 --- a/src/ros2_medkit_integration_tests/demo_nodes/managed_lifecycle_node.cpp +++ b/src/ros2_medkit_integration_tests/demo_nodes/managed_lifecycle_node.cpp @@ -19,6 +19,8 @@ #include #include +#include "ros2_medkit_integration_tests/crash_backtrace.hpp" + using CallbackReturn = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn; // A managed lifecycle node for status integration tests. It DEFAULTS to staying @@ -62,6 +64,10 @@ class ManagedLifecycleNode : public rclcpp_lifecycle::LifecycleNode { }; int main(int argc, char ** argv) { + // This node does not go through run_demo_node(), so it installs the + // crash reporter itself - otherwise it would be the one demo binary that + // still dies without leaving a stack behind. + ros2_medkit_integration_tests::install_crash_backtrace(); // Wrap the body so no exception escapes main (bugprone-exception-escape): the // run_demo_node helper used by the other demo nodes only accepts an // rclcpp::Node, so a LifecycleNode needs its own main. diff --git a/src/ros2_medkit_integration_tests/demo_nodes/unresponsive_param_node.cpp b/src/ros2_medkit_integration_tests/demo_nodes/unresponsive_param_node.cpp index 74e12590c..b1da03e7c 100644 --- a/src/ros2_medkit_integration_tests/demo_nodes/unresponsive_param_node.cpp +++ b/src/ros2_medkit_integration_tests/demo_nodes/unresponsive_param_node.cpp @@ -44,6 +44,8 @@ #include +#include "ros2_medkit_integration_tests/crash_backtrace.hpp" + class UnresponsiveParamNode : public rclcpp::Node { public: UnresponsiveParamNode() : Node("unresponsive_param_node", rclcpp::NodeOptions().start_parameter_services(false)) { @@ -114,6 +116,10 @@ class UnresponsiveParamNode : public rclcpp::Node { }; int main(int argc, char ** argv) { + // This node does not go through run_demo_node(), so it installs the + // crash reporter itself - otherwise it would be the one demo binary that + // still dies without leaving a stack behind. + ros2_medkit_integration_tests::install_crash_backtrace(); // Deliberately NOT ros2_medkit_integration_tests::run_demo_node(): that // helper calls rclcpp::shutdown() only after executor.spin() has already // returned, which works for every other demo node but would deadlock this diff --git a/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/crash_backtrace.hpp b/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/crash_backtrace.hpp new file mode 100644 index 000000000..4ccaf1d32 --- /dev/null +++ b/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/crash_backtrace.hpp @@ -0,0 +1,210 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include +#include + +#include +#include + +// A sanitizer owns the fatal signals and reports far more than a bare stack, so +// this file stands down when one is present. GCC and Clang announce that +// differently, and __has_feature has to be probed in its own directive because +// GCC expands it eagerly inside a compound #if. +#if defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_THREAD__) +#define MEDKIT_CRASH_BACKTRACE_SANITIZED 1 +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) || __has_feature(thread_sanitizer) +#define MEDKIT_CRASH_BACKTRACE_SANITIZED 1 +#endif +#endif + +namespace ros2_medkit_integration_tests { + +/// Marker that prefixes every frame, so one grep separates a crash report from +/// the surrounding node output. +inline constexpr const char kCrashMarker[] = "MEDKIT-CRASH"; + +namespace detail { + +inline constexpr int kMaxFrames = 64; + +/// Storage for the frame addresses. A signal handler must not allocate, so the +/// buffer is reserved up front and reused. +inline void ** crash_frame_buffer() { + static void * frames[kMaxFrames]; + return frames; +} + +/// Lets exactly one thread write a report. +/// +/// sa_mask does NOT do this: it blocks signals for the thread running the +/// handler only. Thread A faulting on SIGSEGV while thread B takes SIGBUS gives +/// two concurrent handlers writing one static frame buffer and interleaving +/// their output. The second entrant says so in one line and gets out of the way. +inline std::atomic_flag & crash_reporting_flag() { + static std::atomic_flag reporting = ATOMIC_FLAG_INIT; + return reporting; +} + +/// A stack of its own for the handler to run on. +/// +/// The commonest silent SIGSEGV is a stack overflow, and that is exactly the one +/// a handler on the ordinary stack cannot report: it needs stack to run, there is +/// none, so it faults again and - the disposition already reset - the process +/// dies having written nothing. Measured before this existed: a null dereference +/// produced 452 bytes and two markers, a stack overflow produced zero of both. +/// +/// SIGSTKSZ is not a compile-time constant on current glibc, so the size is +/// fixed here; 64 KB is far more than backtrace_symbols_fd needs. +inline constexpr std::size_t kAltStackBytes = 64 * 1024; + +inline char * crash_alt_stack() { + static char storage[kAltStackBytes]; + return storage; +} + +/// Writes a string literal. The length comes from the type, so nothing in the +/// handler calls strlen - which POSIX does not list as async-signal-safe. +template +inline void write_literal(const char (&text)[N]) { + const ssize_t written = ::write(STDERR_FILENO, text, N - 1); + static_cast(written); +} + +/// Writes the marker, the signal number and the backtrace, then returns so the +/// default disposition installed by SA_RESETHAND can terminate the process with +/// the original signal. That keeps the exit status the test harness sees +/// unchanged: a segfault still reports as -11, now with frames attached. +inline void crash_handler(int signum) { + // Unwinding from a signal handler is not async-signal-safe: backtrace() can + // need the loader or an allocator lock, and if the faulting thread already + // held one it deadlocks here - in exactly the startup paths this exists to + // diagnose. alarm() is async-signal-safe and SIGALRM's disposition is the + // default, so a wedged unwinder becomes a dead process after five seconds + // instead of a test that hangs to its timeout with nothing to read. The exit + // status is then SIGALRM rather than the original signal, which is the + // trade: a wrong status beats no output and no status at all. + ::alarm(5); + + if (crash_reporting_flag().test_and_set()) { + write_literal(kCrashMarker); + write_literal(" second thread also faulted; its frames are not reported\n"); + ::raise(signum); + return; + } + + write_literal(kCrashMarker); + switch (signum) { + case SIGSEGV: + write_literal(" signal=SIGSEGV\n"); + break; + case SIGBUS: + write_literal(" signal=SIGBUS\n"); + break; + case SIGABRT: + write_literal(" signal=SIGABRT\n"); + break; + default: + write_literal(" signal=other\n"); + break; + } + + void ** frames = crash_frame_buffer(); + const int depth = ::backtrace(frames, kMaxFrames); + // backtrace_symbols_fd writes through the raw fd and allocates nothing, which + // is what makes it usable here; backtrace_symbols would call malloc. + ::backtrace_symbols_fd(frames, depth, STDERR_FILENO); + write_literal(kCrashMarker); + write_literal(" end\n"); + + // Explicit, not implicit. Returning re-executes the faulting instruction, + // which re-raises a genuine SIGSEGV or SIGBUS - but a signal delivered by + // kill(), raise() or pthread_kill() has no instruction to retry, so the + // process would simply resume and then be killed by the alarm above five + // seconds later, reporting SIGALRM instead of the signal that happened. + // Measured before this line existed: raise(SIGSEGV) gave exit status 142, not + // 139. SA_RESETHAND has already restored the default disposition, so this + // terminates the process with the right status in both cases. + ::raise(signum); +} + +} // namespace detail + +/// Report the stack on a fatal signal instead of dying silently. +/// +/// A process killed by SIGSEGV during startup leaves nothing behind: no output, +/// no core file (a container cannot set the host's core_pattern), and the test +/// harness reports only the exit status. Without frames there is no way to tell +/// a defect in this repository from one below it, in rclcpp, rmw or the DDS +/// implementation. +/// +/// Call this before any other work in main(). The first `backtrace` call +/// resolves the unwinder's lazy relocations, which does allocate - so it is made +/// here, at install time, and never inside the handler. +inline void install_crash_backtrace() { +#ifdef MEDKIT_CRASH_BACKTRACE_SANITIZED + return; +#else + void ** frames = detail::crash_frame_buffer(); + static_cast(::backtrace(frames, 1)); + + // sigaltstack is per-thread, and this runs on the thread that calls it - the + // main thread, where a node is constructed and where the startup crashes this + // exists for happen. A stack overflow on a DDS thread is still silent; that is + // a real limit, not an oversight. + stack_t alt{}; + alt.ss_sp = detail::crash_alt_stack(); + alt.ss_size = detail::kAltStackBytes; + alt.ss_flags = 0; + ::sigaltstack(&alt, nullptr); + + struct sigaction action {}; + action.sa_handler = &detail::crash_handler; + // Blocks the other fatal signals for the duration of the handler ON THIS + // THREAD. That is all sa_mask can do - the cross-thread case is what + // crash_reporting_flag() above is for. + ::sigemptyset(&action.sa_mask); + ::sigaddset(&action.sa_mask, SIGSEGV); + ::sigaddset(&action.sa_mask, SIGBUS); + ::sigaddset(&action.sa_mask, SIGABRT); + // SA_RESETHAND restores the default disposition before the handler runs, so + // returning from it re-raises the signal and the process dies the way it + // would have without us. + // SA_ONSTACK puts the handler on the alternate stack above, which is what lets + // it run at all when the ordinary stack is the thing that overflowed. + // static_cast, because these constants are unsigned and sa_flags is int: + // -Wsign-conversion, which this workspace builds with, reports the change of + // value once per translation unit that includes this header. + action.sa_flags = static_cast(SA_RESETHAND | SA_ONSTACK); + + ::sigaction(SIGSEGV, &action, nullptr); + ::sigaction(SIGBUS, &action, nullptr); + ::sigaction(SIGABRT, &action, nullptr); +#endif +} + +/// True when install_crash_backtrace() installs handlers in this build. +inline constexpr bool crash_backtrace_is_active() { +#ifdef MEDKIT_CRASH_BACKTRACE_SANITIZED + return false; +#else + return true; +#endif +} + +} // namespace ros2_medkit_integration_tests diff --git a/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/demo_node_main.hpp b/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/demo_node_main.hpp index dcbe2cc0c..db2992c96 100644 --- a/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/demo_node_main.hpp +++ b/src/ros2_medkit_integration_tests/include/ros2_medkit_integration_tests/demo_node_main.hpp @@ -21,6 +21,8 @@ #include +#include "ros2_medkit_integration_tests/crash_backtrace.hpp" + namespace ros2_medkit_integration_tests { /** @@ -67,6 +69,11 @@ namespace ros2_medkit_integration_tests { * freely. */ inline int run_demo_node(int argc, char ** argv, const std::function()> & node_factory) { + // Installed first: a fatal signal anywhere below, including inside the DDS + // stack while the participant is being created, then names its frames instead + // of leaving an exit status and no output. + install_crash_backtrace(); + sigset_t mask; sigset_t old; sigemptyset(&mask); diff --git a/src/ros2_medkit_integration_tests/package.xml b/src/ros2_medkit_integration_tests/package.xml index 6abce50ed..3ae4e8ae0 100644 --- a/src/ros2_medkit_integration_tests/package.xml +++ b/src/ros2_medkit_integration_tests/package.xml @@ -27,6 +27,7 @@ ament_cmake_pytest + ament_cmake_gmock python3-pytest launch_testing_ament_cmake launch_testing diff --git a/src/ros2_medkit_integration_tests/test/test_crash_backtrace.cpp b/src/ros2_medkit_integration_tests/test/test_crash_backtrace.cpp new file mode 100644 index 000000000..e1836841a --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/test_crash_backtrace.cpp @@ -0,0 +1,170 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include + +#include + +#include +#include + +#include "ros2_medkit_integration_tests/crash_backtrace.hpp" + +using ros2_medkit_integration_tests::install_crash_backtrace; + +namespace { + +// Recursion the optimiser cannot flatten into a loop. Measured across -O0 to +// -O3: without the escaping address GCC turns this into a loop at -O2 and the +// process spins forever instead of overflowing, which made an earlier version of +// the test below hang rather than fail. +volatile char * stack_probe_sink = nullptr; + +// The recursion is the point of this function, so the compiler's warning about +// it is noise. +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Winfinite-recursion" +#endif +__attribute__((noinline)) int recurse_until_the_stack_runs_out(int x) { + volatile char pad[4096]; + pad[0] = static_cast(x); + stack_probe_sink = pad; + // The barrier is what makes this recurse rather than loop. `noinline` and an + // escaping address are not enough inside a translation unit where the + // function has internal linkage: measured, GCC still turned it into a loop at + // -O2 and the process spun instead of overflowing. + asm volatile("" : : "r"(pad) : "memory"); + return recurse_until_the_stack_runs_out(x + pad[0]) + 1; +} +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic pop +#endif + +/// Faults on an unmapped address that is NOT null. +/// +/// Null would be simpler, and was what this did first, but UBSan diagnoses a +/// store through a null pointer before the store happens - so under the ASan job, +/// which builds `asan,ubsan`, no signal was ever raised and four of the cases +/// below passed their "no marker" assertion because nothing crashed rather than +/// because the handler stood down. A non-null unmapped address gives UBSan +/// nothing to object to and still segfaults. +/// +/// Both volatile qualifiers are load-bearing and were picked by measuring: a +/// store through a plain `T * volatile` is deleted at -O2 as an erroneous path +/// and the process then does not crash at all. +/// A regex matching a backtrace frame that belongs to this test binary. +/// +/// Anchored on the basename, not on /proc/self/exe: glibc prints argv[0] for the +/// main executable rather than its resolved path, so a symlinked workspace or a +/// plain `./test_crash_backtrace` would not match a realpath and the test would +/// fail on a perfectly good report. +/// +/// The object and offset pair is what addr2line turns back into a location. +/// glibc's spacing between the offset and the address differs by release +/// (resolute writes ") [0x", noble writes ")[0x"), and a frame carries a symbol +/// name before the "+" whenever the binary exports its symbols, which CMake does +/// by default - so both of those are matched loosely. +const char * own_binary_frame_pattern() { + return R"(test_crash_backtrace\([^)]*\+0x[0-9a-fA-F]+\) ?\[0x[0-9a-fA-F]+\])"; +} + +void crash_by_unmapped_write() { + install_crash_backtrace(); + volatile int * volatile target = reinterpret_cast(0x1000); + *target = 1; +} + +} // namespace + +// The claim under test is not "a handler is installed" but "a process that dies +// on a fatal signal leaves frames behind". Only killing a process proves it, +// which is what a death test does: the body runs in a forked child and the +// assertion matches that child's stderr. +// +// These cases cover the configuration this package is built in, and only that +// one. The header also stands down under a sanitizer so it cannot replace a +// sanitizer's own report - but sanitizers are opt-in per package here, through +// include(ROS2MedkitSanitizers), and this package does not opt in. So +// __SANITIZE_ADDRESS__ is never defined for this translation unit, the +// stand-down never engages, and a test written for it would assert against a +// branch that cannot be reached. The stand-down stays in the header for the day +// this package does opt in; it is not claimed to be tested. +TEST(CrashBacktrace, SegvIsReported) { + ASSERT_DEATH(crash_by_unmapped_write(), "MEDKIT-CRASH signal=SIGSEGV"); +} + +TEST(CrashBacktrace, SegvReportsResolvableFrames) { + // The marker alone would be satisfied by an empty stack. What makes a + // report useful is a frame carrying an object and an offset, because that + // pair is what addr2line turns back into a location. Asserting on a symbol + // NAME would pin the wrong thing: a release build without -rdynamic reports + // offsets for this binary's own frames, and the frames worth reading here + // belong to libraries below us anyway. + // + // Two things are matched loosely on purpose. glibc's spacing between the + // offset and the address differs by release (resolute writes ") [0x", + // noble writes ")[0x"), and a frame carries a symbol name before the "+" + // whenever the binary exports its symbols - a link flag away, and not what + // this test is about. + // Anchored on THIS binary's own path, read at runtime rather than written + // down: a bare offset pattern is satisfied by any frame, and libc's frames + // alone would pass it while saying nothing about whether our own frames came + // back resolvable. The path is not a name pin - it is whatever the test was + // built as. + ASSERT_DEATH(crash_by_unmapped_write(), own_binary_frame_pattern()); +} + +// A stack overflow is the commonest silent SIGSEGV, and it is the one a handler +// on the ordinary stack cannot report - it needs stack to run and there is none. +// This case shipped broken until it was measured: the null dereference above +// produced 452 bytes, and an overflow produced zero. +TEST(CrashBacktrace, SegvOnAnOverflowedStackIsStillReported) { + const auto overflow_the_stack = [] { + install_crash_backtrace(); + static_cast(recurse_until_the_stack_runs_out(1)); + }; + ASSERT_DEATH(overflow_the_stack(), "MEDKIT-CRASH signal=SIGSEGV"); +} + +TEST(CrashBacktrace, AbortIsReportedToo) { + ASSERT_DEATH( + { + install_crash_backtrace(); + std::abort(); + }, + "MEDKIT-CRASH signal=SIGABRT"); +} + +// Exit status is what ctest and launch_testing report, and a handler that +// swallowed the signal would turn a crash into a clean exit and hide it. +TEST(CrashBacktrace, ProcessStillDiesFromTheOriginalSignal) { + EXPECT_EXIT(crash_by_unmapped_write(), ::testing::KilledBySignal(SIGSEGV), "MEDKIT-CRASH end"); +} + +// The crash helper installs the handler itself, so a test that merely calls +// install twice beforehand proves nothing - the third install inside the child +// would carry it. The double install has to happen INSIDE the dying process. +TEST(CrashBacktrace, InstallingTwiceIsHarmless) { + const auto crash_after_installing_twice = [] { + install_crash_backtrace(); + install_crash_backtrace(); + volatile int * volatile target = nullptr; + *target = 1; + }; + EXPECT_EXIT(crash_after_installing_twice(), ::testing::KilledBySignal(SIGSEGV), "MEDKIT-CRASH end"); +}