Repository navigation
Conversation
Every WaveNet buffer holds the maximum buffer size given to Reset(), and process() only asserted that a call fits. With asserts compiled out, a larger call wrote past the end of the condition array, every layer's conv, mixin and activation buffers and the conv ring buffers. AddressSanitizer reports a heap-buffer-overflow in _set_condition_array for each generic WaveNet in example_models/. The A2 fast path grows its buffers for the same call instead, so a host that under-reserves works with A2 models and corrupts the heap with A1 models (see sdatkinson#303). Process such a call as consecutive calls of at most the maximum buffer size. That keeps the model's state and allocates nothing: the per-channel chunk pointers are sized at construction. Growing the buffers would also avoid the overflow, but it allocates on the audio thread and SetMaxBufferSize() reinitialises every history, which changed the output of wavenet_a1_standard.nam by up to 0.57 after the call. The layer-array loop moves unchanged into _process_layer_arrays() so that process() does not become more complex. The new test renders four WaveNets (plain, A2-era layer features, a condition DSP, and two in/two out) in 100-frame calls with a 32-frame maximum buffer size, and requires the output to equal that of the host making those calls in 32/32/32/4-frame pieces, with no allocation. Without the fix it stops at the assert in WaveNet::process(), or, with -DNDEBUG under AddressSanitizer, at the heap-buffer-overflow.
squatchlr
force-pushed
the
submit/oversized
branch
from
September 24, 2026 22:45
fcaa2e7 to
90427a3
Compare
…k head - _process_in_chunks advances by each chunk's length, so the offset never passes num_frames and cannot overflow int for calls within one buffer of INT_MAX frames. No test can make such a call; this is by inspection. - The oversized-call test adds a two-channel WaveNet with a post-stack head, so the chunked path through the head is checked against consecutive calls and for allocations too.
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
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.
Related: sdatkinson#303
Summary
WaveNet::process()sizes every buffer for themaxBufferSizegiven toReset(), and only asserts that a call fits (model.cpp:827). In a releasebuild, a larger call writes past the condition array (
:820), every layer'sconv, mixin and activation buffers, and the conv ring buffers whose lookback is
shorter than the excess. ASan reports a heap-buffer-overflow for every generic
WaveNet in
example_models/.This PR processes such a call as consecutive calls of at most
mMaxBufferSizeframes. The chunk pointers are sized in the constructor, so nothing allocates,
and state carries over between chunks. The recursive call is qualified, so a
subclass override isn't re-entered. The layer-array loop moves unchanged into
_process_layer_arrays(), which keepsprocess()at its current complexity.flowchart LR accTitle: WaveNet::process with the fix accDescr: A call larger than the maximum buffer size is split into consecutive calls of at most that size; each call then runs the unchanged path. n_call["WaveNet::process(input, output, n)"] --> n_fits{"n > mMaxBufferSize?"} n_fits -- no --> n_run["condition array, layer arrays, head<br/>(unchanged)"] n_fits -- yes --> n_split["_process_in_chunks()<br/>calls of at most mMaxBufferSize<br/>no allocation"] n_split -- each chunk --> n_callSplit rather than grow or throw. Growing would allocate on the audio thread
and clear every history: after one such call,
wavenet_a1_standard.nam'soutput was off by up to 0.57.
SequentialModelthrows instead. If you'd preferthat here, it's a few lines and the test adapts. We chose splitting because a
throw on the audio thread usually takes the host down.
sdatkinson#303. A host that sends more frames than it reserved gets a working A2 model
(the A2 fast path grows) and a corrupted heap with the next A1 model. That
matches the report, but it depends on the reporter's host, so this doesn't
claim to close it. Details are on the issue.
Tests
tools/test/test_wavenet/test_oversized_blocks.cppcovers four WaveNets:wavenet.nam,wavenet_a2_feature_test.nam,wavenet_condition_dsp.namand a2-in/2-out model built in the test. Each gets a 32-frame reservation and
100-frame calls, and the output must equal 32/32/32/4-frame calls exactly, with
no allocation. On
mainit fails atAssertion failed: (num_frames <= mMaxBufferSize), function process, file model.cpp, line 827.With
-DNDEBUGunder ASan it fails at the overflow:AddressSanitizer on main
Verification
CI's steps pass on AppleClang 21 and on Linux clang 18.1.3 with both libstdc++
and libc++.
run_testspasses on GCC 14.2 (Pi 5), with-DNAM_ENABLE_A2_FAST=OFF, and under ASan+UBSan. Within the reservation,output is bit-identical to
main.Details
NAM_USE_INLINE_GEMM,each running
run_tests,benchmodel(wavenet.nam,lstm.nam) andrender.run_testswith-DNAM_ENABLE_A2_FAST=OFF, and with-O1 -DNDEBUG -fsanitize=address,undefined.configurations: libstdc++ and
-DNAM_USE_LIBCXX_LINUX=ON, each plain andwith inline GEMM.
run_tests, plain and with inlineGEMM.
mainat-O3: 36 byte-compared renders. They coverevery example model through
render(the slimmables at both sizes), and theA2 fast and generic paths at 64-, 7- and 61-frame blocks on a 128-frame
reservation.
-DNDEBUG: every example WaveNet, andthe A1 model from [BUG] Loading certain A1 models after A2 models can cause audio crackling / dropouts sdatkinson/NeuralAmpModelerCore#303, are clean.
revision with the same check measured within noise on an idle Pi 5 (n=5).
This commit was not timed separately.
Compatibility
Calls within the reservation take the same path, with bit-identical output.
Larger calls were undefined behaviour in release builds; they are now chunked.
Not covered
ConvNethas the same unchecked contract (RingBuffer::Writeonlyasserts). Its own small PR follows.
Linear, including.wavIRs ([FEATURE] Instantiate (linear) models from .wav files sdatkinson/NeuralAmpModelerCore#337), grows its input buffer on an oversizedcall. That is memory-safe, but it allocates and drops the history. It is
left alone here.