Add RB_GC_GUARD to numo_pocketfft_fft to fix crash #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In yoshoku#3 I describe a crash I have been experiencing with numo-pocketfft. When running RSpec tests in the mb-sound project, my FFT tests crash frequently, and occasionally return incorrect results. These errors only happen at the `-O3` optimization level. It appears this is due to Ruby's GC freeing either `x_val` or `z_val` before the function has finished, and other data being written in its place. This fix uses the RB_GC_GUARD macro to make sure that gcc preserves the `x_val` and `z_val` `VALUE`s in stack or registers so that the GC knows they are still in use. ## Testing To test the fix I ran `rspec` in a loop in the `fft` branch of the `mb-sound` project, as well as the reproducing script in yoshoku#3. Before this change, tests would crash frequently. After this change, tests ran without issue for many iterations.
mike-bourgeous
added a commit
to mike-bourgeous/mb-sound
that referenced
this pull request
Jan 5, 2021
yoshoku
reviewed
Jan 15, 2021
mike-bourgeous
added a commit
to mike-bourgeous/mb-sound
that referenced
this pull request
Jan 16, 2021
|
Thank you for your contribution 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #3 I describe a crash I have been experiencing with numo-pocketfft. When running RSpec tests in the mb-sound project, my FFT tests crash frequently, and occasionally return incorrect results. These errors only happen at the
-O3optimization level. It appears this is due to Ruby's GC freeing eitherx_valorz_valbefore the function has finished, and other data being written in its place.This fix uses the RB_GC_GUARD macro to make sure gcc preserves the
x_valandz_valVALUEs in stack or registers so that the GC knows they are still in use.See https://ruby-doc.org/core-2.7.1/doc/extension_rdoc.html#label-Appendix+E.+RB_GC_GUARD+to+protect+from+premature+GC
Testing
To test the fix I ran
rspecin a loop in thefftbranch of themb-soundproject, as well as the reproducing script in #3. Before this change, tests would crash frequently. After this change, tests ran without issue for many iterations.