Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

std::vector "Out of Bounds" memory violation in ResampleAndReturnResults (Undefined behavior) #26

Open
MykonCodes opened this issue Jan 15, 2020 · 1 comment

Comments

@MykonCodes
Copy link

Hey there. Thanks for your great API.
We're in the middle of integrating REAPER into our software, and it seems awesome. However, we noticed a lot of seemingly random crashes, related to either allocation or deallocation, when running a lot of iterations of ResampleAndReturnResults. Looking through it, I stumbled across

  float last_time = (output_[0].resid_index / sample_rate_) + endpoint_padding_;
  int32_t n_frames = RoundUp(last_time / resample_interval);

[...]
for (int32_t i = limit; i >= 0; --i) {
  int32_t frame = RoundUp(output_[i].resid_index / (sample_rate_ * resample_interval));
[...]
}

Let's just do some math here.

Let's assume:
output[0].resid_index = 37976
sampleRate = 44100
resample_interval = 0.02

Yields:
lastTime = 0,8611
n_frames = 44

In the for loop, if i == 0,
frame = 37976 / 882 = 43,05 (round up ->) = 44

That's an out-of-bounds for the f0 vector that's only allocated 43, trying to access 44 (counting from zero). I suspect this to be the reason for the "random" crashes later on. What would the expected behavior be? Should it rather just be:

int32_t frame = RoundUp(output_[i].resid_index / (sample_rate_ * resample_interval)) -1;

?

All the best,
Sebastian

@acsuwut
Copy link

acsuwut commented Nov 13, 2020

I have found this same memory corruption. A few notes from what I found:

Our team believes RoundUp() to be misleadingly named, it is doing traditional rounding where [X + .5, X + 1.5) will round to X. It's more of just...Round.

The point still remains the same. I can end up with a situation where, for example, the float being rounded for size is (97.499999999) and the float being rounded for index is (96.5), so the vector ends up overrunning by one.

I duplicated this from the command line. I unfortunately can't share the wav, but this command:

build/reaper -i <some_unlucky_wav_file> -f freq.f0 -p pitchmark.pm -a -e 0.01

will cause a memory corruption (I hunted down the source using ASAN and it identified this invalid write as described).

It doesn't happen with every wav file, but once you find one where it happens it is consistent.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants