Integer overflow in AudioBlock internal functions

I just ran into a crash caused by an integer overflow when clearing a huge AudioBlock of SIMDRegister<float> samples:

As you can see in the debugger screenshot above, the int cast results in a negative number. I see that the int cast is necessary to interface with FloatVectorOperations::clear – which in turn internally casts the values back to size_t :roll_eyes:

Would be great if that could be fixed somehow, not sure if e.g. templating the size type of all FloatVectorOperations would be an option? For now, I just replaced the call to FloatVectorOperations::clear with a plain zeromem in our juce fork which resolves the issue for us but that doesn’t seem like a general solution and I just wait until we run into another case like that for another AudioBlock function.

most of the C++ committee agrees that ‘size_t’ being unsigned was a mistake, for this very reason (integer overflow). Unless you really expect a plugin to have more than 2,147,483,647 channels, just use ‘int’.

1 Like

In fact, I think the committee advises to just use int, unless you have a really good reason not to! Unsigned simply has too many potential issues.

2 Likes

I think you both are missing the point here. This is not about unsigned vs signed integer. This is about casting a 64 bit size_t into a 32 bit int.

And no, of course I don’t expect my plugin too have more than 2,147,483,647 channels but as you can see in the debugger screenshot above our current plugin indeed needs a buffer capable of holding 1,888,608,256 dsp::SIMDRegister<float> samples in a buffer. For a SIMDRegister<float> the sizeFactor in the audio buffer is 4 so the multiplication result numSamples * sizeFactor in the juce::dsp::AudioBlock internals results in 7,554,433,024 samples in the buffer to be cleared. This value is then casted into a 32 bit int which results in a negative number that can be seen in the debugger screenshot which is passed to FloatVectorOperations::clear, resulting in a crash.

It seems that the maximum number of samples that can be handled by some dsp::AudioBlock member functions is limited by the maximum value of a 32 bit int, although the class uses 64 bit types for the sample count.

I would be totally fine with using signed 64 bit integers, this would definitively solve the problem, so again, this is not about signed vs unsigned but about casting 64 bit to 32 bit and this is about a real issue I ran into and not about some theoretical thing like

1 Like

Ah, you’re holding 7.5 Billion samples in a buffer? That’s quite a lot :wink: (but not unheard of)

Now that the team obviously added size_t support to FloatVectorOperations (thanks for that!)

wouldn’t it be a good idea to remove the now not longer needed int casts in these internal functions to resolve the issues that I described here?

Yes, that’s exactly what I do :wink:

I have that change on a branch, and it should be on develop in a few days.

1 Like

Great news, thank you

Thanks for your patience, this change is now on develop:

Thank you, that should fix all the issues we were having, I’ll pull that changes next week.

One thing that came to my mind regarding the related FloatVectorOperations refactoring: While the implementation strategy with the detail::NameForwarder template seems quite clever, with that change all the functions disappeared from the Doxygen generated documentation. This is what the dev branch documentation looks like right now on the JUCE website:

Developers with some JUCE experience will know that the old functions are there, but newcomers might have a hard time finding them if they don’t find them by chance with their IDE’s autocompletion. I’m no Doxygen pro, but is there any trick to at least make the now undocumented functions re-appear there?

Thanks for pointing that out, I’ll try to think of a workaround.