Consolidate benchmark shell scripts into shared lib + merged sweep script - #2
Merged
Conversation
Copilotcreated this pull request from a session on behalf of daedalusJuly 15, 2026 23:35 View session
daedalus
marked this pull request as ready for review
July 15, 2026 23:35
Reviewer's GuideRefactors the benchmarking shell scripts around a shared helper library, merges the two sweep scripts into a single, deduplicated benchmark sweep, fixes SHM cleanup and retry/coverage handling, aligns all benchmarks and docs with the removal of the deprecated --meta-elo flag, and updates Elo reporting to the new internal API. Sequence diagram for run_with_retry coverage and SHM handlingsequenceDiagram
participant BenchScript as bench.sh_or_bench_sweep.sh
participant BenchLib as bench_common_sh
participant Fuzzer as fuzzer_tool
participant SHM as OS_SHM
BenchScript->>BenchLib: run_with_retry(log, fuzz args)
loop attempts up to BENCH_MAX_RETRIES
BenchLib->>Fuzzer: python -m fuzzer_tool fuzz ...
Fuzzer-->>BenchLib: write log
alt [log is empty]
BenchLib-->>BenchScript: "Run produced no log output" message
else [log has content]
BenchLib->>BenchLib: check_coverage(log, label)
BenchLib->>BenchLib: verify_shm(log, label)
BenchLib->>SHM: shmat(shm_id)
SHM-->>BenchLib: bitmap bytes
alt [bitmap has non-zero bytes]
BenchLib-->>BenchScript: success
Note over BenchLib,BenchScript: break loop
else [coverage-blind run]
BenchLib-->>BenchScript: "Coverage did not attach" message
end
end
BenchLib->>BenchLib: cleanup_shm()
BenchLib->>SHM: ipcs/ipcrm on orphaned segments
end
BenchLib-->>BenchScript: failure after BENCH_MAX_RETRIES
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
bench_common.sh,verify_shmusespython3whilerun_with_retryandrun_combousepython; consider standardizing on one interpreter (python3 -m fuzzer_tool) to avoid environment-dependent failures. run_with_retrytreats both "no log produced" and explicit coverage failures as the same retry path and always prints "Coverage did not attach"; you might want to branch the messaging so the final failure reason clearly distinguishes between startup crashes and SHM-attachment issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `bench_common.sh`, `verify_shm` uses `python3` while `run_with_retry` and `run_combo` use `python`; consider standardizing on one interpreter (`python3 -m fuzzer_tool`) to avoid environment-dependent failures.
-`run_with_retry` treats both "no log produced" and explicit coverage failures as the same retry path and always prints "Coverage did not attach"; you might want to branch the messaging so the final failure reason clearly distinguishes between startup crashes and SHM-attachment issues.
## Individual Comments### Comment 1
<locationpath="tools/lib/bench_common.sh"line_range="16" />
<code_context>
+# Capture matching SHM IDs into a variable first: under `set -o pipefail`,+# piping straight into `while read` would abort the script (via `set -e`)+# whenever grep finds no matches (the common case with no stale segments).+ shmids=$(ipcs -m 2>/dev/null | grep "$(whoami)" | awk '{print $2}' || true)
+ if [[ -n "$shmids" ]]; then
+ while read -r shmid; do
</code_context>
<issue_to_address>
**suggestion (bug_risk):** User matching in SHM cleanup can accidentally match other usernames that contain the current username as a substring.
`grep "$(whoami)"` matches any line where the username appears as a substring (e.g., `foo` also matches `foobar`), so it can select SHM segments owned by other users. To restrict matches to the owner field, use something like:
-`awk '$3 == "'"$(whoami)"'" {print $2}'`, or
-`grep -E "^[^ ]+ +[^ ]+ +$(whoami) " | awk '{print $2}'`
so that only segments owned by the current user are removed.
Suggested implementation:
``` local before shmids before=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l) # Capture matching SHM IDs into a variable first: under `set -o pipefail`, # piping straight into `while read` would abort the script (via `set -e`) # whenever the filter finds no matches (the common case with no stale segments). shmids=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user {print $2}')`````` local after after=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)```
</issue_to_address>
### Comment 2
<locationpath="tools/lib/bench_common.sh"line_range="123-129" />
<code_context>
+# Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries
+# on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup
+# between attempts.
+BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}"
+
+run_with_retry() {
</code_context>
<issue_to_address>
**suggestion:** Consider validating `BENCH_MAX_RETRIES` to ensure it is a positive integer before using it in numeric comparisons.
Because `BENCH_MAX_RETRIES` is now environment-configurable, non-numeric or zero/negative values (e.g. `BENCH_MAX_RETRIES=foo` or `0`) can make `[[ $attempt -le $BENCH_MAX_RETRIES ]]` fail or behave unpredictably under `set -euo pipefail`. Consider normalizing the value once (e.g. default to 3 when unset or not a positive integer) so the retry loop stays robust to misconfiguration.
```suggestion# ── Run with retry ────────────────────────────────────────────────────# Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries# on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup# between attempts.BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}"# Normalize BENCH_MAX_RETRIES to a positive integer; fall back to 3 on# invalid or non-positive values to keep the retry loop robust.if ! [[ "$BENCH_MAX_RETRIES" =~ ^[1-9][0-9]*$ ]]; then echo "[!] Invalid BENCH_MAX_RETRIES='$BENCH_MAX_RETRIES'; using default of 3" >&2 BENCH_MAX_RETRIES=3firun_with_retry() {```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Capture matching SHM IDs into a variable first: under `set -o pipefail`, | ||
| # piping straight into `while read` would abort the script (via `set -e`) | ||
| # whenever grep finds no matches (the common case with no stale segments). | ||
| shmids=$(ipcs -m 2>/dev/null | grep "$(whoami)" | awk '{print $2}' || true) |
There was a problem hiding this comment.
suggestion (bug_risk): User matching in SHM cleanup can accidentally match other usernames that contain the current username as a substring.
grep "$(whoami)" matches any line where the username appears as a substring (e.g., foo also matches foobar), so it can select SHM segments owned by other users. To restrict matches to the owner field, use something like:
awk '$3 == "'"$(whoami)"'" {print $2}', orgrep -E "^[^ ]+ +[^ ]+ +$(whoami) " | awk '{print $2}'
so that only segments owned by the current user are removed.
Suggested implementation:
local before shmids
before=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)
# Capture matching SHM IDs into a variable first: under `set -o pipefail`,
# piping straight into `while read` would abort the script (via `set -e`)
# whenever the filter finds no matches (the common case with no stale segments).
shmids=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user {print $2}')
local after
after=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)
Comment on lines
+123
to
+129
| # ── Run with retry ──────────────────────────────────────────────────── | ||
| # Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries | ||
| # on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup | ||
| # between attempts. | ||
| BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}" | ||
| run_with_retry() { |
There was a problem hiding this comment.
suggestion: Consider validating BENCH_MAX_RETRIES to ensure it is a positive integer before using it in numeric comparisons.
Because BENCH_MAX_RETRIES is now environment-configurable, non-numeric or zero/negative values (e.g. BENCH_MAX_RETRIES=foo or 0) can make [[ $attempt -le $BENCH_MAX_RETRIES ]] fail or behave unpredictably under set -euo pipefail. Consider normalizing the value once (e.g. default to 3 when unset or not a positive integer) so the retry loop stays robust to misconfiguration.
Suggested change
| # ── Run with retry ──────────────────────────────────────────────────── | |
| # Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries | |
| # on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup | |
| # between attempts. | |
| BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}" | |
| run_with_retry() { | |
| # ── Run with retry ──────────────────────────────────────────────────── | |
| # Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries | |
| # on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup | |
| # between attempts. | |
| BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}" | |
| # Normalize BENCH_MAX_RETRIES to a positive integer; fall back to 3 on | |
| # invalid or non-positive values to keep the retry loop robust. | |
| if! [[ "$BENCH_MAX_RETRIES"=~ ^[1-9][0-9]*$ ]];then | |
| echo"[!] Invalid BENCH_MAX_RETRIES='$BENCH_MAX_RETRIES'; using default of 3">&2 | |
| BENCH_MAX_RETRIES=3 | |
| fi | |
| run_with_retry() { |
There was a problem hiding this comment.
Pull request overview
This PR consolidates the benchmarking tooling by extracting duplicated shell helpers into a shared library, merges the second sweep script into the primary sweep, and aligns docs/reporting with the consolidation of the former --meta-elo behavior into --elo.
Changes:
- Added
tools/lib/bench_common.shand updatedtools/bench.sh/tools/bench_sweep.shto source shared SHM cleanup, coverage verification, and log-extraction helpers. - Merged
tools/bench_sweep2.shscenarios intotools/bench_sweep.shand removedtools/bench_sweep2.sh. - Updated reporting/docs to stop referencing removed meta-elo fields/flags and document the new benchmark layout.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/lib/bench_common.sh | New shared benchmark helper library (SHM cleanup, extraction, coverage verification, retry, sweep runner). |
| tools/bench.sh | Switched to shared helper library; removed --meta-elo usage; updated CI extraction formatting. |
| tools/bench_sweep.sh | Switched to shared helper library; merged/expanded sweep phases; removed --meta-elo combos; increased results output. |
| tools/bench_sweep2.sh | Removed (folded into tools/bench_sweep.sh). |
| src/fuzzer_tool/services/report.py | Fixed report gating to use _use_elo instead of removed _use_meta_elo. |
| README.md | Updated benchmark configuration docs and referenced shared helper library. |
| docs/TODO.md | Recorded benchmark consolidation and the report.py_use_meta_elo fix. |
| docs/compose/reports/fuzzer-optimization-journey.md | Updated example command to remove --meta-elo. |
| AGENTS.md | Updated tools tree and documented Elo behavior without separate --meta-elo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+73
to
+90
| local has_data | ||
| has_data=$(python3 -c " | ||
| import ctypes, ctypes.util | ||
| libc = ctypes.CDLL(ctypes.util.find_library('c') or 'libc.so.6', use_errno=True) | ||
| libc.shmat.restype = ctypes.c_void_p | ||
| ptr = libc.shmat($shm_id, None, 0) | ||
| if ptr is None or ptr == -1: | ||
| print('FAIL') | ||
| else: | ||
| size = 4096 # default map size | ||
| bitmap = (ctypes.c_uint8 * size).from_address(ptr) | ||
| non_zero = sum(1 for i in range(size) if bitmap[i] != 0) | ||
| libc.shmdt(ptr) | ||
| if non_zero > 0: | ||
| print(f'OK:{non_zero}') | ||
| else: | ||
| print('EMPTY') | ||
| " 2>/dev/null) |
| while [[ $attempt -le $BENCH_MAX_RETRIES ]]; do | ||
| echo "[*] Attempt $attempt/$BENCH_MAX_RETRIES..." | ||
| python -m fuzzer_tool "$@" 2>&1 | tee "$log" |
daedalus added a commit
that referenced
this pull request
Jul 16, 2026
Consolidate benchmark shell scripts into shared lib + merged sweep script
daedalus added a commit
that referenced
this pull request
Jul 17, 2026
ENTROPY_HISTORY_MAX=200, ENTROPY_HISTORY_TRIM=100, ENTROPY_WINDOW=4, ENTROPY_FLAT_THRESHOLD=0.001 All four findings verified: - #2: zlib.crc32 returns deterministic int for LSH bucket keys ✓ - #3: grammar repeat bounds already clamped with max(hi, lo) ✓ - #4: edge_tracker uses zlib.crc32, not builtin hash() ✓
daedalus added a commit
that referenced
this pull request
Aug 21, 2026
Bug report 2026-08-21, CRITICAL #2. cli/commands.py passed contextual, contextual_alpha, contextual_lambda and lineage_backtrack to run_parallel(), which accepted none of them and has no **kwargs catch-all. Every `--jobs > 1` run died before spawning a worker: TypeError: run_parallel() got an unexpected keyword argument 'contextual' Parallel fuzzing was unreachable from the CLI. Fuzzer.__init__ already accepts all four, so this is purely missing plumbing: add them to both the worker and run_parallel signatures and forward them, mirroring how `lineage` is already threaded through. fuzzer-tool fuzz <target> --jobs 2 -n 200 [worker-0] Done. execs=200 crashes=0 eps=482 corpus=2 [worker-1] Done. execs=200 crashes=0 eps=508 corpus=2
daedalus added a commit
that referenced
this pull request
Aug 21, 2026
Bug report 2026-08-21, CRITICAL #2. cli/commands.py passed contextual, contextual_alpha, contextual_lambda and lineage_backtrack to run_parallel(), which accepted none of them and has no **kwargs catch-all. Every `--jobs > 1` run died before spawning a worker: TypeError: run_parallel() got an unexpected keyword argument 'contextual' Parallel fuzzing was unreachable from the CLI. Fuzzer.__init__ already accepts all four, so this is purely missing plumbing: add them to both the worker and run_parallel signatures and forward them, mirroring how `lineage` is already threaded through. fuzzer-tool fuzz <target> --jobs 2 -n 200 [worker-0] Done. execs=200 crashes=0 eps=482 corpus=2 [worker-1] Done. execs=200 crashes=0 eps=508 corpus=2
daedalus pushed a commit
that referenced
this pull request
Aug 22, 2026
Annotates the bug report's status column, which had drifted badly: only #1 was marked FIXED, but #2 (6f7a866), #5 (c6fa0ce) and #7/#8/#9/#14/#21/#40 (3712812) had all been closed by earlier commits without the doc being updated. Anyone working the backlog was one step from re-fixing them. The learnings note covers the part worth keeping: that the obvious pytest timeout traded away a property the codebase depends on, and that both of the first two attempts to verify process-group cleanup produced a false FAIL -- a pgrep matching the harness's own command line, then a ps count including zombies that this container's PID 1 never reaps. When a cleanup assertion fails, suspect the observation before the code.
daedalus added a commit
that referenced
this pull request
Sep 4, 2026
…point A campaign against ffmpeg_read reported an ASAN heap-buffer-overflow in mov_read_header. The top two frames were ours: #0 __afl_put_hexbytes afl_shim.c:1313 #1 __afl_cmplog_bytes afl_shim.c:1419 #2 afl_cmp_strncmp afl_shim.c:1698 #3 strncmp #4 url_find_protocol Our instrumentation crashing, filed as a crash in the target. The length an interceptor passes is a semantic bound -- how many bytes the real call was permitted to look at -- not a readable one. strncmp stops at the first NUL or mismatch, so char *p = malloc(2); p[0] = 'h'; p[1] = 'x'; strncmp(p, "http:", 5); is legal C: two bytes are compared and byte 2 is never touched. The logger read all five. Fix it at __afl_cmplog_bytes rather than in strncmp. There are 18 direct call sites, each open-coding its own length with a different convention (n, n + 1, k, a hand-rolled CMPLOG_MAX_OPERAND clamp). strcmp was written carefully, the memmem site already carries a comment about this exact class of read, and strncmp and strncasecmp were the ones that were missed. One guard at the only place that dereferences the operands means the next interceptor cannot forget it. Two layers, covering different builds: - Page clamp, unconditional. A read that stays inside the page holding p cannot fault, because the caller just read from that page. This is the only guard a non-ASAN target gets, and it is where it matters most: a fault in the nosan .so is an in-process SIGSEGV with our frame on top and no redzone diagnostic to say whose it was. - __asan_region_is_poisoned when the weak symbol resolves, tightening the page bound to the exact object bound. NULL in a non-ASAN build, so that path costs one null check. Not a truncation to the first mismatch, which was the first thing I tried. It is provably safe and it guts the record: with buf all 'A', strncmp(buf, "PREFIX_", 7) mismatches at byte 0, so the literal never reaches the pool -- and the token is the thing redqueen injects. test_regression_cmplog_shim_merge catches exactly that, and clamping by readability instead keeps it green: before fix: CMP 41414141414141 5052454649585f -15 7 after fix: CMP 41414141414141 5052454649585f -15 7 (unchanged) overread: CMP 6878 6874 4 5 (was: 5 bytes, ASAN abort) Adds tests/test_regression_cmplog_strncmp_overread.py, which fails without the change. It covers strncmp, strncasecmp and a fully readable compare, so a fix that clamps too aggressively fails the third case. Two tracecmp_visibility failures in the suite pre-date this change; the same two fail on a clean tree.
daedalus added a commit
that referenced
this pull request
Sep 4, 2026
…point A campaign against ffmpeg_read reported an ASAN heap-buffer-overflow in mov_read_header. The top two frames were ours: #0 __afl_put_hexbytes afl_shim.c:1313 #1 __afl_cmplog_bytes afl_shim.c:1419 #2 afl_cmp_strncmp afl_shim.c:1698 #3 strncmp #4 url_find_protocol Our instrumentation crashing, filed as a crash in the target. The length an interceptor passes is a semantic bound -- how many bytes the real call was permitted to look at -- not a readable one. strncmp stops at the first NUL or mismatch, so char *p = malloc(2); p[0] = 'h'; p[1] = 'x'; strncmp(p, "http:", 5); is legal C: two bytes are compared and byte 2 is never touched. The logger read all five. Fix it at __afl_cmplog_bytes rather than in strncmp. There are 18 direct call sites, each open-coding its own length with a different convention (n, n + 1, k, a hand-rolled CMPLOG_MAX_OPERAND clamp). strcmp was written carefully, the memmem site already carries a comment about this exact class of read, and strncmp and strncasecmp were the ones that were missed. One guard at the only place that dereferences the operands means the next interceptor cannot forget it. Two layers, covering different builds: - Page clamp, unconditional. A read that stays inside the page holding p cannot fault, because the caller just read from that page. This is the only guard a non-ASAN target gets, and it is where it matters most: a fault in the nosan .so is an in-process SIGSEGV with our frame on top and no redzone diagnostic to say whose it was. - __asan_region_is_poisoned when the weak symbol resolves, tightening the page bound to the exact object bound. NULL in a non-ASAN build, so that path costs one null check. Not a truncation to the first mismatch, which was the first thing I tried. It is provably safe and it guts the record: with buf all 'A', strncmp(buf, "PREFIX_", 7) mismatches at byte 0, so the literal never reaches the pool -- and the token is the thing redqueen injects. test_regression_cmplog_shim_merge catches exactly that, and clamping by readability instead keeps it green: before fix: CMP 41414141414141 5052454649585f -15 7 after fix: CMP 41414141414141 5052454649585f -15 7 (unchanged) overread: CMP 6878 6874 4 5 (was: 5 bytes, ASAN abort) Adds tests/test_regression_cmplog_strncmp_overread.py, which fails without the change. It covers strncmp, strncasecmp and a fully readable compare, so a fix that clamps too aggressively fails the third case. Two tracecmp_visibility failures in the suite pre-date this change; the same two fail on a clean tree.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
tools/bench.sh,tools/bench_sweep.sh, andtools/bench_sweep2.shduplicated the same helper functions (cleanup_shm,extract,extract_ci,run_combo,verify_shm,check_coverage,run_with_retry) across ~650 lines, andbench_sweep2.shre-ran many combos already covered bybench_sweep.sh.Shared library
tools/lib/bench_common.sh, sourced by both remaining scriptsextract_cinow takes an explicit delimiter parameter (|for CSV rows," "for table display) instead of callers post-processing the outputMerged sweep script
tools/bench_sweep2.shintotools/bench_sweep.sh(8 phases, 71 combos)--meta-eloflag removal (s3/s4,gt1-gt3,f1-f3, etc.)f16-f18,t8,z1-z4variance-check phase)tools/bench_sweep2.shBug fix
cleanup_shmpipedgrepoutput directly intowhile read; underset -o pipefail+set -e, this aborted the entire script whenever there were no stale SHM segments to clean (the common case). Fixed by capturing grep output into a variable before the loop.run_with_retrynow distinguishes "no log produced" (target crashed before startup) from "coverage failed to attach" for clearer retry diagnosticsDocs
tools/bench.sh/tools/bench_sweep.sh/tools/lib/bench_common.shlayoutSummary by Sourcery
Consolidate benchmark shell scripts into a shared library, merge and expand the feature-sweep benchmark, and align scripts/docs with the updated Elo/meta-elo behavior.
Bug Fixes:
Enhancements: