From 54a67562dbe4a5811f43c086337f06d83e2e8030 Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Mon, 27 Jul 2026 18:19:56 +0200 Subject: [PATCH] Fix Memory-management issues from TODO.md - JaySynth's destructor now uses delete[] to match every new T[n] allocation (m_pVoices, pPer_voice_controls, pCurrNoteInfos, humanize_voice_param[i], ppHumanizedSliders[i], m_ppAudioThread, m_ppEventAudioThreadRdy), fixing undefined behavior from the mismatched scalar delete. - Found and fixed the same new[]/delete mismatch pattern via ScopedPointer in PluginProcessor.cpp: ScopedPointer always calls scalar delete (per JUCE's own doc comment "do not give it an array to hold!"), so ScopedPointer holding a new char[...] in setStateInformation/setCurrentProgramStateInformation had the same bug. Replaced both with HeapBlock, JUCE's array-owning, malloc/free-backed smart pointer. - Added a shared SynthCheckAlloc() helper (synth_defs.h/synth_debug.c) that aborts with a diagnostic instead of returning NULL, and wrapped all 24 malloc call sites across the C DSP core (env.c, lfo.c, vcf.c, vco.c, blit.c, wavetable.c, voice.c, param_scale.c). All are one-time init/bufsize-change calls, never in the per-block render path, so this adds no real-time-thread overhead. Verified with clean debug and release builds (no new warnings/errors) and a full link. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011dhtwRLARk4eiPngcQykLJ --- TODO.md | 6 +++--- src/plug/JaySynth.cpp | 18 +++++++++--------- src/plug/PluginProcessor.cpp | 5 ++--- src/synth/blit.c | 10 +++++----- src/synth/env.c | 2 +- src/synth/lfo.c | 6 +++--- src/synth/param_scale.c | 2 +- src/synth/synth_debug.c | 12 ++++++++++++ src/synth/synth_defs.h | 5 +++++ src/synth/vcf.c | 8 ++++---- src/synth/vco.c | 2 +- src/synth/voice.c | 24 ++++++++++++------------ src/synth/wavetable.c | 4 ++-- 13 files changed, 60 insertions(+), 44 deletions(-) diff --git a/TODO.md b/TODO.md index 13ef449..42ab2a2 100644 --- a/TODO.md +++ b/TODO.md @@ -18,10 +18,10 @@ Findings from a code-review pass over `src/plug` and `src/synth` (2026-07-27, br - [x] **~450KB–900KB of stack arrays allocated per voice per audio block.** `VoiceProcessDataV`'s 11 `SYNTH_MAX_BUFSIZE`-sized locals (`src/synth/voice.c`) are now persistent per-voice buffers (`pBuf_VCO_fmout`, `pBuf_vco_pwm`, etc., declared in `voice.h`), allocated once in `VoiceSetBufsize`/freed in `VoiceFree` — same lifecycle/pattern already used by `pBuf_Q` and by `vcf.c`/`env.c`/`lfo.c`. - [x] **`SynthDebug` used unbounded `vsprintf` into a fixed 1024-byte buffer.** `src/synth/synth_debug.c` now uses `vsnprintf` with the buffer size. (Left the blocking `puts()`/`OutputDebugString` I/O as-is — it's compiled out entirely unless `SYNTH_DEBUG` is defined, so release builds are unaffected, and a lock-free logging redesign felt like overreach for a debug-only path.) -## Memory management +## Memory management (fixed) -- [ ] **`new[]`/`delete` mismatches (UB) in `JaySynth`'s destructor** — `m_pVoices`, `pPer_voice_controls`, `pCurrNoteInfos`, `humanize_voice_param[i]`, `ppHumanizedSliders[i]`, `m_ppAudioThread`, `m_ppEventAudioThreadRdy` all allocated with `new T[n]` but freed with scalar `delete` (`src/plug/JaySynth.cpp:62/186, 77/184, 80/185, 100-101/173-174, 124-125/160,191`). Fix: change to `delete[]` at each site. -- [ ] **`malloc` is never null-checked** across the entire C DSP core (`env.c`, `lfo.c`, `vcf.c`, `vco.c`, `blit.c`, `wavetable.c`, `voice.c`, `param_scale.c`). Fix: check and fail gracefully (or `abort()` with a diagnostic) rather than crashing on first audio callback. +- [x] **`new[]`/`delete` mismatches (UB) in `JaySynth`'s destructor** — `m_pVoices`, `pPer_voice_controls`, `pCurrNoteInfos`, `humanize_voice_param[i]`, `ppHumanizedSliders[i]`, `m_ppAudioThread`, `m_ppEventAudioThreadRdy` (`src/plug/JaySynth.cpp`) all now freed with `delete[]` to match their `new T[n]` allocations. Also found and fixed the same pattern via `ScopedPointer` in `PluginProcessor.cpp` (`setStateInformation`/`setCurrentProgramStateInformation`) — `ScopedPointer` always calls scalar `delete` (per JUCE's own doc comment), so `ScopedPointer pUncompressedData = new char[...]` was the same bug; replaced with `HeapBlock`, JUCE's array-owning smart pointer. +- [x] **`malloc` was never null-checked** across the C DSP core. Added a shared `SynthCheckAlloc()` helper (`synth_defs.h`/`synth_debug.c`) that aborts with a clear diagnostic instead of returning NULL, and wrapped all 24 `malloc` call sites across `env.c`, `lfo.c`, `vcf.c`, `vco.c`, `blit.c`, `wavetable.c`, `voice.c`, `param_scale.c`. All are one-time init/bufsize-change calls, never in the per-block render path, so this adds no real-time overhead. ## Patch/Bank import-export diff --git a/src/plug/JaySynth.cpp b/src/plug/JaySynth.cpp index 3e9f1d3..3263409 100644 --- a/src/plug/JaySynth.cpp +++ b/src/plug/JaySynth.cpp @@ -158,7 +158,7 @@ JaySynth::~JaySynth() { delete(m_ppAudioThread[i]); } - delete(m_ppAudioThread); + delete[] m_ppAudioThread; SynthDebug("delete params\n"); paramInfoFree(&pb_range[0]); @@ -171,25 +171,25 @@ JaySynth::~JaySynth() { paramInfoFree(&humanize_voice_param[i][j]); } - delete (humanize_voice_param[i]); - delete (ppHumanizedSliders[i]); + delete[] humanize_voice_param[i]; + delete[] ppHumanizedSliders[i]; } - delete (humanize_voice_param); - delete (ppHumanizedSliders); + delete[] humanize_voice_param; + delete[] ppHumanizedSliders; for (i = max_num_voices; --i >= 0;) { removeVoice(i); } - delete (pPer_voice_controls); - delete (pCurrNoteInfos); - delete (m_pVoices); + delete[] pPer_voice_controls; + delete[] pCurrNoteInfos; + delete[] m_pVoices; for (i=0; i < m_num_audiothreads; i++) { delete (m_ppEventAudioThreadRdy[i]); } - delete(m_ppEventAudioThreadRdy); + delete[] m_ppEventAudioThreadRdy; } diff --git a/src/plug/PluginProcessor.cpp b/src/plug/PluginProcessor.cpp index 6d78bf0..20c6dc5 100644 --- a/src/plug/PluginProcessor.cpp +++ b/src/plug/PluginProcessor.cpp @@ -972,7 +972,7 @@ void JaySynthAudioProcessor::bankImportXml (const XmlElement *xml) void JaySynthAudioProcessor::setStateInformation (const void* data, int sizeInBytes) { - ScopedPointer pUncompressedData = new char[8192*1024]; + HeapBlock pUncompressedData (8192*1024); MemoryInputStream inputStream(data, sizeInBytes, false); GZIPDecompressorInputStream GZIPDec(inputStream); @@ -1093,8 +1093,7 @@ void JaySynthAudioProcessor::setCurrentProgramStateInformation (const void* data void* pData; int size; - ScopedPointer pUncompressedData; - pUncompressedData = new char[2*65536]; + HeapBlock pUncompressedData (2*65536); MemoryInputStream inputStream(data, sizeInBytes, false); GZIPDecompressorInputStream GZIPDec(inputStream); diff --git a/src/synth/blit.c b/src/synth/blit.c index dee60d7..92f9093 100644 --- a/src/synth/blit.c +++ b/src/synth/blit.c @@ -139,7 +139,7 @@ void BLIT_ModInit(blit_common_t *pCom) synth_float_t x, dx, a, b, blit, blep; // Calculate table sizes - pCom->pTableSizes = (UINT32*)malloc(BLIT_NUM_HARM_MAX*sizeof(UINT32)); + pCom->pTableSizes = (UINT32*)SynthCheckAlloc(malloc(BLIT_NUM_HARM_MAX*sizeof(UINT32))); for (m=0; m < BLIT_NUM_HARM_MAX; m++) { pCom->pTableSizes[m] = m*BLIT_TABLE_OVERSAMPLING; @@ -148,18 +148,18 @@ void BLIT_ModInit(blit_common_t *pCom) } // Generate Kaiser window - ppKaiser = (synth_float_t**)malloc(BLIT_NUM_HARM_MAX*sizeof(synth_float_t*)); + ppKaiser = (synth_float_t**)SynthCheckAlloc(malloc(BLIT_NUM_HARM_MAX*sizeof(synth_float_t*))); for (m=0; m < BLIT_NUM_HARM_MAX; m++) { - ppKaiser[m] = (synth_float_t*)malloc(pCom->pTableSizes[m]*sizeof(synth_float_t)); + ppKaiser[m] = (synth_float_t*)SynthCheckAlloc(malloc(pCom->pTableSizes[m]*sizeof(synth_float_t))); CalcKaiser(NULL, ppKaiser[m], 8.f, pCom->pTableSizes[m]); } // Allocate BLIT table - pCom->ppBLEP = (synth_float_t**)malloc(BLIT_NUM_HARM_MAX*sizeof(synth_float_t*)); + pCom->ppBLEP = (synth_float_t**)SynthCheckAlloc(malloc(BLIT_NUM_HARM_MAX*sizeof(synth_float_t*))); for (m=0; m < BLIT_NUM_HARM_MAX; m++) { - pCom->ppBLEP[m] = (synth_float_t*)malloc(pCom->pTableSizes[m]*sizeof(synth_float_t)); + pCom->ppBLEP[m] = (synth_float_t*)SynthCheckAlloc(malloc(pCom->pTableSizes[m]*sizeof(synth_float_t))); } // Generate BLIT diff --git a/src/synth/env.c b/src/synth/env.c index a0f945b..cb56be4 100644 --- a/src/synth/env.c +++ b/src/synth/env.c @@ -43,7 +43,7 @@ void ENV_SetBufsize(envgen_t *pObj, UINT32 size) if (!size) return; - pObj->pOut = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); + pObj->pOut = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); } void ENV_SetFS(envgen_t *pObj, synth_float_t fs) diff --git a/src/synth/lfo.c b/src/synth/lfo.c index 64134c1..2197327 100644 --- a/src/synth/lfo.c +++ b/src/synth/lfo.c @@ -276,9 +276,9 @@ void LFO_SetBufsize(lfo_t *pObj, UINT32 size) if (!size) return; - pObj->pOut = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pPhase = (sync_result_t*)malloc(pObj->bufsize*sizeof(sync_result_t)); - pObj->pWave = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); + pObj->pOut = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pPhase = (sync_result_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(sync_result_t))); + pObj->pWave = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); } diff --git a/src/synth/param_scale.c b/src/synth/param_scale.c index ef5b9c3..9518523 100644 --- a/src/synth/param_scale.c +++ b/src/synth/param_scale.c @@ -986,7 +986,7 @@ void paramInfoInit(param_info_t *pObj, UINT32 id, const char *pName) pObj->pName = NULL; if (pName) { - pObj->pName = (char*)malloc(strlen(pName)+1); + pObj->pName = (char*)SynthCheckAlloc(malloc(strlen(pName)+1)); memcpy(pObj->pName, pName, strlen(pName)+1); } diff --git a/src/synth/synth_debug.c b/src/synth/synth_debug.c index 21b95f7..e2f04e7 100644 --- a/src/synth/synth_debug.c +++ b/src/synth/synth_debug.c @@ -6,6 +6,7 @@ #endif #include #include +#include // -------------------------------------------------------------- // internal funcs @@ -14,6 +15,17 @@ // Exported functions // -------------------------------------------------------------- +void *SynthCheckAlloc(void *ptr) +{ + if (!ptr) + { + fprintf(stderr, "JaySynth: out of memory, aborting.\n"); + abort(); + } + + return ptr; +} + void SynthDebug(const char *fmtstr, ...) { #ifndef SYNTH_DEBUG diff --git a/src/synth/synth_defs.h b/src/synth/synth_defs.h index 2e39060..c172569 100644 --- a/src/synth/synth_defs.h +++ b/src/synth/synth_defs.h @@ -55,6 +55,11 @@ extern "C" { #endif void SynthDebug(const char *fmtstr, ...); + +// Checks the result of a malloc() call. Aborts with a diagnostic instead of +// returning NULL, so an allocation failure never turns into a silent +// null-pointer dereference deep inside DSP processing. +void *SynthCheckAlloc(void *ptr); #if defined(__cplusplus) } #endif diff --git a/src/synth/vcf.c b/src/synth/vcf.c index 855091c..ad93f0f 100644 --- a/src/synth/vcf.c +++ b/src/synth/vcf.c @@ -55,8 +55,8 @@ void VCF_ModInit(vcf_t *pObj) { vcf_common_t *pCom = pObj->pCom; - pCom->pLUT_cos = (synth_float_t*)malloc(FILTER_TABLE_SIZE*sizeof(synth_float_t)); - pCom->pLUT_sin = (synth_float_t*)malloc(FILTER_TABLE_SIZE*sizeof(synth_float_t)); + pCom->pLUT_cos = (synth_float_t*)SynthCheckAlloc(malloc(FILTER_TABLE_SIZE*sizeof(synth_float_t))); + pCom->pLUT_sin = (synth_float_t*)SynthCheckAlloc(malloc(FILTER_TABLE_SIZE*sizeof(synth_float_t))); } void VCF_ModFree(vcf_t *pObj) @@ -141,8 +141,8 @@ void VCF_SetBufsize(vcf_t *pObj, UINT32 size) if (!size) return; - pObj->pOut = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pCoef = (vcf_coef_t*)malloc(pObj->bufsize*sizeof(vcf_coef_t)); + pObj->pOut = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pCoef = (vcf_coef_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(vcf_coef_t))); pObj->pCoeff_last = pObj->pCoef; VCF_Coeff_update(pObj); diff --git a/src/synth/vco.c b/src/synth/vco.c index 62af8d3..bf17156 100644 --- a/src/synth/vco.c +++ b/src/synth/vco.c @@ -87,7 +87,7 @@ void VCO_SetBufsize(osc_t *pObj, UINT32 size) if (!size) return; - pObj->pOut = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); + pObj->pOut = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); } diff --git a/src/synth/voice.c b/src/synth/voice.c index 24b8323..7f63dfb 100644 --- a/src/synth/voice.c +++ b/src/synth/voice.c @@ -306,23 +306,23 @@ void VoiceSetBufsize(voice_t *pObj, UINT32 size) if (!size) return; - pObj->pBuf_Q = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); + pObj->pBuf_Q = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); for (i = 0; i < (INT32)VOICE_NUM_OSC; i++) { - pObj->pBuf_VCO_fmout[i] = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vco_pitch[i] = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_osc_out_smoothed[i] = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); + pObj->pBuf_VCO_fmout[i] = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vco_pitch[i] = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_osc_out_smoothed[i] = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); } - pObj->pBuf_vco_pwm = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vco_fm = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vco_am = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vcf_fm = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vcf_qm = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vca_am = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_vca_pan = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); - pObj->pBuf_osc_out = (synth_float_t*)malloc(pObj->bufsize*sizeof(synth_float_t)); + pObj->pBuf_vco_pwm = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vco_fm = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vco_am = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vcf_fm = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vcf_qm = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vca_am = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_vca_pan = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); + pObj->pBuf_osc_out = (synth_float_t*)SynthCheckAlloc(malloc(pObj->bufsize*sizeof(synth_float_t))); } void VoiceEvent(voice_t *pObj, UINT32 type) diff --git a/src/synth/wavetable.c b/src/synth/wavetable.c index 28df102..7158202 100644 --- a/src/synth/wavetable.c +++ b/src/synth/wavetable.c @@ -173,10 +173,10 @@ void WT_ModInit(wt_common_t *pCom) // Allocate memory for (i=0; i < WT_NUM_WAVETABLES; i++) { - pCom->ppWavetables[i] = (synth_float_t**)malloc(WT_WAVETABLE_NUM_ENTRIES*sizeof(synth_float_t*)); + pCom->ppWavetables[i] = (synth_float_t**)SynthCheckAlloc(malloc(WT_WAVETABLE_NUM_ENTRIES*sizeof(synth_float_t*))); for (j=0; j < WT_WAVETABLE_NUM_ENTRIES; j++) { - pCom->ppWavetables[i][j] = (synth_float_t*)malloc(WT_WAVE_SIZE*sizeof(synth_float_t)); + pCom->ppWavetables[i][j] = (synth_float_t*)SynthCheckAlloc(malloc(WT_WAVE_SIZE*sizeof(synth_float_t))); } }