Fix SIGTRAP: bounds-check chart.serum()/antigen() + skip stale serum-coverage maps - #36
Open
drserajames wants to merge 2 commits into
Open
Fix SIGTRAP: bounds-check chart.serum()/antigen() + skip stale serum-coverage maps#36drserajames wants to merge 2 commits into
drserajames wants to merge 2 commits into
Conversation
The report's serum_coverage_export crashed with SIGTRAP (exit 133) on
h3-hi-guinea-pig-cdc. The crash was NOT in the native map renderer
(all sc-* maps render cleanly); it was in serum_coverage_webpage.
serum_coverage/ accumulates map PDFs across runs and is never cleaned.
This lab's dir held maps sc-000..sc-045 from an earlier run of a larger
chart, but the current chart has only 9 sera. serum_coverage_webpage
globs every *.pdf and serum_title() does chart.serum(int(stem)) with no
bounds check, so it called chart.serum(45) on a 9-serum chart. The
pybind chart.serum()/chart.antigen() accessors did an unchecked
sera()[serum_index{n}], which under libc++ FAST hardening (build-py314)
calls __builtin_trap() -> SIGTRAP (and silently reads OOB memory on the
non-hardened build-arm64). This also made the crash look non-renderer
and heap-nondeterministic: rendering completes, the trap fires only in
the post-render webpage step.
Two fixes:
- cc/py/chart-v3.cc: bounds-check chart.serum()/chart.antigen(); raise a
catchable pybind11::index_error (Python IndexError) on out-of-range
instead of trapping / reading OOB. Matches the existing SeqdbSelected
accessor pattern.
- py/ae/report/commander.py: serum_coverage_webpage skips maps whose
serum index is not present in the current chart (stale/foreign files),
so the page reflects only this chart's sera and never indexes out of
range. Labs whose output matches the chart are unaffected.
Verified: h3-hi-guinea-pig-cdc serum_coverage_export now exits 0 (was
133), page lists exactly its 9 sera, skips the 74 stale maps; a normal
lab (vidrl, 26 sera) is unchanged (26 pages, nothing skipped);
chart.serum(45) now raises IndexError.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
serum-coverage/ PDFs accumulated across runs — a shrunk chart or a serum_selector (e.g. h1-cdc's single-serum export) left maps for sera the run won't regenerate (h1-cdc had 274 stale; h3-hi-guinea-pig-cdc 74). Beyond clutter, those stale files name sera outside the current chart, which is what drove chart.serum() out of range. Make serum_coverage_export authoritative for the dir: before rendering, remove any existing sc-*.pdf not in this run's job set, plus the regenerated gridage/index/coverage web artifacts. Unrelated files are left untouched. The webpage stale-skip + the chart.serum()/antigen() bounds-check remain as defence-in-depth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix SIGTRAP: bounds-check chart.serum()/antigen() + skip stale serum-coverage maps
A full report run crashed with SIGTRAP (exit 133) during one lab's
serum_coverage_export. Root cause was a latent out-of-bounds access, not the renderer:serum-coverage/dirs accumulate map PDFs across runs and are never cleaned.h3-hi-guinea-pig-cdcheldsc-000…045from an earlier 46-serum chart, but the current chart has 9 sera.serum_coverage_webpageglobs every*.pdfandserum_titlecallschart.serum(int(stem.split("-")[1]))with no bounds check →chart.serum(45)on a 9-serum chart.chart.serum()/chart.antigen()did an uncheckedsera()[serum_index{n}]. Underbuild-py314's-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST, the OOB access calls__builtin_trap()→ SIGTRAP. On the non-hardenedbuild-arm64it silently reads OOB memory — a real bug either way.Fix (two complementary parts)
cc/py/chart-v3.cc— bounds-checkchart.serum()/chart.antigen()and raise a catchablepybind11::index_error(PythonIndexError) on out-of-range, matching the existingSeqdbSelectedaccessor pattern. Closes the UB for every caller.py/ae/report/commander.py—serum_coverage_webpageskips maps whose serum index isn't present in the current chart (graceful degrade — stale/foreign files name sera that don't exist here), so the page reflects only this chart's sera and never indexes out of range.Proof
h3-hi-guinea-pig-cdc serum_coverage_export: exit 0 (was 133), page lists exactly its 9 sera, skips the stale maps.h3-hi-guinea-pig-vidrl(26 sera, no stale files) — exit 0, 26 pages, nothing skipped (unchanged for a normal lab).chart.serum(45)→IndexError: serum index 45 out of range (chart has 9 sera).Other labs'
serum-coverage/dirs carry the same stale accumulation, so they had the same latent crash — this covers them all. WHO-data gate clean; 2 code files, no data.Update: clean the serum-coverage/ dir (not just skip)
serum_coverage_exportis now authoritative for its output dir: before rendering it removes any existingsc-*.pdfnot in the current run's job set, plus the regeneratedgridage-*.json/index-*.html/coverage.html. This physically clears the stale accumulation (h1-cdc's 274, h3-hi-guinea-pig-cdc's 74) rather than only skipping it at webpage time — a shrunk chart or aserum_selector(e.g. h1-cdc's single-serum export) no longer leaves orphaned maps. Unrelated files are untouched. The webpage skip + the C++ bounds-check remain as defence-in-depth. Verified: a simulated 46→9-serum dir collapses to exactly the 18 current maps, unrelated files preserved.