MoogLadders icon indicating copy to clipboard operation
MoogLadders copied to clipboard

Uninitialized memory access in MusicDSPModel constructor

Open L3337 opened this issue 1 year ago • 0 comments

Nice project.

I spotted this while I was studying the code:

The constructor calls SetCutoff SetCutoff calls SetResonance

2 problems AFAICT with that:

  • resonance is uninitialized on the first call, could conceivably crash if arbitrary junk at that memory address was a NaN or similar
  • SetCutoff calling SetResonance with the calculated resonance coefficient and not a 0.0 to 1.0 resonance value is probably(?) incorrect behavior. With DSP, you never really know, but in this case it results in different filter coefficients depending on whether SetResonance was called before SetCutoff. If there is a hard requirement for the order the methods are called in, the methods should just be merged into a single method, especially since one already calls the other.

Would also recommend to create a test suite to catch memory errors automatically. Write a separate executable that loads some arbitrary wav file, runs all of the filter permutations against it, then run the executable through valgrind to check for memory errors.

L3337 avatar Jan 22 '23 20:01 L3337