Add Vst2RawChunk, a raw effGetChunk/effSetChunk bypass around JUCE's VSTPluginInstance::usesChunks() gate, plus --loadBankChunk/--saveBankChunk/ --loadPatchChunk/--savePatchChunk CLI flags and a new test_chunk_roundtrip regression test asserting the saved file is genuinely zlib/XML (0x78 header, >1000 bytes), not a per-parameter fallback. Building and using this bypass disproved an earlier documented finding: the compiled VST2 wrapper does set effFlagsProgramChunks and usesChunks() is genuinely true, so the previous "chunk-advertisement gap" theory in TODO.md and tests/README.md was wrong. Corrected in both places. The real cause of old bundled .fxp sample files having no effect remains open and is now narrowed to a JaySynth-side XML/schema parsing question, not a host/plugin capability mismatch. Also adds a PluginHost-method-to-VST2-wiring-to-test coverage table in tests/README.md, and documents the remaining "exercised but not asserted" gaps (channel/param counts, DSP correctness) in TODO.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011dhtwRLARk4eiPngcQykLJ
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). Goes through JUCE's generic AudioProcessor::get/ setStateInformation/get/setCurrentProgramStateInformation - see the
corrected-finding note below for what that path actually does.
bank_chunk_roundtrip — same round-trip, but via effGetChunk/
effSetChunk called directly on the raw AEffect* (Vst2RawChunk.h/
--saveBankChunk/--loadBankChunk), bypassing JUCE's host-side wrapper
entirely. Also positively confirms the real chunk path was used (not just
that a parameter round-trips): checks the saved file starts with a zlib
stream header (0x78) and is far larger than a raw 140-parameter float
array could be (140×4 = 560 bytes) - a plain per-parameter fallback
wouldn't look anything like this.
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. See the
corrected-finding note below for why this only checks "doesn't crash," not
"changes the patch."
PluginHost method coverage table
Every method on PluginHost (src/PluginHost.h), what it actually wires
to on the VST2 side for a hosted plugin, and which test(s) exercise it.
"Exercised" means the call happens during that test; "asserted" means the
test actually checks something about its result, not just that it ran
without crashing.
PluginHost method |
VST2 wiring | Covering test(s) |
|---|---|---|
load() |
effOpen (via createPluginInstance), then inside prepareToPlay: effSetSampleRate, effSetBlockSize, effMainsChanged(1), effStartProcess |
Every test - exercised, not specifically asserted beyond "didn't return false" |
releaseResources() |
effMainsChanged(0)/effStopProcess (VSTPluginInstance::releaseResources) |
Every test calls it before exit (main.cpp) - exercised only, no assertion |
getNumInputChannels() / getNumOutputChannels() |
Reads cached AEffect::numInputs/numOutputs, populated at effOpen |
Every test (printed in the "loaded ... (N in/N out...)" banner) - exercised, not asserted against an expected value for any specific plugin |
getNumParameters() |
Reads cached AEffect::numParams |
Every test (same load banner; also implicit in every --printParams use) - exercised, not asserted |
getParameter(index) |
AEffect::getParameter callback |
patch_save_load_roundtrip, bank_save_load_roundtrip, bank_chunk_roundtrip, patch_chunk_roundtrip (all via --printParams) - asserted (value checked within tolerance) |
setParameter(index, value) |
AEffect::setParameter callback |
Same four round-trip tests (via --param) - asserted indirectly (the value it sets is what gets read back) |
process() - audio only |
processReplacing/processDoubleReplacing |
Every test - asserted only for NaN/Inf absence, not correctness of the DSP output itself |
process() - MIDI note on/off |
effProcessEvents (VstMidiEvent) |
oversized_blocksize, nrpn_out_of_range, nrpn_in_range, stability_sweep (all use --note) - exercised (audio is produced), not asserted beyond non-silence/no-NaN |
process() - MIDI CC/NRPN |
effProcessEvents (VstMidiEvent, controller sub-type) |
nrpn_out_of_range, nrpn_in_range - exercised and the specific regression (bounds check) is asserted via clean completion |
getCurrentProgramStateInformation() |
saveToFXBFile(isFXB=false) → effGetChunk(index=1) (confirmed chunk-based, see below) |
patch_save_load_roundtrip - asserted |
setCurrentProgramStateInformation() |
loadFromFXBFile → effSetChunk(index=1) |
patch_save_load_roundtrip - asserted; also used by stability_sweep via --patch, but not asserted there - see the corrected-finding note, those particular files don't actually change anything |
getStateInformation() |
saveToFXBFile(isFXB=true) → effGetChunk(index=0) |
bank_save_load_roundtrip - asserted |
setStateInformation() |
loadFromFXBFile → effSetChunk(index=0) |
bank_save_load_roundtrip - asserted |
getInstance() |
n/a (plain accessor, returns the raw AudioPluginInstance*) |
Used internally by every Vst2RawChunk-based test to reach the raw AEffect* |
(bypass, not on PluginHost) Vst2RawChunk::getChunk/setChunk |
effGetChunk/effSetChunk called directly on the raw AEffect*, bypassing usesChunks() and AudioProcessor entirely |
bank_chunk_roundtrip, patch_chunk_roundtrip - asserted, plus the zlib-header/size check that positively confirms the real chunk path ran |
Known gaps: load()/releaseResources()/getNumInputChannels()/
getNumOutputChannels()/getNumParameters() are exercised by literally
every test run but never asserted against a specific expected value for
a specific plugin (e.g. nothing currently pins "JaySynth must report
exactly 0 in / 2 out / 140 parameters" - a change to any of those would go
unnoticed by this suite unless it also broke something else). Same for
process()'s actual DSP correctness - the suite checks "ran without
crashing/NaN," never "the audio sounds right." Both are reasonable
follow-ups noted in tools/testhost/TODO.md.
Corrected finding: it's not a chunk-advertisement gap after all
An earlier version of this suite (and tools/testhost/TODO.md) claimed
that JaySynth's compiled VST2 wrapper never sets effFlagsProgramChunks,
so JUCE's host-side usesChunks() would see false and silently skip the
chunk mechanism - offered as the reason bundled sample .fxp files have no
effect when loaded via --patch. That diagnosis was wrong. It's
disproved directly: --saveBank/--savePatch (the exact same generic
AudioProcessor::getStateInformation call) produce real FBCh/FPCh-
magic files containing a genuine zlib-compressed XML blob (confirmed via
hex dump), not a per-parameter fallback - meaning usesChunks() is
genuinely true. To be certain, Vst2RawChunk was added specifically to
rule out any remaining JUCE-side gating: it calls effGetChunk/
effSetChunk straight on the raw AEffect*, and feeding a bundled sample
file's inner chunk bytes through it this way still has zero effect.
test_chunk_roundtrip above proves this exact mechanism round-trips
correctly for files this build produces.
So the real, still-open question is narrower: why do these specific
(10+ year old) external sample files fail to apply, given the mechanism
that's supposed to load them demonstrably works? Most likely a compression
or XML-schema shape from an older JaySynth/JUCE combination that
JaySynthAudioProcessor::setCurrentProgramStateInformation/
getXmlFromBinary no longer parses - not investigated further here (would
mean decompressing and diffing one of these files' XML against what this
build produces). Not one of the tracked repo-root TODO.md findings.
Tracked 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.