Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid stack overflow in R2Stretcher::study. #102

Closed
wants to merge 1 commit into from

Conversation

psobot
Copy link

@psobot psobot commented Jun 18, 2024

The study method calls alloca in a loop, which causes a large amount of data to be allocated on the stack - proportional to the provided input. (alloca does not follow C++'s RAII rules; the allocated memory stays on the stack until the function exits, rather than the enclosing scope.) With a large enough input, this can cause a stack overflow.

This PR avoids the problem by only calling alloca at most once and re-using the buffer if necessary.

(Discovered by @f0k in spotify/pedalboard#340.)

The `study` method calls `alloca` in a loop, which causes a large amount of data to be allocated on the stack - proportional to the provided input. (`alloca` does not follow C++'s RAII rules; the allocated memory stays on the stack until the function exits, rather than the enclosing scope.) With a large enough input, this can cause a stack overflow.

This commit avoids the problem by only calling `alloca` at most once and re-using the buffer if necessary.
@cannam
Copy link
Member

cannam commented Jun 19, 2024

Oof, that's nasty. Thanks for the report! I may look at doing this without alloca (which was always a bit gross).

@cannam
Copy link
Member

cannam commented Jun 26, 2024

Fixed in 832f577. Thanks again!

@cannam cannam closed this Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants