Files
JaySynth/tools/testhost/tests
jensandClaude Sonnet 5 f54689f071 Add regression suite testing JaySynth's TODO.md findings via testhost
tools/testhost/tests/run_tests.sh dynamically exercises the tracked
hardening findings using testhost itself - no GUI, sub-second wall
time:

- oversized_blocksize (Critical #5): --blockSize 16384 under a
  timeout, catching either the overflow or the deadlock the original
  bug could cause.
- nrpn_out_of_range/nrpn_in_range (Critical #1): JSON scenarios
  sending the 5-message MIDI NRPN CC sequence, once targeting the
  max 14-bit ID (16383) and once a legitimate in-range ID (500), so
  a regression that made the bounds check too aggressive would also
  show up.
- patch_save_load_roundtrip / bank_save_load_roundtrip: save a
  distinctive continuous parameter value, reload in a fresh process,
  check it round-trips within tolerance - covers the
  setCurrentProgramStateInformation no-op bug and the
  patchDecodeXml/patchImportXml fixes.
- stability_sweep: loads every bundled .fxp under extras/sounds/ and
  renders a held note through each, as a broad crash/NaN net.

Added --printParams to main.cpp (prints every parameter's index and
value) to make the round-trip tests possible - this was also already
a noted nice-to-have in testhost/TODO.md.

While building the oversized_blocksize test, found and fixed (on the
hardening branch, commit d713ca5) a second, previously-unknown bug:
VCF_CalcCoeff_LPF/_HPF/_BPF advance the coefficient buffer pointer
once per (sample, filter-section) pair, but the buffer was only ever
allocated for bufsize samples, not bufsize*sections - a 4th-order
filter overflowed it for any block between 4097 and 8192 samples.
Found via bisecting the crashing block size then confirming with an
ASan build; see tests/README.md's debugging-notes section for exactly
how, including the ASan symbolizer hang encountered along the way and
the workaround.

Also discovered (documented in tools/testhost/TODO.md's Investigate
section, not fixed - out of scope): JaySynth's compiled VST2 wrapper
never advertises chunk support to the host, so real .fxp/.fxb sample
files have no effect when loaded through the generic AudioProcessor
state API. The round-trip tests work around this by using testhost's
own save output as the fixture rather than an external sample file.

All 10 checks pass against the current hardening-branch build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011dhtwRLARk4eiPngcQykLJ
2026-07-27 20:06:44 +02:00
..

Regression suite for JaySynth's hardening findings

run_tests.sh uses testhost itself to dynamically exercise the findings tracked in the repo-root TODO.md (the "hardening" review) — no GUI, no manual clicking, runs in well under a second of wall-clock time.

Running

MAKE_HOME=/path/to/submodule/make ./run_tests.sh [path/to/JaySynth.so]

Builds testhost first if it isn't already built. Defaults to build/linux/release/JaySynth.so if no path is given. Exit code is 0 if everything passed, 1 otherwise (CI-friendly). On failure, per-test logs and WAVs are kept in a mktemp -d workdir printed at the end; on success that workdir is deleted.

What each test actually covers

oversized_blocksize (Critical #5) — renders through --blockSize 16384 (double SYNTH_MAX_BUFSIZE). Before the fix this either overflowed processBlock's fixed 8192-sample stack buffer or deadlocked the render worker threads; timeout is what actually catches the deadlock case, since a hung process would otherwise just sit here forever. This test is also what caught a second, previously-unknown bug: VCF_CalcCoeff_LPF/_HPF/_BPF advance their coefficient-buffer pointer once per (sample, filter-section) pair, but the buffer was only ever allocated for bufsize samples, not bufsize × sections — a 4th-order filter (2 sections) overflowed it for any block between 4097 and 8192 samples, a range this fix's own "chunk to ≤8192" logic doesn't protect against on its own. Found via AddressSanitizer after this test kept crashing at blockSize values below 8192; fixed in src/synth/vcf.c (see TODO.md's Critical section). If this test ever regresses again, don't assume it's the same bug — bisect the block size first (see the debugging notes at the bottom of this file for how that investigation went).

nrpn_out_of_range/nrpn_in_range (Critical #1) — sends the standard 5-message MIDI NRPN CC sequence (99/98/6/38/98) via a JSON scenario, once targeting the maximum possible 14-bit NRPN ID (16383) and once a legitimate in-range ID (500). Both must complete cleanly; the in-range case exists so a regression that made the bounds check too aggressive (rejecting valid IDs) would also be visible as a behavioural difference. Caveat: "doesn't crash" is a necessary but not fully sufficient check here. The original bug was an out-of-bounds write into last_midiCC_info[], a plain array embedded inside the larger heap-allocated JaySynth object — not a separate heap allocation — so an overflow of a few hundred entries lands in other member fields of the same object rather than past its heap allocation. That means it can silently corrupt adjacent state without necessarily crashing in a short test run, and it's why the ID is pushed to the actual maximum (16383) rather than something more modest: to maximize the chance that, absent the fix, the write would land far enough away to hit unrelated memory and actually crash. The real guarantee is the bounds-check line itself (src/plug/JaySynth.cpp, handleController), not this test's silence.

patch_save_load_roundtrip / bank_save_load_roundtrip — covers two related fixes: (a) setCurrentProgramStateInformation used to pass patchImportXml an XML node one level too deep, making the VST2 "copy plugin state" path a complete no-op; (b) the patchDecodeXml/ patchDecodeXml_legacy deduplication and patchImportXml's legacy-branch restructuring. Sets a continuous parameter (index 0 = SYNTH_PARAM_VOLUME) to a distinctive value, saves, reloads in a fresh process, and checks the value reads back within a small tolerance (not exact equality — the slider/internal-value scaling curve, toParam/toSlider, isn't bit-exact across a round trip). Uses testhost's own save→load cycle as the fixture, not an externally-provided .fxp/.fxb — see the chunk-format note below for why.

stability_sweep — loads every bundled .fxp under extras/sounds/ and renders a held note through each, checking for crashes/hangs/NaN. A broad regression net, not a targeted test for any one finding.

Two things discovered while building this suite that are worth knowing

The VCF coefficient-buffer bug above (already fixed, see TODO.md).

JaySynth's compiled VST2 wrapper never advertises chunk support to the host, so loading the bundled sample .fxp files through testhost's generic --patch (i.e. through AudioProcessor::setCurrentProgramState- Information, exactly what any real VST2 host uses) has zero effect — confirmed by diffing --printParams output with and without --patch. Those sample files are legitimately in VST2's opaque-chunk ("FPCh") format (verified via hex dump: correct CcnK/FPCh/Jsy1 header), but JUCE 3.1.1's plugin-side VST wrapper (sdk/juce/JUCE-3.1.1/modules/ juce_audio_plugin_client/VST/juce_VST_Wrapper.cpp, compiled into JaySynth.so) never sets the VST2 effFlagsProgramChunks flag on the AEffect struct, so JUCE's own host-side VSTPluginInstance::usesChunks() sees false and setChunkData() silently no-ops — for both loading and saving. This is not one of the tracked TODO.md findings and this suite doesn't attempt to fix it (it would mean patching JUCE's own VST wrapper, a materially bigger and riskier change). It's why stability_sweep only checks "doesn't crash," not "changes the patch," and why the round-trip tests use testhost's own save output as the fixture rather than a real sample file — testhost's save path has the same usesChunks()-is-false behaviour, so save and load are at least consistent with each other, even though neither talks to the external .fxp files' actual chunk data. Tracked as a discovered issue in tools/testhost/TODO.md's "Investigate" section.

Debugging notes: how the VCF bug was actually found

oversized_blocksize initially failed with free(): invalid next size (normal) - glibc's heap-corruption detector, tripped during process teardown, not at the point of the actual overflow. Bisecting --blockSize found the real threshold was 4096/4097, not 8192 as expected. Rather than guess further, an ASan build of just src/synth + src/plug (env-var override: CFLAGS/CXXFLAGS="-fsanitize=address -fno-omit-frame-pointer -g -O1" LDFLAGS="-fsanitize=address", CONFIG=asan to keep it in its own build directory) run via LD_PRELOAD=$(clang -print-file-name=libclang_rt. asan-x86_64.so) pinpointed it exactly. Two gotchas worth remembering if this needs doing again: (1) ASAN_OPTIONS=symbolize=0 is necessary in this environment — the default external llvm-symbolizer fork hangs indefinitely rather than resolving addresses, so use symbolize=0 and resolve the reported JaySynth.so+0x... offsets with addr2line -e JaySynth.so <offset> (or just nm/objdump if addr2line needs exact offsets adjusted for the binary's load bias) instead of trusting ASan's own backtrace; (2) rebuilding the ASan variant needs a real git worktree avoided here — sdk/juce/JUCE-3.1.1 is vendored as a zip+patch pair extracted on demand, not a plain tracked directory, so a fresh worktree doesn't have it. Simpler to git stash uncommitted work and build the ASan variant directly on the branch with the fix under investigation.